Attachment 373546 Details for Bug 199292 – Patch

[patch] Patch

bug-199292-20190706011942.patch (text/plain), 8.26 KB, created by Alexey Shvayka on 2019-07-05 15:19:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Shvayka

Created: 2019-07-05 15:19:44 PDT

Size: 8.26 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 247181)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2019-07-05  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSON.parse incorrectly handles array proxies
>+        https://bugs.webkit.org/show_bug.cgi?id=199292
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/json-parse-reviver-array-proxy.js: Added.
>+        * stress/json-parse-reviver-revoked-proxy.js: Added.
>+        * test262/expectations.yaml: Mark 6 test cases as passing.
>+
> 2019-07-05  Alexey Shvayka  <shvaikalesh@gmail.com>
> 
>         [JSC] Clean up ArraySpeciesCreate
>Index: JSTests/stress/json-parse-reviver-array-proxy.js
>===================================================================
>--- JSTests/stress/json-parse-reviver-array-proxy.js	(nonexistent)
>+++ JSTests/stress/json-parse-reviver-array-proxy.js	(working copy)
>@@ -0,0 +1,28 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+const json = '{"a": 1, "b": 2}';
>+
>+for (let i = 1; i < 10000; i++) {
>+    let keys = [];
>+    let proxy = new Proxy([2, 3], {
>+        get: function(target, key) {
>+            keys.push(key);
>+            return target[key];
>+        },
>+        ownKeys: function() {
>+            throw new Error('[[OwnPropertyKeys]] should not be called');
>+        },
>+    });
>+
>+    let result = JSON.parse(json, function(key, value) {
>+        if (key === 'a')
>+            this.b = proxy;
>+        return value;
>+    });
>+
>+    shouldBe(keys.toString(), 'length,0,1');
>+    shouldBe(JSON.stringify(result), '{"a":1,"b":[2,3]}');
>+}
>Index: JSTests/stress/json-parse-reviver-revoked-proxy.js
>===================================================================
>--- JSTests/stress/json-parse-reviver-revoked-proxy.js	(nonexistent)
>+++ JSTests/stress/json-parse-reviver-revoked-proxy.js	(working copy)
>@@ -0,0 +1,48 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function revivedObjProxy(key, value) {
>+    if (key === 'a') {
>+        let {proxy, revoke} = Proxy.revocable({}, {});
>+        revoke();
>+        this.b = proxy;
>+    }
>+
>+    return value;
>+}
>+
>+function revivedArrProxy(key, value) {
>+    if (key === '0') {
>+        let {proxy, revoke} = Proxy.revocable([], {});
>+        revoke();
>+        this[1] = proxy;
>+    }
>+
>+    return value;
>+}
>+
>+const objJSON = '{"a": 1, "b": 2}';
>+const arrJSON = '[3, 4]';
>+const isArrayError = 'TypeError: Array.isArray cannot be called on a Proxy that has been revoked';
>+
>+for (let i = 1; i < 10000; i++) {
>+    let error;
>+    try {
>+        JSON.parse(objJSON, revivedObjProxy);
>+    } catch (e) {
>+        error = e;
>+    }
>+    shouldBe(error.toString(), isArrayError);
>+}
>+
>+for (let i = 1; i < 10000; i++) {
>+    let error;
>+    try {
>+        JSON.parse(arrJSON, revivedArrProxy);
>+    } catch (e) {
>+        error = e;
>+    }
>+    shouldBe(error.toString(), isArrayError);
>+}
>Index: JSTests/test262/expectations.yaml
>===================================================================
>--- JSTests/test262/expectations.yaml	(revision 247164)
>+++ JSTests/test262/expectations.yaml	(working copy)
>@@ -1037,15 +1037,6 @@ test/built-ins/Function/prototype/toStri
> test/built-ins/GeneratorFunction/proto-from-ctor-realm.js:
>   default: 'TypeError: Type error'
>   strict mode: 'TypeError: Type error'
>-test/built-ins/JSON/parse/revived-proxy.js:
>-  default: 'Test262Error: proxy for array Expected SameValue(ÃÂ«trueÃÂ», ÃÂ«falseÃÂ») to be true'
>-  strict mode: 'Test262Error: proxy for array Expected SameValue(ÃÂ«trueÃÂ», ÃÂ«falseÃÂ») to be true'
>-test/built-ins/JSON/parse/reviver-array-length-coerce-err.js:
>-  default: 'Test262Error: Expected a Test262Error to be thrown but no exception was thrown at all'
>-  strict mode: 'Test262Error: Expected a Test262Error to be thrown but no exception was thrown at all'
>-test/built-ins/JSON/parse/reviver-array-length-get-err.js:
>-  default: 'Test262Error: Expected a Test262Error to be thrown but no exception was thrown at all'
>-  strict mode: 'Test262Error: Expected a Test262Error to be thrown but no exception was thrown at all'
> test/built-ins/Map/proto-from-ctor-realm.js:
>   default: 'Test262Error: Expected SameValue(ÃÂ«[object Map]ÃÂ», ÃÂ«[object Map]ÃÂ») to be true'
>   strict mode: 'Test262Error: Expected SameValue(ÃÂ«[object Map]ÃÂ», ÃÂ«[object Map]ÃÂ») to be true'
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 247164)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2019-07-05  Alexey Shvayka  <shvaikalesh@gmail.com>
>+
>+        JSON.parse incorrectly handles array proxies
>+        https://bugs.webkit.org/show_bug.cgi?id=199292
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        1. Use isArray to correctly detect proxied arrays.
>+        2. Make "length" lookup observable to array proxies and handle exceptions.
>+
>+        * runtime/JSONObject.cpp:
>+        (JSC::Walker::walk):
>+
> 2019-07-05  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r247115.
>Index: Source/JavaScriptCore/runtime/JSONObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSONObject.cpp	(revision 247164)
>+++ Source/JavaScriptCore/runtime/JSONObject.cpp	(working copy)
>@@ -32,6 +32,7 @@
> #include "Error.h"
> #include "ExceptionHelpers.h"
> #include "JSArray.h"
>+#include "JSArrayInlines.h"
> #include "JSGlobalObject.h"
> #include "LiteralParser.h"
> #include "Lookup.h"
>@@ -662,20 +663,21 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>         switch (state) {
>             arrayStartState:
>             case ArrayStartState: {
>-                ASSERT(inValue.isObject());
>-                ASSERT(asObject(inValue)->inherits<JSArray>(vm));
>+                // isArray(inValue) is true
>                 if (markedStack.size() > maximumFilterRecursion)
>                     return throwStackOverflowError(m_exec, scope);
> 
>-                JSArray* array = asArray(inValue);
>+                JSObject* array = asObject(inValue);
>                 markedStack.appendWithCrashOnOverflow(array);
>-                arrayLengthStack.append(array->length());
>+                unsigned length = toLength(m_exec, array);
>+                RETURN_IF_EXCEPTION(scope, { });
>+                arrayLengthStack.append(length);
>                 indexStack.append(0);
>             }
>             arrayStartVisitMember:
>             FALLTHROUGH;
>             case ArrayStartVisitMember: {
>-                JSArray* array = jsCast<JSArray*>(markedStack.last());
>+                JSObject* array = asObject(markedStack.last());
>                 uint32_t index = indexStack.last();
>                 unsigned arrayLength = arrayLengthStack.last();
>                 if (index == arrayLength) {
>@@ -704,7 +706,7 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>                 FALLTHROUGH;
>             }
>             case ArrayEndVisitMember: {
>-                JSArray* array = jsCast<JSArray*>(markedStack.last());
>+                JSObject* array = asObject(markedStack.last());
>                 JSValue filteredValue = callReviver(array, jsString(m_exec, String::number(indexStack.last())), outValue);
>                 RETURN_IF_EXCEPTION(scope, { });
>                 if (filteredValue.isUndefined())
>@@ -718,7 +720,7 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>             objectStartState:
>             case ObjectStartState: {
>                 ASSERT(inValue.isObject());
>-                ASSERT(!asObject(inValue)->inherits<JSArray>(vm));
>+                // isArray(inValue) is false
>                 if (markedStack.size() > maximumFilterRecursion)
>                     return throwStackOverflowError(m_exec, scope);
> 
>@@ -778,8 +780,9 @@ NEVER_INLINE JSValue Walker::walk(JSValu
>                     outValue = inValue;
>                     break;
>                 }
>-                JSObject* object = asObject(inValue);
>-                if (object->inherits<JSArray>(vm))
>+                bool valueIsArray = isArray(m_exec, inValue);
>+                RETURN_IF_EXCEPTION(scope, { });
>+                if (valueIsArray)
>                     goto arrayStartState;
>                 goto objectStartState;
>         }

Actions: View | Formatted Diff | Diff

Attachments on bug 199292: 373059 | 373546 | 376429 | 377862 | 380451