Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Object.defineProperty #200

Closed
wants to merge 1 commit into from

Conversation

jordanbtucker
Copy link
Member

Properties should be set with Object.defineProperty to maintain backward compatibility with JSON.parse.

Currently JSON5 exhibits the following behavior.

    JSON.parse(`{"__proto__": 1}`)
//  {__proto__: 1}
    JSON5.parse(`{"__proto__": 1}`)
//  {}

However, when JSON5 uses Object.defineProperty instead of object[key] = value to set the properties of objects, it maintains backward compatibility.

Fixes #199

@jordanbtucker jordanbtucker self-assigned this Jun 6, 2019
@jordanbtucker
Copy link
Member Author

The question is, is this a major, minor, or patch version bump?

@LongTengDao
Copy link

What do you mean for:

However, when JSON5 uses Object.defineProperty instead of object[key] = value to set the properties of objects, it maintains backward compatibility.

@jordanbtucker
Copy link
Member Author

@LongTengDao Before this PR, JSON5 sets object properties by using the syntax object[key] = value.

This is a problem for the special property __proto__ because it is not enumerable or writable. In other words, object['__proto__'] = value has no effect.

However, if you use Object.defineProperty, then you can create a new __proto__ property that is enumerable and writable and hides the original __proto__ property. This is how JSON does it (via CreateDataProperty), and that's what this PR implements.

@LongTengDao
Copy link

LongTengDao commented Aug 28, 2019

This is a problem for the special property __proto__ because it is not enumerable or writable. In other words, object['__proto__'] = value has no effect.

__proto__ is a getter/setter on Object.prototype. It only ignore setting primitive value (exclude null). If set object or function to it, there is side effect (but be proto, not property, which not expected here).

When would this PR be released?

@jordanbtucker jordanbtucker added this to the v3.0.0 milestone Oct 4, 2019
@jordanbtucker
Copy link
Member Author

jordanbtucker commented Oct 1, 2022

Merged in 4a8c456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should __proto__ property be treated specially?
2 participants