url: more precise URLSearchParams constructor

PR-URL: https://github.com/nodejs/node/pull/13026
Ref: https://github.com/w3c/web-platform-tests/pull/5813
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/url.js

@@ -814,7 +814,8 @@ class URLSearchParams {
   constructor(init = undefined) {
     if (init === null || init === undefined) {
       this[searchParams] = [];
-    } else if (typeof init === 'object') {
+    } else if ((typeof init === 'object' && init !== null) ||
+               typeof init === 'function') {
       const method = init[Symbol.iterator];
       if (method === this[Symbol.iterator]) {
         // While the spec does not have this branch, we can use it as a
@@ -830,12 +831,16 @@ class URLSearchParams {
         // Note: per spec we have to first exhaust the lists then process them
         const pairs = [];
         for (const pair of init) {
-          if (typeof pair !== 'object' ||
+          if ((typeof pair !== 'object' && typeof pair !== 'function') ||
+              pair === null ||
               typeof pair[Symbol.iterator] !== 'function') {
             throw new errors.TypeError('ERR_INVALID_TUPLE', 'Each query pair',
                                        '[name, value]');
           }
-          pairs.push(Array.from(pair));
+          const convertedPair = [];
+          for (const element of pair)
+            convertedPair.push(toUSVString(element));
+          pairs.push(convertedPair);
         }
 
         this[searchParams] = [];
@@ -844,17 +849,21 @@ class URLSearchParams {
             throw new errors.TypeError('ERR_INVALID_TUPLE', 'Each query pair',
                                        '[name, value]');
           }
-          const key = toUSVString(pair[0]);
+          this[searchParams].push(pair[0], pair[1]);
-          const value = toUSVString(pair[1]);
-          this[searchParams].push(key, value);
         }
       } else {
         // record<USVString, USVString>
+        // Need to use reflection APIs for full spec compliance.
         this[searchParams] = [];
-        for (var key of Object.keys(init)) {
+        const keys = Reflect.ownKeys(init);
-          key = toUSVString(key);
+        for (var i = 0; i < keys.length; i++) {
-          const value = toUSVString(init[key]);
+          const key = keys[i];
-          this[searchParams].push(key, value);
+          const desc = Reflect.getOwnPropertyDescriptor(init, key);
+          if (desc !== undefined && desc.enumerable) {
+            const typedKey = toUSVString(key);
+            const typedValue = toUSVString(init[key]);
+            this[searchParams].push(typedKey, typedValue);
+          }
         }
       }
     } else {

test/parallel/test-whatwg-url-searchparams-constructor.js

@@ -11,7 +11,7 @@ const {
 /* eslint-disable */
 var params;  // Strict mode fix for WPT.
 /* WPT Refs:
-   https://github.com/w3c/web-platform-tests/blob/e94c604916/url/urlsearchparams-constructor.html
+   https://github.com/w3c/web-platform-tests/blob/54c3502d7b/url/urlsearchparams-constructor.html
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
 test(function() {
@@ -87,6 +87,17 @@ test(function() {
     assert_equals(params.get('a b'), 'c');
 }, 'Parse +');
 
+test(function() {
+    const testValue = '+15555555555';
+    const params = new URLSearchParams();
+    params.set('query', testValue);
+    var newParams = new URLSearchParams(params.toString());
+
+    assert_equals(params.toString(), 'query=%2B15555555555');
+    assert_equals(params.get('query'), testValue);
+    assert_equals(newParams.get('query'), testValue);
+}, 'Parse encoded +');
+
 test(function() {
     var params = new URLSearchParams('a=b c');
     assert_equals(params.get('a'), 'b c');
@@ -156,7 +167,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": [["c", "x"], ["a", "?"]], "output": [["c", "x"], ["a", "?"]], "name": "array with two keys" },
+  { "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(() => {
         let params = new URLSearchParams(val.input),
@@ -179,12 +191,12 @@ test(() => {
 /* eslint-enable */
 
 // Tests below are not from WPT.
-{
+function makeIterableFunc(array) {
-//   assert.throws(() => {
+  return Object.assign(() => {}, {
-//     new URLSearchParams({
+    [Symbol.iterator]() {
-//         toString() { throw new TypeError('Illegal invocation'); }
+      return array[Symbol.iterator]();
-//     });
+    }
-//   }, TypeError);
+  });
 }
 
 {
@@ -200,17 +212,25 @@ test(() => {
   });
 
   let params;
-  // URLSearchParams constructor, undefined and null as argument
   params = new URLSearchParams(undefined);
   assert.strictEqual(params.toString(), '');
   params = new URLSearchParams(null);
   assert.strictEqual(params.toString(), '');
+  params = new URLSearchParams(
+    makeIterableFunc([['key', 'val'], ['key2', 'val2']])
+  );
+  assert.strictEqual(params.toString(), 'key=val&key2=val2');
+  params = new URLSearchParams(
+    makeIterableFunc([['key', 'val'], ['key2', 'val2']].map(makeIterableFunc))
+  );
+  assert.strictEqual(params.toString(), 'key=val&key2=val2');
   assert.throws(() => new URLSearchParams([[1]]), tupleError);
   assert.throws(() => new URLSearchParams([[1, 2, 3]]), tupleError);
   assert.throws(() => new URLSearchParams({ [Symbol.iterator]: 42 }),
                 iterableError);
   assert.throws(() => new URLSearchParams([{}]), tupleError);
   assert.throws(() => new URLSearchParams(['a']), tupleError);
+  assert.throws(() => new URLSearchParams([null]), tupleError);
   assert.throws(() => new URLSearchParams([{ [Symbol.iterator]: 42 }]),
                 tupleError);
 }
@@ -221,15 +241,14 @@ test(() => {
     valueOf() { throw new Error('valueOf'); }
   };
   const sym = Symbol();
+  const toStringError = /^Error: toString$/;
+  const symbolError = /^TypeError: Cannot convert a Symbol value to a string$/;
 
-  assert.throws(() => new URLSearchParams({ a: obj }), /^Error: toString$/);
+  assert.throws(() => new URLSearchParams({ a: obj }), toStringError);
-  assert.throws(() => new URLSearchParams([['a', obj]]), /^Error: toString$/);
+  assert.throws(() => new URLSearchParams([['a', obj]]), toStringError);
-  assert.throws(() => new URLSearchParams(sym),
+  assert.throws(() => new URLSearchParams(sym), symbolError);
-                /^TypeError: Cannot convert a Symbol value to a string$/);
+  assert.throws(() => new URLSearchParams({ [sym]: 'a' }), symbolError);
-  assert.throws(() => new URLSearchParams({ a: sym }),
+  assert.throws(() => new URLSearchParams({ a: sym }), symbolError);
-                /^TypeError: Cannot convert a Symbol value to a string$/);
+  assert.throws(() => new URLSearchParams([[sym, 'a']]), symbolError);
-  assert.throws(() => new URLSearchParams([[sym, 'a']]),
+  assert.throws(() => new URLSearchParams([['a', sym]]), symbolError);
-                /^TypeError: Cannot convert a Symbol value to a string$/);
-  assert.throws(() => new URLSearchParams([['a', sym]]),
-                /^TypeError: Cannot convert a Symbol value to a string$/);
 }