Attachment 373178 Details for Bug 163446 – Patch

[patch] Patch

bug-163446-20190630013306.patch (text/plain), 7.54 KB, created by Alexey Shvayka on 2019-06-29 15:33:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Shvayka

Created: 2019-06-29 15:33:07 PDT

Size: 7.54 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 246951)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,12 @@
>+2019-06-29  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSON.parse should not modify non-configurable properties.
>+        https://bugs.webkit.org/show_bug.cgi?id=163446
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/json-parse-reviver-non-configurable.js: Added.
>+
> 2019-06-28  Justin Michaud  <justin_michaud@apple.com>
> 
>         Add b3 macro lowering for CheckMul on arm64
>Index: JSTests/stress/json-parse-reviver-non-configurable.js
>===================================================================
>--- JSTests/stress/json-parse-reviver-non-configurable.js	(nonexistent)
>+++ JSTests/stress/json-parse-reviver-non-configurable.js	(working copy)
>@@ -0,0 +1,35 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function objReviver(key, value) {
>+    if (key === 'a')
>+        Object.defineProperty(this, 'b', {configurable: false});
>+    if (key === 'b') return 12;
>+
>+    return value;
>+}
>+
>+function arrReviver(key, value) {
>+    if (key === '0')
>+        Object.defineProperty(this, '1', {configurable: false});
>+    if (key === '1') return 24;
>+
>+    return value;
>+}
>+
>+const objJSON = '{"a": 1, "b": 2}';
>+const arrJSON = '[3, 4]';
>+
>+for (let i = 1; i < 10000; i++) {
>+    let obj = JSON.parse(objJSON, objReviver);
>+    shouldBe(obj.a, 1);
>+    shouldBe(obj.b, 2);
>+}
>+
>+for (let i = 1; i < 10000; i++) {
>+    let arr = JSON.parse(arrJSON, arrReviver);
>+    shouldBe(arr[0], 3);
>+    shouldBe(arr[1], 4);
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 246841)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-06-29  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSON.parse should not modify non-configurable properties.
>+        https://bugs.webkit.org/show_bug.cgi?id=163446
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Spec: https://tc39.es/ecma262/#sec-internalizejsonproperty, step 2.
>+
>+        * runtime/JSONObject.cpp: Use [[DefineOwnProperty]] instead of [[Set]].
>+        * runtime/JSObject.h: Clarify comments regarding [[DefineOwnProperty]].
>+
> 2019-06-26  Keith Miller  <keith_miller@apple.com>
> 
>         remove unneeded didBecomePrototype() calls
>Index: Source/JavaScriptCore/runtime/JSONObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSONObject.cpp	(revision 246841)
>+++ Source/JavaScriptCore/runtime/JSONObject.cpp	(working copy)
>@@ -709,8 +709,11 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>                 RETURN_IF_EXCEPTION(scope, { });
>                 if (filteredValue.isUndefined())
>                     array->methodTable(vm)->deletePropertyByIndex(array, m_exec, indexStack.last());
>-                else
>-                    array->putDirectIndex(m_exec, indexStack.last(), filteredValue, 0, PutDirectIndexShouldNotThrow);
>+                else {
>+                    PropertyDescriptor desc(filteredValue, static_cast<unsigned>(PropertyAttribute::None));
>+                    bool shouldThrow = false;
>+                    array->defineOwnIndexedProperty(m_exec, indexStack.last(), desc, shouldThrow);
>+                }
>                 RETURN_IF_EXCEPTION(scope, { });
>                 indexStack.last()++;
>                 goto arrayStartVisitMember;
>@@ -761,13 +764,15 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>             case ObjectEndVisitMember: {
>                 JSObject* object = jsCast<JSObject*>(markedStack.last());
>                 Identifier prop = propertyStack.last()[indexStack.last()];
>-                PutPropertySlot slot(object);
>                 JSValue filteredValue = callReviver(object, jsString(m_exec, prop.string()), outValue);
>                 RETURN_IF_EXCEPTION(scope, { });
>                 if (filteredValue.isUndefined())
>                     object->methodTable(vm)->deleteProperty(object, m_exec, prop);
>-                else
>-                    object->methodTable(vm)->put(object, m_exec, prop, filteredValue, slot);
>+                else {
>+                    PropertyDescriptor desc(filteredValue, static_cast<unsigned>(PropertyAttribute::None));
>+                    bool shouldThrow = false;
>+                    object->methodTable(vm)->defineOwnProperty(object, m_exec, prop, desc, shouldThrow);
>+                }
>                 RETURN_IF_EXCEPTION(scope, { });
>                 indexStack.last()++;
>                 goto objectStartVisitMember;
>Index: Source/JavaScriptCore/runtime/JSObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSObject.h	(revision 246841)
>+++ Source/JavaScriptCore/runtime/JSObject.h	(working copy)
>@@ -209,7 +209,8 @@ public:
>     //  - accessors are not called.
>     //  - it will ignore extensibility and read-only properties if PutDirectIndexLikePutDirect is passed as the mode (the default).
>     // This method creates a property with attributes writable, enumerable and configurable all set to true if attributes is zero,
>-    // otherwise, it creates a property with the provided attributes. Semantically, this is performing defineOwnProperty.
>+    // otherwise, it creates a property with the provided attributes.
>+    // This is pretty much like performing defineOwnProperty, but without extensibility check and property descriptor validation.
>     bool putDirectIndex(ExecState* exec, unsigned propertyName, JSValue value, unsigned attributes, PutDirectIndexMode mode)
>     {
>         ASSERT(!value.isCustomGetterSetter());
>@@ -237,7 +238,8 @@ public:
>         }
>         return putDirectIndexSlowOrBeyondVectorLength(exec, propertyName, value, attributes, mode);
>     }
>-    // This is semantically equivalent to performing defineOwnProperty(propertyName, {configurable:true, writable:true, enumerable:true, value:value}).
>+    // This is pretty much like performing defineOwnProperty(propertyName, {configurable:true, writable:true, enumerable:true, value:value}),
>+    // but without extensibility check and property descriptor validation.
>     bool putDirectIndex(ExecState* exec, unsigned propertyName, JSValue value)
>     {
>         return putDirectIndex(exec, propertyName, value, 0, PutDirectIndexLikePutDirect);
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 246841)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-06-29  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSON.parse should not modify non-configurable properties.
>+        https://bugs.webkit.org/show_bug.cgi?id=163446
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * js/resources/JSON-parse.js:
>+        (createTests.logOrder): Always return `value` so that [[DefineOwnProperty]] would be no-op.
>+
> 2019-06-26  Russell Epstein  <russell_e@apple.com>
> 
>         Layout Test imported/mozilla/svg/text/selectSubString-2.svg is failing.
>Index: LayoutTests/js/resources/JSON-parse.js
>===================================================================
>--- LayoutTests/js/resources/JSON-parse.js	(revision 246841)
>+++ LayoutTests/js/resources/JSON-parse.js	(working copy)
>@@ -414,7 +414,7 @@ function createTests() {
>     var logOrderString;
>     function logOrder(key, value) {
>         logOrderString += key +":"+JSON.stringify(value);
>-        return null;
>+        return value;
>     }
>     result.push(function(jsonObject){
>         logOrderString = "";

Actions: View | Formatted Diff | Diff

Attachments on bug 163446: 373115 | 373116 | 373119 | 373120 | 373121 | 373128 | 373136 | 373178 | 373180 | 373199 | 373595 | 404651