Changeset 268709 in webkit

Timestamp:

Oct 19, 2020 7:25:16 PM (4 years ago)

Author:

Alexey Shvayka

Message:

[WebIDL] convertRecord() should handle duplicate keys for USVString records
​https://bugs.webkit.org/show_bug.cgi?id=217612

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

• web-platform-tests/url/urlsearchparams-constructor.any-expected.txt:
• web-platform-tests/url/urlsearchparams-constructor.any.js:
• web-platform-tests/url/urlsearchparams-constructor.any.worker-expected.txt:

Source/WebCore:

Before this patch, due to unpaired surrogates replacement in stringToUSVString(),
convertRecord() could append the same key multiple times, violating the spec [1].

This change adds duplicate handling while preserving common case performance,
and aligns WebKit with Blink and Gecko. Since a Proxy object can no longer return
duplicate keys [2], only USVString records needed to be fixed.

Test: imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.html

[1] ​https://heycam.github.io/webidl/#es-record (step 4.2.4 + example below)
[2] ​https://github.com/tc39/ecma262/pull/833

• bindings/js/JSDOMConvertRecord.h:

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.js (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.worker-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/bindings/js/JSDOMConvertRecord.h (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2020-10-19  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        [WebIDL] convertRecord() should handle duplicate keys for USVString records
+        https://bugs.webkit.org/show_bug.cgi?id=217612
+
+        Reviewed by Sam Weinig.
+
+        * web-platform-tests/url/urlsearchparams-constructor.any-expected.txt:
+        * web-platform-tests/url/urlsearchparams-constructor.any.js:
+        * web-platform-tests/url/urlsearchparams-constructor.any.worker-expected.txt:
+
 2020-10-19  Antti Koivisto  <antti@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any-expected.txt

@@ -23 +23 @@
 PASS Construct with object with two keys
 PASS Construct with array with two keys
+PASS Construct with 2 unpaired surrogates (no trailing)
+PASS Construct with 3 unpaired surrogates (no leading)
 PASS Construct with object with NULL, non-ASCII, and surrogate keys
 PASS Custom [Symbol.iterator]

trunk/LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.js

@@ -201 +201 @@
   { "input": {c: "x", a: "?"}, "output": [["c", "x"], ["a", "?"]], "name": "object with two keys" },
   { "input": [["c", "x"], ["a", "?"]], "output": [["c", "x"], ["a", "?"]], "name": "array with two keys" },
+  { "input": {"\uD835x": "1", "xx": "2", "\uD83Dx": "3"}, "output": [["\uFFFDx", "3"], ["xx", "2"]], "name": "2 unpaired surrogates (no trailing)" },
+  { "input": {"x\uDC53": "1", "x\uDC5C": "2", "x\uDC65": "3"}, "output": [["x\uFFFD", "3"]], "name": "3 unpaired surrogates (no leading)" },
   { "input": {"a\0b": "42", "c\uD83D": "23", "d\u1234": "foo"}, "output": [["a\0b", "42"], ["c\uFFFD", "23"], ["d\u1234", "foo"]], "name": "object with NULL, non-ASCII, and surrogate keys" }
 ].forEach((val) => {

trunk/LayoutTests/imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.worker-expected.txt

@@ -23 +23 @@
 PASS Construct with object with two keys
 PASS Construct with array with two keys
+PASS Construct with 2 unpaired surrogates (no trailing)
+PASS Construct with 3 unpaired surrogates (no leading)
 PASS Construct with object with NULL, non-ASCII, and surrogate keys
 PASS Custom [Symbol.iterator]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-19  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        [WebIDL] convertRecord() should handle duplicate keys for USVString records
+        https://bugs.webkit.org/show_bug.cgi?id=217612
+
+        Reviewed by Sam Weinig.
+
+        Before this patch, due to unpaired surrogates replacement in stringToUSVString(),
+        convertRecord() could append the same key multiple times, violating the spec [1].
+
+        This change adds duplicate handling while preserving common case performance,
+        and aligns WebKit with Blink and Gecko. Since a Proxy object can no longer return
+        duplicate keys [2], only USVString records needed to be fixed.
+
+        Test: imported/w3c/web-platform-tests/url/urlsearchparams-constructor.any.html
+
+        [1] https://heycam.github.io/webidl/#es-record (step 4.2.4 + example below)
+        [2] https://github.com/tc39/ecma262/pull/833
+
+        * bindings/js/JSDOMConvertRecord.h:
+
 2020-10-19  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h

@@ -128 +128 @@
                 RETURN_IF_EXCEPTION(scope, { });
                 
-                // 4. If typedKey is already a key in result, set its value to typedValue.
-                // Note: This can happen when O is a proxy object.
-                // FIXME: Handle this case.
+                // 4. Set result[typedKey] to typedValue.
+                // Note: It's possible that typedKey is already in result if K is USVString and key contains unpaired surrogates.
+                if constexpr (std::is_same_v<K, IDLUSVString>) {
+                    if (!typedKey.is8Bit()) {
+                        auto index = result.findMatching([&](auto& entry) { return entry.key == typedKey; });
+                        if (index != notFound) {
+                            result[index].value = typedValue;
+                            continue;
+                        }
+                    }
+                }
                 
-                // 5. Otherwise, append to result a mapping (typedKey, typedValue).
                 result.append({ typedKey, typedValue });
             }