JSON.parse should not modify frozen objects

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

The ES6 spec for JSON.parse (https://tc39.github.io/ecma262/#sec-json.parse and https://tc39.github.io/ecma262/#sec-internalizejsonproperty) states that it uses CreateDataProperty() (https://tc39.github.io/ecma262/#sec-createdataproperty) to set values returned by a reviver.  The spec for CreateDataPropertyOrThrow states:

"This abstract operation creates a property whose attributes are set to the same defaults used for properties created by the ECMAScript language assignment operator. Normally, the property will not already exist. If it does exist and is not configurable or if O is not extensible, [[DefineOwnProperty]] will return false."

Since the properties of frozen objects are not extensible, not configurable, and not writeable, JSON.parse should fail to write to any frozen objects.  Similarly, JSON.parse should fail to delete properties in frozen objects.

[Mark Lam]

Created attachment 291578 [details]
proposed patch.

[Saam Barati]

Comment on attachment 291578 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=291578&action=review

> JSTests/stress/json-parse-on-frozen-object.js:11
> +        this.b = Object.freeze(["unmodifiable"]);

This is a nice test file. 
Can you also add a test for making the property at 0 non-configurable and non-writable?

> JSTests/stress/json-parse-on-frozen-object.js:38
> +        return undefined;

The behavior here is to indicate that property 0 should be deleted?

[Mark Lam]

(In reply to comment #2)
> Comment on attachment 291578 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291578&action=review
> 
> > JSTests/stress/json-parse-on-frozen-object.js:11
> > +        this.b = Object.freeze(["unmodifiable"]);
> 
> This is a nice test file. 
> Can you also add a test for making the property at 0 non-configurable and
> non-writable?

I'll work on that.
 
> > JSTests/stress/json-parse-on-frozen-object.js:38
> > +        return undefined;
> 
> The behavior here is to indicate that property 0 should be deleted?

Yes.  It's in the spec.  If the reviver returns undefined, JSON.parse deletes the property.

[Mark Lam]

(In reply to comment #3)
> (In reply to comment #2)
> > Can you also add a test for making the property at 0 non-configurable and
> > non-writable?
> 
> I'll work on that.

Interesting.  This uncovered a bug in a different area. I'll land this patch first, and fix that in a separate bug and add the non-configurable non-writable test there.

[Mark Lam]

I'll address the bug with non-configurable properties in https://bugs.webkit.org/show_bug.cgi?id=163446.

Landed in r207341: <http://trac.webkit.org/r207341>.

[Mark Lam]

<rdar://problem/15221406>

[Mark Lam]

*** Bug 122759 has been marked as a duplicate of this bug. ***