querystring, url: handle repeated sep in search

* update state machine in parse * repeated sep should be adjusted * `&=&=` should be `{ '': [ '', '' ] }` * add test cases for querystring and URLSearchParams Fixes: #10454 PR-URL: #10967 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
nodejs · Feb 2, 2017 · 4e259b2 · 4e259b2
1 parent 0d9ea4f
commit 4e259b2
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/lib/querystring.js b/lib/querystring.js
@@ -292,6 +292,7 @@ function parse(qs, sep, eq, options) {
   const customDecode = (decode !== qsUnescape);
 
   const keys = [];
+  var posIdx = 0;
   var lastPos = 0;
   var sepIdx = 0;
   var eqIdx = 0;
@@ -319,26 +320,34 @@ function parse(qs, sep, eq, options) {
           key = decodeStr(key, decode);
         if (valEncoded)
           value = decodeStr(value, decode);
-        // Use a key array lookup instead of using hasOwnProperty(), which is
-        // slower
-        if (keys.indexOf(key) === -1) {
-          obj[key] = value;
-          keys[keys.length] = key;
-        } else {
-          const curValue = obj[key];
-          // A simple Array-specific property check is enough here to
-          // distinguish from a string value and is faster and still safe since
-          // we are generating all of the values being assigned.
-          if (curValue.pop)
-            curValue[curValue.length] = value;
-          else
-            obj[key] = [curValue, value];
+
+        if (key || value || lastPos - posIdx > sepLen || i === 0) {
+          // Use a key array lookup instead of using hasOwnProperty(), which is
+          // slower
+          if (keys.indexOf(key) === -1) {
+            obj[key] = value;
+            keys[keys.length] = key;
+          } else {
+            const curValue = obj[key] || '';
+            // A simple Array-specific property check is enough here to
+            // distinguish from a string value and is faster and still safe
+            // since we are generating all of the values being assigned.
+            if (curValue.pop)
+              curValue[curValue.length] = value;
+            else if (curValue)
+              obj[key] = [curValue, value];
+          }
+        } else if (i === 1) {
+          // A pair with repeated sep could be added into obj in the first loop
+          // and it should be deleted
+          delete obj[key];
         }
         if (--pairs === 0)
           break;
         keyEncoded = valEncoded = customDecode;
         encodeCheck = 0;
         key = value = '';
+        posIdx = lastPos;
         lastPos = i + 1;
         sepIdx = eqIdx = 0;
       }

diff --git a/test/parallel/test-querystring.js b/test/parallel/test-querystring.js
@@ -51,6 +51,14 @@ const qsTestCases = [
      __defineGetter__: 'baz' }],
   // See: https://github.com/joyent/node/issues/3058
   ['foo&bar=baz', 'foo=&bar=baz', { foo: '', bar: 'baz' }],
+  ['a=b&c&d=e', 'a=b&c=&d=e', { a: 'b', c: '', d: 'e' }],
+  ['a=b&c=&d=e', 'a=b&c=&d=e', { a: 'b', c: '', d: 'e' }],
+  ['a=b&=c&d=e', 'a=b&=c&d=e', { a: 'b', '': 'c', d: 'e' }],
+  ['a=b&=&c=d', 'a=b&=&c=d', { a: 'b', '': '', c: 'd' }],
+  ['&&foo=bar&&', 'foo=bar', { foo: 'bar' }],
+  ['&&&&', '', {}],
+  ['&=&', '=', { '': '' }],
+  ['&=&=', '=&=', { '': [ '', '' ]}],
   [null, '', {}],
   [undefined, '', {}]
 ];