Changeset 208123 in webkit


Ignore:
Timestamp:
Oct 30, 2016 2:14:37 AM (7 years ago)
Author:
Yusuke Suzuki
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/JavaScriptCore:

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):

Location:
trunk
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r208117 r208123  
     12016-10-29  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [JSC] JSON.stringify should handle Proxy which is non JSArray but isArray is true
     4        https://bugs.webkit.org/show_bug.cgi?id=164123
     5
     6        Reviewed by Mark Lam.
     7
     8        * stress/json-stringify-with-non-jsarray-array.js: Added.
     9        (shouldBe):
     10        (shouldBe.JSON.stringify.new.Proxy):
     11
    1122016-10-29  Saam Barati  <sbarati@apple.com>
    213

  • trunk/Source/JavaScriptCore/ChangeLog

    r208117 r208123  
     12016-10-29  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [JSC] JSON.stringify should handle Proxy which is non JSArray but isArray is true
     4        https://bugs.webkit.org/show_bug.cgi?id=164123
     5
     6        Reviewed by Mark Lam.
     7
     8        When JSON.stringify encounter the undefined value, the result depends
     9        on the context. If it is produced under the object property context, we ignore
     10        that property. On the other hand, if it is produced under the array element
     11        context, we produce "null".
     12
     13        For example,
     14            // https://tc39.github.io/ecma262/#sec-serializejsonobject section 8.b.
     15            // Skip the property that value is undefined.
     16            JSON.stringify({ value: undefined });  // => "{}"
     17
     18            // https://tc39.github.io/ecma262/#sec-serializejsonarray section 8.b.
     19            // Write "null" when the element is undefined.
     20            JSON.stringify([undefined]);           // => "[null]"
     21
     22        At that time, we decide the context based on the `holder->inherits(JSArray::info())`.
     23        But it is not correct since we have a holder that creates the array element context
     24        but it is not JSArray subtype. ES6 Proxy to an array is one example. In that case,
     25        `isArray(exec, proxy)` returns `true`, but `inherits(JSArray::info())` returns false.
     26        Since we already have this `isArray()` value in Stringifier::Holder, we should reuse
     27        this here. And this is the correct behavior in the ES6 spec.
     28
     29        * runtime/JSONObject.cpp:
     30        (JSC::Stringifier::Holder::isArray):
     31        (JSC::Stringifier::stringify):
     32        (JSC::Stringifier::appendStringifiedValue):
     33        (JSC::Stringifier::Holder::Holder):
     34        (JSC::Stringifier::Holder::appendNextProperty):
     35
    1362016-10-29  Saam Barati  <sbarati@apple.com>
    237

  • trunk/Source/JavaScriptCore/runtime/JSONObject.cpp

    r207785 r208123  
    9494    class Holder {
    9595    public:
     96        enum RootHolderTag { RootHolder };
    9697        Holder(VM&, ExecState*, JSObject*);
     98        Holder(RootHolderTag, VM&, JSObject*);
    9799
    98100        JSObject* object() const { return m_object.get(); }
     101        bool isArray() const { return m_isArray; }
    99102
    100103        bool appendNextProperty(Stringifier&, StringBuilder&);
    … …  
    103106        Local<JSObject> m_object;
    104107        const bool m_isArray;
    105         bool m_isJSArray;
     108        const bool m_isJSArray;
    106109        unsigned m_index;
    107110        unsigned m_size;
    … …  
    115118
    116119    enum StringifyResult { StringifyFailed, StringifySucceeded, StringifyFailedDueToUndefinedOrSymbolValue };
    117     StringifyResult appendStringifiedValue(StringBuilder&, JSValue, JSObject* holder, const PropertyNameForFunctionCall&);
     120    StringifyResult appendStringifiedValue(StringBuilder&, JSValue, const Holder&, const PropertyNameForFunctionCall&);
    118121
    119122    bool willIndent() const;
    … …  
    260263
    261264    StringBuilder result;
    262     if (appendStringifiedValue(result, value.get(), object, emptyPropertyName) != StringifySucceeded)
     265    Holder root(Holder::RootHolder, vm, object);
     266    if (appendStringifiedValue(result, value.get(), root, emptyPropertyName) != StringifySucceeded)
    263267        return Local<Unknown>(vm, jsUndefined());
    264268    RETURN_IF_EXCEPTION(scope, Local<Unknown>(vm, jsNull()));
    … …  
    297301}
    298302
    299 Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder& builder, JSValue value, JSObject* holder, const PropertyNameForFunctionCall& propertyName)
     303Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder& builder, JSValue value, const Holder& holder, const PropertyNameForFunctionCall& propertyName)
    300304{
    301305    VM& vm = m_exec->vm();
    … …  
    311315        args.append(propertyName.value(m_exec));
    312316        args.append(value);
    313         value = call(m_exec, m_replacer.get(), m_replacerCallType, m_replacerCallData, holder, args);
     317        value = call(m_exec, m_replacer.get(), m_replacerCallType, m_replacerCallData, holder.object(), args);
    314318        RETURN_IF_EXCEPTION(scope, StringifyFailed);
    315319    }
    316320
    317     if ((value.isUndefined() || value.isSymbol()) && !holder->inherits(JSArray::info()))
     321    if ((value.isUndefined() || value.isSymbol()) && !holder.isArray())
    318322        return StringifyFailedDueToUndefinedOrSymbolValue;
    319323
    … …  
    360364    CallData callData;
    361365    if (object->methodTable()->getCallData(object, callData) != CallType::None) {
    362         if (holder->inherits(JSArray::info())) {
     366        if (holder.isArray()) {
    363367            builder.appendLiteral("null");
    364368            return StringifySucceeded;
    … …  
    420424inline Stringifier::Holder::Holder(VM& vm, ExecState* exec, JSObject* object)
    421425    : m_object(vm, object)
    422     , m_isArray(isArray(exec, object))
     426    , m_isArray(JSC::isArray(exec, object))
     427    , m_isJSArray(m_isArray && isJSArray(object))
    423428    , m_index(0)
    424429#ifndef NDEBUG
    … …  
    428433}
    429434
     435inline Stringifier::Holder::Holder(RootHolderTag, VM& vm, JSObject* object)
     436    : m_object(vm, object)
     437    , m_isArray(false)
     438    , m_isJSArray(false)
     439    , m_index(0)
     440#ifndef NDEBUG
     441    , m_size(0)
     442#endif
     443{
     444}
     445
    430446bool Stringifier::Holder::appendNextProperty(Stringifier& stringifier, StringBuilder& builder)
    431447{
    … …  
    439455    if (!m_index) {
    440456        if (m_isArray) {
    441             m_isJSArray = isJSArray(m_object.get());
    442457            if (m_isJSArray)
    443458                m_size = asArray(m_object.get())->length();
    444             else
     459            else {
    445460                m_size = m_object->get(exec, vm.propertyNames->length).toUInt32(exec);
     461                RETURN_IF_EXCEPTION(scope, false);
     462            }
    446463            builder.append('[');
    447464        } else {
    … …  
    493510
    494511        // Append the stringified value.
    495         stringifyResult = stringifier.appendStringifiedValue(builder, value, m_object.get(), index);
     512        stringifyResult = stringifier.appendStringifiedValue(builder, value, *this, index);
     513        ASSERT(stringifyResult != StringifyFailedDueToUndefinedOrSymbolValue);
    496514    } else {
    497515        // Get the value.
    … …  
    517535
    518536        // Append the stringified value.
    519         stringifyResult = stringifier.appendStringifiedValue(builder, value, m_object.get(), propertyName);
     537        stringifyResult = stringifier.appendStringifiedValue(builder, value, *this, propertyName);
    520538    }
    521539
Note: See TracChangeset for help on using the changeset viewer.