[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] tc39/ecma262#833

* bindings/js/JSDOMConvertRecord.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@268709 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

LayoutTests/imported/w3c/ChangeLog

@@ -1,3 +1,14 @@
+2020-10-19  Alexey Shvayka  <[email protected]>
+
+        [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  <[email protected]>
 
         Update imported/w3c/web-platform-tests/css/selectors/

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

@@ -22,6 +22,8 @@ PASS Constructor with sequence of sequences of strings
 PASS Construct with object with +
 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]
 

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

@@ -200,6 +200,8 @@ test(function() {
   { "input": {"+": "%C2"}, "output": [["+", "%C2"]], "name": "object with +" },
   { "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) => {
     test(() => {

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

@@ -22,6 +22,8 @@ PASS Constructor with sequence of sequences of strings
 PASS Construct with object with +
 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]
 

Source/WebCore/ChangeLog

@@ -1,3 +1,24 @@
+2020-10-19  Alexey Shvayka  <[email protected]>
+
+        [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  <[email protected]>
 
         [MotionMark] Add state change items to represent changes to stroke and fill state

Source/WebCore/bindings/js/JSDOMConvertRecord.h

@@ -127,11 +127,18 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
                 auto typedValue = Converter<V>::convert(lexicalGlobalObject, subValue, args...);
                 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 });
             }
         }