Title: [233918] trunk
- Revision
- 233918
- Author
- [email protected]
- Date
- 2018-07-18 11:31:09 -0700 (Wed, 18 Jul 2018)
Log Message
[JSC] JSON.stringify's replacer should use `isArray` instead of JSArray checks
https://bugs.webkit.org/show_bug.cgi?id=187755
Reviewed by Mark Lam.
JSTests:
* stress/json-stringify-gap-calculation-should-be-after-replacer-check.js: Added.
(shouldThrow):
(shouldThrow.string.toString):
* test262/expectations.yaml:
Source/_javascript_Core:
JSON.stringify used `inherits<JSArray>(vm)` to determine whether the given replacer is an array replacer.
But this is wrong. According to the spec, we should use `isArray`[1], which accepts Proxies. This difference
makes one test262 test failed.
This patch changes the code to using `isArray()`. And we reorder the evaluations of replacer check and ident space check
to align these checks to the spec's order.
[1]: https://tc39.github.io/ecma262/#sec-json.stringify
* runtime/JSONObject.cpp:
(JSC::Stringifier::Stringifier):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (233917 => 233918)
--- trunk/JSTests/ChangeLog 2018-07-18 18:27:08 UTC (rev 233917)
+++ trunk/JSTests/ChangeLog 2018-07-18 18:31:09 UTC (rev 233918)
@@ -1,3 +1,15 @@
+2018-07-18 Yusuke Suzuki <[email protected]>
+
+ [JSC] JSON.stringify's replacer should use `isArray` instead of JSArray checks
+ https://bugs.webkit.org/show_bug.cgi?id=187755
+
+ Reviewed by Mark Lam.
+
+ * stress/json-stringify-gap-calculation-should-be-after-replacer-check.js: Added.
+ (shouldThrow):
+ (shouldThrow.string.toString):
+ * test262/expectations.yaml:
+
2018-07-12 Yusuke Suzuki <[email protected]>
[JSC] Generator and AsyncGeneratorMethod's prototype is incorrect
Added: trunk/JSTests/stress/json-stringify-gap-calculation-should-be-after-replacer-check.js (0 => 233918)
--- trunk/JSTests/stress/json-stringify-gap-calculation-should-be-after-replacer-check.js (rev 0)
+++ trunk/JSTests/stress/json-stringify-gap-calculation-should-be-after-replacer-check.js 2018-07-18 18:31:09 UTC (rev 233918)
@@ -0,0 +1,33 @@
+var proxy = Proxy.revocable([], {});
+proxy.revoke();
+
+function shouldThrow(func, errorMessage) {
+ var errorThrown = false;
+ var error = null;
+ try {
+ func();
+ } catch (e) {
+ errorThrown = true;
+ error = e;
+ }
+ if (!errorThrown)
+ throw new Error('not thrown');
+ if (String(error) !== errorMessage)
+ throw new Error(`bad error: ${String(error)}`);
+}
+
+shouldThrow(() => {
+ var string = new String("Hello");
+ string.toString = function () {
+ throw new Error("Out");
+ };
+ JSON.stringify({}, proxy.proxy, string);
+}, `TypeError: Array.isArray cannot be called on a Proxy that has been revoked`);
+
+shouldThrow(() => {
+ var string = new String("Hello");
+ string.toString = function () {
+ throw new Error("Out");
+ };
+ JSON.stringify({}, [], string);
+}, `Error: Out`);
Modified: trunk/JSTests/test262/expectations.yaml (233917 => 233918)
--- trunk/JSTests/test262/expectations.yaml 2018-07-18 18:27:08 UTC (rev 233917)
+++ trunk/JSTests/test262/expectations.yaml 2018-07-18 18:31:09 UTC (rev 233918)
@@ -1034,9 +1034,6 @@
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/JSON/stringify/replacer-proxy-revoked.js:
- default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
- strict mode: 'Test262Error: Expected a TypeError 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'
Modified: trunk/Source/_javascript_Core/ChangeLog (233917 => 233918)
--- trunk/Source/_javascript_Core/ChangeLog 2018-07-18 18:27:08 UTC (rev 233917)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-07-18 18:31:09 UTC (rev 233918)
@@ -1,5 +1,24 @@
2018-07-18 Yusuke Suzuki <[email protected]>
+ [JSC] JSON.stringify's replacer should use `isArray` instead of JSArray checks
+ https://bugs.webkit.org/show_bug.cgi?id=187755
+
+ Reviewed by Mark Lam.
+
+ JSON.stringify used `inherits<JSArray>(vm)` to determine whether the given replacer is an array replacer.
+ But this is wrong. According to the spec, we should use `isArray`[1], which accepts Proxies. This difference
+ makes one test262 test failed.
+
+ This patch changes the code to using `isArray()`. And we reorder the evaluations of replacer check and ident space check
+ to align these checks to the spec's order.
+
+ [1]: https://tc39.github.io/ecma262/#sec-json.stringify
+
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::Stringifier):
+
+2018-07-18 Yusuke Suzuki <[email protected]>
+
[JSC] Root wrapper object in JSON.stringify is not necessary if replacer is not callable
https://bugs.webkit.org/show_bug.cgi?id=187752
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (233917 => 233918)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-07-18 18:27:08 UTC (rev 233917)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-07-18 18:31:09 UTC (rev 233918)
@@ -226,37 +226,35 @@
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- m_gap = gap(exec, space);
- if (UNLIKELY(scope.exception()))
- return;
+ if (m_replacer.isObject()) {
+ JSObject* replacerObject = asObject(m_replacer);
- if (!m_replacer.isObject())
- return;
-
- JSObject* replacerObject = asObject(m_replacer);
- if (replacerObject->inherits<JSArray>(vm)) {
- m_usingArrayReplacer = true;
- JSArray* array = jsCast<JSArray*>(replacerObject);
- unsigned length = array->get(exec, vm.propertyNames->length).toUInt32(exec);
- if (UNLIKELY(scope.exception()))
- return;
- for (unsigned i = 0; i < length; ++i) {
- JSValue name = array->get(exec, i);
- if (UNLIKELY(scope.exception()))
- break;
-
- if (auto* nameObject = jsDynamicCast<JSObject*>(vm, name)) {
- if (!nameObject->inherits<NumberObject>(vm) && !nameObject->inherits<StringObject>(vm))
- continue;
- } else if (!name.isNumber() && !name.isString())
- continue;
-
- m_arrayReplacerPropertyNames.add(name.toString(exec)->toIdentifier(exec));
+ m_replacerCallType = CallType::None;
+ if (!replacerObject->isCallable(vm, m_replacerCallType, m_replacerCallData)) {
+ bool isArrayReplacer = JSC::isArray(exec, replacerObject);
+ RETURN_IF_EXCEPTION(scope, );
+ if (isArrayReplacer) {
+ m_usingArrayReplacer = true;
+ unsigned length = replacerObject->get(exec, vm.propertyNames->length).toUInt32(exec);
+ RETURN_IF_EXCEPTION(scope, );
+ for (unsigned i = 0; i < length; ++i) {
+ JSValue name = replacerObject->get(exec, i);
+ RETURN_IF_EXCEPTION(scope, );
+ if (name.isObject()) {
+ auto* nameObject = jsCast<JSObject*>(name);
+ if (!nameObject->inherits<NumberObject>(vm) && !nameObject->inherits<StringObject>(vm))
+ continue;
+ } else if (!name.isNumber() && !name.isString())
+ continue;
+ m_arrayReplacerPropertyNames.add(name.toString(exec)->toIdentifier(exec));
+ RETURN_IF_EXCEPTION(scope, );
+ }
+ }
}
- return;
}
- m_replacerCallType = replacerObject->methodTable(vm)->getCallData(replacerObject, m_replacerCallData);
+ scope.release();
+ m_gap = gap(exec, space);
}
JSValue Stringifier::stringify(JSValue value)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
