Log Message
[JSC] JSON.stringify should handle Proxy which is non JSArray but isArray is true https://bugs.webkit.org/show_bug.cgi?id=164123
Reviewed by Mark Lam.
JSTests:
* stress/json-stringify-with-non-jsarray-array.js: Added.
(shouldBe):
(shouldBe.JSON.stringify.new.Proxy):
Source/_javascript_Core:
When JSON.stringify encounter the undefined value, the result depends
on the context. If it is produced under the object property context, we ignore
that property. On the other hand, if it is produced under the array element
context, we produce "null".
For example,
// https://tc39.github.io/ecma262/#sec-serializejsonobject section 8.b.
// Skip the property that value is undefined.
JSON.stringify({ value: undefined }); // => "{}"
// https://tc39.github.io/ecma262/#sec-serializejsonarray section 8.b.
// Write "null" when the element is undefined.
JSON.stringify([undefined]); // => "[null]"
At that time, we decide the context based on the `holder->inherits(JSArray::info())`.
But it is not correct since we have a holder that creates the array element context
but it is not JSArray subtype. ES6 Proxy to an array is one example. In that case,
`isArray(exec, proxy)` returns `true`, but `inherits(JSArray::info())` returns false.
Since we already have this `isArray()` value in Stringifier::Holder, we should reuse
this here. And this is the correct behavior in the ES6 spec.
* runtime/JSONObject.cpp:
(JSC::Stringifier::Holder::isArray):
(JSC::Stringifier::stringify):
(JSC::Stringifier::appendStringifiedValue):
(JSC::Stringifier::Holder::Holder):
(JSC::Stringifier::Holder::appendNextProperty):
Modified Paths
- trunk/JSTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/runtime/JSONObject.cpp
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (208122 => 208123)
--- trunk/JSTests/ChangeLog 2016-10-30 09:14:03 UTC (rev 208122)
+++ trunk/JSTests/ChangeLog 2016-10-30 09:14:37 UTC (rev 208123)
@@ -1,3 +1,14 @@
+2016-10-29 Yusuke Suzuki <[email protected]>
+
+ [JSC] JSON.stringify should handle Proxy which is non JSArray but isArray is true
+ https://bugs.webkit.org/show_bug.cgi?id=164123
+
+ Reviewed by Mark Lam.
+
+ * stress/json-stringify-with-non-jsarray-array.js: Added.
+ (shouldBe):
+ (shouldBe.JSON.stringify.new.Proxy):
+
2016-10-29 Saam Barati <[email protected]>
We should have a way of profiling when a get_by_id is pure and to emit a PureGetById in the DFG/FTL
Added: trunk/JSTests/stress/json-stringify-with-non-jsarray-array.js (0 => 208123)
--- trunk/JSTests/stress/json-stringify-with-non-jsarray-array.js (rev 0)
+++ trunk/JSTests/stress/json-stringify-with-non-jsarray-array.js 2016-10-30 09:14:37 UTC (rev 208123)
@@ -0,0 +1,36 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error(`bad value: ${String(actual)}`);
+}
+
+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)}`);
+}
+
+shouldBe(JSON.stringify(new Proxy([undefined], {})), `[null]`);
+shouldBe(JSON.stringify(new Proxy([function() { }], {})), `[null]`);
+shouldBe(JSON.stringify(new Proxy({ value: undefined }, {})), `{}`);
+
+shouldThrow(function () {
+ let proxy = new Proxy([], {
+ get(theTarget, propName) {
+ if (propName === 'length')
+ throw new Error('ok');
+ return 42;
+ }
+ });
+ JSON.stringify(proxy);
+}, `Error: ok`);
Modified: trunk/Source/_javascript_Core/ChangeLog (208122 => 208123)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-30 09:14:03 UTC (rev 208122)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-30 09:14:37 UTC (rev 208123)
@@ -1,3 +1,38 @@
+2016-10-29 Yusuke Suzuki <[email protected]>
+
+ [JSC] JSON.stringify should handle Proxy which is non JSArray but isArray is true
+ https://bugs.webkit.org/show_bug.cgi?id=164123
+
+ Reviewed by Mark Lam.
+
+ When JSON.stringify encounter the undefined value, the result depends
+ on the context. If it is produced under the object property context, we ignore
+ that property. On the other hand, if it is produced under the array element
+ context, we produce "null".
+
+ For example,
+ // https://tc39.github.io/ecma262/#sec-serializejsonobject section 8.b.
+ // Skip the property that value is undefined.
+ JSON.stringify({ value: undefined }); // => "{}"
+
+ // https://tc39.github.io/ecma262/#sec-serializejsonarray section 8.b.
+ // Write "null" when the element is undefined.
+ JSON.stringify([undefined]); // => "[null]"
+
+ At that time, we decide the context based on the `holder->inherits(JSArray::info())`.
+ But it is not correct since we have a holder that creates the array element context
+ but it is not JSArray subtype. ES6 Proxy to an array is one example. In that case,
+ `isArray(exec, proxy)` returns `true`, but `inherits(JSArray::info())` returns false.
+ Since we already have this `isArray()` value in Stringifier::Holder, we should reuse
+ this here. And this is the correct behavior in the ES6 spec.
+
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::Holder::isArray):
+ (JSC::Stringifier::stringify):
+ (JSC::Stringifier::appendStringifiedValue):
+ (JSC::Stringifier::Holder::Holder):
+ (JSC::Stringifier::Holder::appendNextProperty):
+
2016-10-29 Saam Barati <[email protected]>
We should have a way of profiling when a get_by_id is pure and to emit a PureGetById in the DFG/FTL
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (208122 => 208123)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-10-30 09:14:03 UTC (rev 208122)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-10-30 09:14:37 UTC (rev 208123)
@@ -93,9 +93,12 @@
private:
class Holder {
public:
+ enum RootHolderTag { RootHolder };
Holder(VM&, ExecState*, JSObject*);
+ Holder(RootHolderTag, VM&, JSObject*);
JSObject* object() const { return m_object.get(); }
+ bool isArray() const { return m_isArray; }
bool appendNextProperty(Stringifier&, StringBuilder&);
@@ -102,7 +105,7 @@
private:
Local<JSObject> m_object;
const bool m_isArray;
- bool m_isJSArray;
+ const bool m_isJSArray;
unsigned m_index;
unsigned m_size;
RefPtr<PropertyNameArrayData> m_propertyNames;
@@ -114,7 +117,7 @@
JSValue toJSONImpl(JSValue value, JSValue toJSONFunction, const PropertyNameForFunctionCall&);
enum StringifyResult { StringifyFailed, StringifySucceeded, StringifyFailedDueToUndefinedOrSymbolValue };
- StringifyResult appendStringifiedValue(StringBuilder&, JSValue, JSObject* holder, const PropertyNameForFunctionCall&);
+ StringifyResult appendStringifiedValue(StringBuilder&, JSValue, const Holder&, const PropertyNameForFunctionCall&);
bool willIndent() const;
void indent();
@@ -259,7 +262,8 @@
object->putDirect(vm, vm.propertyNames->emptyIdentifier, value.get());
StringBuilder result;
- if (appendStringifiedValue(result, value.get(), object, emptyPropertyName) != StringifySucceeded)
+ Holder root(Holder::RootHolder, vm, object);
+ if (appendStringifiedValue(result, value.get(), root, emptyPropertyName) != StringifySucceeded)
return Local<Unknown>(vm, jsUndefined());
RETURN_IF_EXCEPTION(scope, Local<Unknown>(vm, jsNull()));
@@ -296,7 +300,7 @@
return call(m_exec, asObject(toJSONFunction), callType, callData, value, args);
}
-Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder& builder, JSValue value, JSObject* holder, const PropertyNameForFunctionCall& propertyName)
+Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder& builder, JSValue value, const Holder& holder, const PropertyNameForFunctionCall& propertyName)
{
VM& vm = m_exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -310,11 +314,11 @@
MarkedArgumentBuffer args;
args.append(propertyName.value(m_exec));
args.append(value);
- value = call(m_exec, m_replacer.get(), m_replacerCallType, m_replacerCallData, holder, args);
+ value = call(m_exec, m_replacer.get(), m_replacerCallType, m_replacerCallData, holder.object(), args);
RETURN_IF_EXCEPTION(scope, StringifyFailed);
}
- if ((value.isUndefined() || value.isSymbol()) && !holder->inherits(JSArray::info()))
+ if ((value.isUndefined() || value.isSymbol()) && !holder.isArray())
return StringifyFailedDueToUndefinedOrSymbolValue;
if (value.isNull()) {
@@ -359,7 +363,7 @@
CallData callData;
if (object->methodTable()->getCallData(object, callData) != CallType::None) {
- if (holder->inherits(JSArray::info())) {
+ if (holder.isArray()) {
builder.appendLiteral("null");
return StringifySucceeded;
}
@@ -419,7 +423,8 @@
inline Stringifier::Holder::Holder(VM& vm, ExecState* exec, JSObject* object)
: m_object(vm, object)
- , m_isArray(isArray(exec, object))
+ , m_isArray(JSC::isArray(exec, object))
+ , m_isJSArray(m_isArray && isJSArray(object))
, m_index(0)
#ifndef NDEBUG
, m_size(0)
@@ -427,6 +432,17 @@
{
}
+inline Stringifier::Holder::Holder(RootHolderTag, VM& vm, JSObject* object)
+ : m_object(vm, object)
+ , m_isArray(false)
+ , m_isJSArray(false)
+ , m_index(0)
+#ifndef NDEBUG
+ , m_size(0)
+#endif
+{
+}
+
bool Stringifier::Holder::appendNextProperty(Stringifier& stringifier, StringBuilder& builder)
{
ASSERT(m_index <= m_size);
@@ -438,11 +454,12 @@
// First time through, initialize.
if (!m_index) {
if (m_isArray) {
- m_isJSArray = isJSArray(m_object.get());
if (m_isJSArray)
m_size = asArray(m_object.get())->length();
- else
+ else {
m_size = m_object->get(exec, vm.propertyNames->length).toUInt32(exec);
+ RETURN_IF_EXCEPTION(scope, false);
+ }
builder.append('[');
} else {
if (stringifier.m_usingArrayReplacer)
@@ -492,7 +509,8 @@
stringifier.startNewLine(builder);
// Append the stringified value.
- stringifyResult = stringifier.appendStringifiedValue(builder, value, m_object.get(), index);
+ stringifyResult = stringifier.appendStringifiedValue(builder, value, *this, index);
+ ASSERT(stringifyResult != StringifyFailedDueToUndefinedOrSymbolValue);
} else {
// Get the value.
PropertySlot slot(m_object.get(), PropertySlot::InternalMethodType::Get);
@@ -516,7 +534,7 @@
builder.append(' ');
// Append the stringified value.
- stringifyResult = stringifier.appendStringifiedValue(builder, value, m_object.get(), propertyName);
+ stringifyResult = stringifier.appendStringifiedValue(builder, value, *this, propertyName);
}
// From this point on, no access to the this pointer or to any members, because the
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
