Changeset 275544 in webkit

Timestamp:

Apr 6, 2021 1:20:41 PM (3 years ago)

Author:

Alexey Shvayka

Message:

Array's toString() is incorrect if join() is non-callable
​https://bugs.webkit.org/show_bug.cgi?id=224215

Reviewed by Yusuke Suzuki.

JSTests:

• stress/array-toString-non-callable-join.js: Added.

Source/JavaScriptCore:

This patch exposes objectPrototypeToString() to be used by Array.prototype.toString
if "join" lookup doesn't return a callable value [1].

Fixes Array's toString() to return the correct tag instead of internal className,
perform Symbol.toStringTag lookup, and throw for revoked Proxy objects.
Aligns JSC with V8 and SpiderMonkey.

Also, a few objectPrototypeToString() tweaks: a bit nicer undefined / null
checks and simpler toObject() exception handling.

[1]: ​https://tc39.es/ecma262/#sec-array.prototype.tostring (step 3)

• runtime/ArrayPrototype.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

• runtime/ObjectPrototype.cpp:

(JSC::objectPrototypeToString):
(JSC::JSC_DEFINE_HOST_FUNCTION):

• runtime/ObjectPrototype.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/array-toString-non-callable-join.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ArrayPrototype.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/ObjectPrototype.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/ObjectPrototype.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-04-06  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Array's toString() is incorrect if join() is non-callable
+        https://bugs.webkit.org/show_bug.cgi?id=224215
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/array-toString-non-callable-join.js: Added.
+
 2021-04-05  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-04-06  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Array's toString() is incorrect if join() is non-callable
+        https://bugs.webkit.org/show_bug.cgi?id=224215
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch exposes objectPrototypeToString() to be used by Array.prototype.toString
+        if "join" lookup doesn't return a callable value [1].
+
+        Fixes Array's toString() to return the correct tag instead of internal `className`,
+        perform Symbol.toStringTag lookup, and throw for revoked Proxy objects.
+        Aligns JSC with V8 and SpiderMonkey.
+
+        Also, a few objectPrototypeToString() tweaks: a bit nicer `undefined` / `null`
+        checks and simpler toObject() exception handling.
+
+        [1]: https://tc39.es/ecma262/#sec-array.prototype.tostring (step 3)
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        * runtime/ObjectPrototype.cpp:
+        (JSC::objectPrototypeToString):
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        * runtime/ObjectPrototype.h:
+
 2021-04-06  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -35 +35 @@
 #include "JSStringJoiner.h"
 #include "ObjectConstructor.h"
+#include "ObjectPrototype.h"
 #include "StringRecursionChecker.h"
 #include <algorithm>
@@ -613 +614 @@
     if (!canUseDefaultArrayJoinForToString(vm, thisObject)) {
         // 2. Let func be the result of calling the [[Get]] internal method of array with argument "join".
-        JSValue function = JSValue(thisObject).get(globalObject, vm.propertyNames->join);
+        JSValue function = thisObject->get(globalObject, vm.propertyNames->join);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
         // 3. If IsCallable(func) is false, then let func be the standard built-in method Object.prototype.toString (15.2.4.2).
-        bool customJoinCase = false;
-        if (!function.isCell())
-            customJoinCase = true;
         auto callData = getCallData(vm, function);
-        if (callData.type == CallData::Type::None)
-            customJoinCase = true;
-
-        if (UNLIKELY(customJoinCase))
-            RELEASE_AND_RETURN(scope, JSValue::encode(jsMakeNontrivialString(globalObject, "[object ", thisObject->methodTable(vm)->className(thisObject, vm), "]")));
+        if (UNLIKELY(callData.type == CallData::Type::None))
+            RELEASE_AND_RETURN(scope, JSValue::encode(objectPrototypeToString(globalObject, thisObject)));
 
         // 4. Return the result of calling the [[Call]] internal method of func providing array as the this value and an empty arguments list.

trunk/Source/JavaScriptCore/runtime/ObjectPrototype.cpp

@@ -312 +312 @@
 }
 
-JSC_DEFINE_HOST_FUNCTION(objectProtoFuncToString, (JSGlobalObject* globalObject, CallFrame* callFrame))
-{
-    VM& vm = globalObject->vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    JSValue thisValue = callFrame->thisValue().toThis(globalObject, ECMAMode::strict());
-    if (thisValue.isUndefinedOrNull())
-        return JSValue::encode(thisValue.isUndefined() ? vm.smallStrings.undefinedObjectString() : vm.smallStrings.nullObjectString());
+JSString* objectPrototypeToString(JSGlobalObject* globalObject, JSValue thisValue)
+{
+    VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    if (thisValue.isUndefined())
+        return vm.smallStrings.undefinedObjectString();
+    if (thisValue.isNull())
+        return vm.smallStrings.nullObjectString();
     JSObject* thisObject = thisValue.toObject(globalObject);
-    EXCEPTION_ASSERT(!!scope.exception() == !thisObject);
-    if (!thisObject)
-        return JSValue::encode(jsUndefined());
+    scope.assertNoException();
 
     Integrity::auditStructureID(vm, thisObject->structureID());
     auto result = thisObject->structure(vm)->cachedSpecialProperty(CachedSpecialPropertyKey::ToStringTag);
     if (result)
-        return JSValue::encode(result);
+        return asString(result);
 
     String tag = thisObject->methodTable(vm)->toStringName(thisObject, globalObject);
-    RETURN_IF_EXCEPTION(scope, { });
+    RETURN_IF_EXCEPTION(scope, nullptr);
     JSString* jsTag = nullptr;
 
@@ -339 +338 @@
     if (hasProperty) {
         JSValue tagValue = slot.getValue(globalObject, vm.propertyNames->toStringTagSymbol);
-        RETURN_IF_EXCEPTION(scope, { });
+        RETURN_IF_EXCEPTION(scope, nullptr);
         if (tagValue.isString())
             jsTag = asString(tagValue);
@@ -350 +349 @@
 
     JSString* jsResult = jsString(globalObject, vm.smallStrings.objectStringStart(), jsTag, vm.smallStrings.singleCharacterString(']'));
-    RETURN_IF_EXCEPTION(scope, { });
+    RETURN_IF_EXCEPTION(scope, nullptr);
     thisObject->structure(vm)->cacheSpecialProperty(globalObject, vm, jsResult, CachedSpecialPropertyKey::ToStringTag, slot);
-    return JSValue::encode(jsResult);
+    return jsResult;
+}
+
+JSC_DEFINE_HOST_FUNCTION(objectProtoFuncToString, (JSGlobalObject* globalObject, CallFrame* callFrame))
+{
+    JSValue thisValue = callFrame->thisValue().toThis(globalObject, ECMAMode::strict());
+    return JSValue::encode(objectPrototypeToString(globalObject, thisValue));
 }
 

trunk/Source/JavaScriptCore/runtime/ObjectPrototype.h

@@ -52 +52 @@
 
 JS_EXPORT_PRIVATE JSC_DECLARE_HOST_FUNCTION(objectProtoFuncToString);
+JSString* objectPrototypeToString(JSGlobalObject*, JSValue thisValue);
 bool objectPrototypeHasOwnProperty(JSGlobalObject*, JSValue base, const Identifier& property);