From 315c3aaf43f28236de6afb6f1a81d50479709fdb Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sun, 14 May 2017 11:53:55 -0700 Subject: [PATCH] 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 Reviewed-By: Joyee Cheung Reviewed-By: James M Snell --- lib/internal/url.js | 29 +++++---- ...est-whatwg-url-searchparams-constructor.js | 59 ++++++++++++------- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index bb33f75b090109..c7a359f89bb932 100644 --- a/lib/internal/url.js +++ b/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]); - const value = toUSVString(pair[1]); - this[searchParams].push(key, value); + this[searchParams].push(pair[0], pair[1]); } } else { // record + // Need to use reflection APIs for full spec compliance. this[searchParams] = []; - for (var key of Object.keys(init)) { - key = toUSVString(key); - const value = toUSVString(init[key]); - this[searchParams].push(key, value); + const keys = Reflect.ownKeys(init); + for (var i = 0; i < keys.length; i++) { + const key = keys[i]; + 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 { diff --git a/test/parallel/test-whatwg-url-searchparams-constructor.js b/test/parallel/test-whatwg-url-searchparams-constructor.js index 643ba3c5f40ffb..03ea62462165df 100644 --- a/test/parallel/test-whatwg-url-searchparams-constructor.js +++ b/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. -{ -// assert.throws(() => { -// new URLSearchParams({ -// toString() { throw new TypeError('Illegal invocation'); } -// }); -// }, TypeError); +function makeIterableFunc(array) { + return Object.assign(() => {}, { + [Symbol.iterator]() { + return array[Symbol.iterator](); + } + }); } { @@ -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(); - - assert.throws(() => new URLSearchParams({ a: obj }), /^Error: toString$/); - assert.throws(() => new URLSearchParams([['a', obj]]), /^Error: toString$/); - assert.throws(() => new URLSearchParams(sym), - /^TypeError: Cannot convert a Symbol value to a string$/); - assert.throws(() => new URLSearchParams({ a: sym }), - /^TypeError: Cannot convert a Symbol value to a string$/); - assert.throws(() => new URLSearchParams([[sym, 'a']]), - /^TypeError: Cannot convert a Symbol value to a string$/); - assert.throws(() => new URLSearchParams([['a', sym]]), - /^TypeError: Cannot convert a Symbol value to a string$/); + const toStringError = /^Error: toString$/; + const symbolError = /^TypeError: Cannot convert a Symbol value to a string$/; + + assert.throws(() => new URLSearchParams({ a: obj }), toStringError); + assert.throws(() => new URLSearchParams([['a', obj]]), toStringError); + assert.throws(() => new URLSearchParams(sym), symbolError); + assert.throws(() => new URLSearchParams({ [sym]: 'a' }), symbolError); + assert.throws(() => new URLSearchParams({ a: sym }), symbolError); + assert.throws(() => new URLSearchParams([[sym, 'a']]), symbolError); + assert.throws(() => new URLSearchParams([['a', sym]]), symbolError); }