[Fix] proper comma parsing of URL-encoded commas (#361)

Fixes #311. Followup to #336.
ljharb · Mar 25, 2020 · 0ece6d8 · 0ece6d8
1 parent 0c358d3
commit 0ece6d8
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 18 deletions.
diff --git a/lib/parse.js b/lib/parse.js
@@ -37,6 +37,17 @@ var parseArrayValue = function (val, options) {
     return val;
 };
 
+var maybeMap = function maybeMap(val, fn) {
+    if (isArray(val)) {
+        var mapped = [];
+        for (var i = 0; i < val.length; i += 1) {
+            mapped.push(fn(val[i]));
+        }
+        return mapped;
+    }
+    return fn(val);
+};
+
 // This is what browsers will submit when the ✓ character occurs in an
 // application/x-www-form-urlencoded body and the encoding of the page containing
 // the form is iso-8859-1, or when the submitted form has an accept-charset
@@ -85,23 +96,18 @@ var parseValues = function parseQueryStringValues(str, options) {
             val = options.strictNullHandling ? null : '';
         } else {
             key = options.decoder(part.slice(0, pos), defaults.decoder, charset);
-            var encodedVal = part.slice(pos + 1);
-            if (options.comma && encodedVal.indexOf(',') !== -1) {
-                val = encodedVal.split(',')
-                    .map(function (encodedFragment) {
-                        return options.decoder(encodedFragment, defaults.decoder, charset);
-                    });
-            } else {
-                val = options.decoder(encodedVal, defaults.decoder, charset);
-            }
+            val = maybeMap(
+                parseArrayValue(part.slice(pos + 1), options),
+                function (encodedVal) {
+                    return options.decoder(encodedVal, defaults.decoder, charset);
+                }
+            );
         }
 
         if (val && options.interpretNumericEntities && charset === 'iso-8859-1') {
             val = interpretNumericEntities(val);
         }
 
-        val = parseArrayValue(val, options);
-
         if (part.indexOf('[]=') > -1) {
             val = isArray(val) ? [val] : val;
         }
@@ -116,8 +122,8 @@ var parseValues = function parseQueryStringValues(str, options) {
     return obj;
 };
 
-var parseObject = function (chain, val, options) {
-    var leaf = parseArrayValue(val, options);
+var parseObject = function (chain, val, options, valuesParsed) {
+    var leaf = valuesParsed ? val : parseArrayValue(val, options);
 
     for (var i = chain.length - 1; i >= 0; --i) {
         var obj;
@@ -145,13 +151,13 @@ var parseObject = function (chain, val, options) {
             }
         }
 
-        leaf = obj;
+        leaf = obj; // eslint-disable-line no-param-reassign
     }
 
     return leaf;
 };
 
-var parseKeys = function parseQueryStringKeys(givenKey, val, options) {
+var parseKeys = function parseQueryStringKeys(givenKey, val, options, valuesParsed) {
     if (!givenKey) {
         return;
     }
@@ -202,7 +208,7 @@ var parseKeys = function parseQueryStringKeys(givenKey, val, options) {
         keys.push('[' + key.slice(segment.index) + ']');
     }
 
-    return parseObject(keys, val, options);
+    return parseObject(keys, val, options, valuesParsed);
 };
 
 var normalizeParseOptions = function normalizeParseOptions(opts) {
@@ -254,7 +260,7 @@ module.exports = function (str, opts) {
     var keys = Object.keys(tempObj);
     for (var i = 0; i < keys.length; ++i) {
         var key = keys[i];
-        var newObj = parseKeys(key, tempObj[key], options);
+        var newObj = parseKeys(key, tempObj[key], options, typeof str === 'string');
         obj = utils.merge(obj, newObj, options);
     }
 

diff --git a/test/parse.js b/test/parse.js
@@ -430,7 +430,7 @@ test('parse()', function (t) {
     });
 
     t.test('parses comma delimited array while having percent-encoded comma treated as normal text', function (st) {
-        st.deepEqual(qs.parse('foo=a%2Cb', { comma: true }), { foo: ['a', 'b'] });
+        st.deepEqual(qs.parse('foo=a%2Cb', { comma: true }), { foo: 'a,b' });
         st.deepEqual(qs.parse('foo=a%2C%20b,d', { comma: true }), { foo: ['a, b', 'd'] });
         st.deepEqual(qs.parse('foo=a%2C%20b,c%2C%20d', { comma: true }), { foo: ['a, b', 'c, d'] });