url: spec-compliant URLSearchParams serializer

PR-URL: #11626 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]>
nodejs · Mar 14, 2017 · d77a758 · d77a758
1 parent 92bcc13
commit d77a758
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 30 deletions.
diff --git a/benchmark/url/legacy-vs-whatwg-url-searchparams-serialize.js b/benchmark/url/legacy-vs-whatwg-url-searchparams-serialize.js
@@ -7,7 +7,7 @@ const inputs = require('../fixtures/url-inputs.js').searchParams;
 const bench = common.createBenchmark(main, {
   type: Object.keys(inputs),
   method: ['legacy', 'whatwg'],
-  n: [1e5]
+  n: [1e6]
 });
 
 function useLegacy(n, input, prop) {

diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1,7 +1,7 @@
 'use strict';
 
 const util = require('util');
-const { StorageObject } = require('internal/querystring');
+const { hexTable, StorageObject } = require('internal/querystring');
 const binding = process.binding('url');
 const context = Symbol('context');
 const cannotBeBase = Symbol('cannot-be-base');
@@ -597,18 +597,99 @@ function getParamsFromObject(obj) {
   return values;
 }
 
-function getObjectFromParams(array) {
-  const obj = new StorageObject();
-  for (var i = 0; i < array.length; i += 2) {
-    const name = array[i];
-    const value = array[i + 1];
-    if (obj[name]) {
-      obj[name].push(value);
-    } else {
-      obj[name] = [value];
+// Adapted from querystring's implementation.
+// Ref: https://url.spec.whatwg.org/#concept-urlencoded-byte-serializer
+const noEscape = [
+//0, 1, 2, 3, 4, 5, 6, 7, 8, 9, A, B, C, D, E, F
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x00 - 0x0F
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0x10 - 0x1F
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 1, 0, // 0x20 - 0x2F
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 0x30 - 0x3F
+  0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 0x40 - 0x4F
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1, // 0x50 - 0x5F
+  0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 0x60 - 0x6F
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0  // 0x70 - 0x7F
+];
+
+// Special version of hexTable that uses `+` for U+0020 SPACE.
+const paramHexTable = hexTable.slice();
+paramHexTable[0x20] = '+';
+
+function escapeParam(str) {
+  const len = str.length;
+  if (len === 0)
+    return '';
+
+  var out = '';
+  var lastPos = 0;
+
+  for (var i = 0; i < len; i++) {
+    var c = str.charCodeAt(i);
+
+    // ASCII
+    if (c < 0x80) {
+      if (noEscape[c] === 1)
+        continue;
+      if (lastPos < i)
+        out += str.slice(lastPos, i);
+      lastPos = i + 1;
+      out += paramHexTable[c];
+      continue;
+    }
+
+    if (lastPos < i)
+      out += str.slice(lastPos, i);
+
+    // Multi-byte characters ...
+    if (c < 0x800) {
+      lastPos = i + 1;
+      out += paramHexTable[0xC0 | (c >> 6)] +
+             paramHexTable[0x80 | (c & 0x3F)];
+      continue;
+    }
+    if (c < 0xD800 || c >= 0xE000) {
+      lastPos = i + 1;
+      out += paramHexTable[0xE0 | (c >> 12)] +
+             paramHexTable[0x80 | ((c >> 6) & 0x3F)] +
+             paramHexTable[0x80 | (c & 0x3F)];
+      continue;
     }
+    // Surrogate pair
+    ++i;
+    var c2;
+    if (i < len)
+      c2 = str.charCodeAt(i) & 0x3FF;
+    else {
+      // This branch should never happen because all URLSearchParams entries
+      // should already be converted to USVString. But, included for
+      // completion's sake anyway.
+      c2 = 0;
+    }
+    lastPos = i + 1;
+    c = 0x10000 + (((c & 0x3FF) << 10) | c2);
+    out += paramHexTable[0xF0 | (c >> 18)] +
+           paramHexTable[0x80 | ((c >> 12) & 0x3F)] +
+           paramHexTable[0x80 | ((c >> 6) & 0x3F)] +
+           paramHexTable[0x80 | (c & 0x3F)];
   }
-  return obj;
+  if (lastPos === 0)
+    return str;
+  if (lastPos < len)
+    return out + str.slice(lastPos);
+  return out;
+}
+
+// application/x-www-form-urlencoded serializer
+// Ref: https://url.spec.whatwg.org/#concept-urlencoded-serializer
+function serializeParams(array) {
+  const len = array.length;
+  if (len === 0)
+    return '';
+
+  var output = `${escapeParam(array[0])}=${escapeParam(array[1])}`;
+  for (var i = 2; i < len; i += 2)
+    output += `&${escapeParam(array[i])}=${escapeParam(array[i + 1])}`;
+  return output;
 }
 
 // Mainly to mitigate func-name-matching ESLint rule
@@ -993,7 +1074,7 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
       throw new TypeError('Value of `this` is not a URLSearchParams');
     }
 
-    return querystring.stringify(getObjectFromParams(this[searchParams]));
+    return serializeParams(this[searchParams]);
   }
 });
 

diff --git a/test/fixtures/url-tests.js b/test/fixtures/url-tests.js
@@ -4639,7 +4639,7 @@ module.exports =
     "port": "",
     "pathname": "/foo/bar",
     "search": "??a=b&c=d",
-    // "searchParams": "%3Fa=b&c=d",
+    "searchParams": "%3Fa=b&c=d",
     "hash": ""
   },
   "# Scheme only",

diff --git a/test/parallel/test-whatwg-url-constructor.js b/test/parallel/test-whatwg-url-constructor.js
@@ -120,12 +120,12 @@ function runURLSearchParamTests() {
     // And in the other direction, altering searchParams propagates
     // back to 'search'.
     searchParams.append('i', ' j ')
-    // assert_equals(url.search, '?e=f&g=h&i=+j+')
-    // assert_equals(url.searchParams.toString(), 'e=f&g=h&i=+j+')
+    assert_equals(url.search, '?e=f&g=h&i=+j+')
+    assert_equals(url.searchParams.toString(), 'e=f&g=h&i=+j+')
     assert_equals(searchParams.get('i'), ' j ')
 
     searchParams.set('e', 'updated')
-    // assert_equals(url.search, '?e=updated&g=h&i=+j+')
+    assert_equals(url.search, '?e=updated&g=h&i=+j+')
     assert_equals(searchParams.get('e'), 'updated')
 
     var url2 = bURL('http://example.org/file??a=b&c=d')

diff --git 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/405394a/url/urlsearchparams-constructor.html
+   https://github.com/w3c/web-platform-tests/blob/e94c604916/url/urlsearchparams-constructor.html
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
 test(function() {
@@ -154,7 +154,7 @@ test(function() {
 }, "Constructor with sequence of sequences of strings");
 
 [
-//   { "input": {"+": "%C2"}, "output": [[" ", "\uFFFD"]], "name": "object with +" },
+  { "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" }
 ].forEach((val) => {

diff --git a/test/parallel/test-whatwg-url-searchparams-stringifier.js b/test/parallel/test-whatwg-url-searchparams-stringifier.js
@@ -10,14 +10,14 @@ const { test, assert_equals } = common.WPT;
    https://github.com/w3c/web-platform-tests/blob/8791bed/url/urlsearchparams-stringifier.html
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
-// test(function() {
-//     var params = new URLSearchParams();
-//     params.append('a', 'b c');
-//     assert_equals(params + '', 'a=b+c');
-//     params.delete('a');
-//     params.append('a b', 'c');
-//     assert_equals(params + '', 'a+b=c');
-// }, 'Serialize space');
+test(function() {
+    var params = new URLSearchParams();
+    params.append('a', 'b c');
+    assert_equals(params + '', 'a=b+c');
+    params.delete('a');
+    params.append('a b', 'c');
+    assert_equals(params + '', 'a+b=c');
+}, 'Serialize space');
 
 test(function() {
     var params = new URLSearchParams();
@@ -114,8 +114,8 @@ test(function() {
     var params;
     params = new URLSearchParams('a=b&c=d&&e&&');
     assert_equals(params.toString(), 'a=b&c=d&e=');
-    // params = new URLSearchParams('a = b &a=b&c=d%20');
-    // assert_equals(params.toString(), 'a+=+b+&a=b&c=d+');
+    params = new URLSearchParams('a = b &a=b&c=d%20');
+    assert_equals(params.toString(), 'a+=+b+&a=b&c=d+');
     // The lone '=' _does_ survive the roundtrip.
     params = new URLSearchParams('a=&a=b');
     assert_equals(params.toString(), 'a=&a=b');

diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js
@@ -7,7 +7,7 @@ const URL = require('url').URL;
 // Tests below are not from WPT.
 const serialized = 'a=a&a=1&a=true&a=undefined&a=null&a=%EF%BF%BD' +
                    '&a=%EF%BF%BD&a=%F0%9F%98%80&a=%EF%BF%BD%EF%BF%BD' +
-                   '&a=%5Bobject%20Object%5D';
+                   '&a=%5Bobject+Object%5D';
 const values = ['a', 1, true, undefined, null, '\uD83D', '\uDE00',
                 '\uD83D\uDE00', '\uDE00\uD83D', {}];
 const normalizedValues = ['a', '1', 'true', 'undefined', 'null', '\uFFFD',