Get rid of PHP/Rails style parameter munging.

- Handle only the most basic of query string parsing and construction. Leave the rest (e.g. Rails/PHP behaviors) to modules higher up the stack, like Express.
nodejs · Sep 13, 2010 · 422d3c9 · 422d3c9
1 parent debf389
commit 422d3c9
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 127 deletions.
diff --git a/doc/api.markdown b/doc/api.markdown
@@ -2988,9 +2988,10 @@ Take a base URL, and a href URL, and resolve them as a browser would for an anch
 
 This module provides utilities for dealing with query strings.  It provides the following methods:
 
-### querystring.stringify(obj, sep='&', eq='=', munge=true)
+### querystring.stringify(obj, sep='&', eq='=')
 
 Serialize an object to a query string.  Optionally override the default separator and assignment characters.
+
 Example:
 
     querystring.stringify({foo: 'bar'})
@@ -3001,40 +3002,18 @@ Example:
     // returns
     'foo:bar;baz:bob'
 
-By default, this function will perform PHP/Rails-style parameter munging for arrays and objects used as
-values within `obj`.
-Example:
-
-    querystring.stringify({foo: ['bar', 'baz', 'boz']})
-    // returns
-    'foo%5B%5D=bar&foo%5B%5D=baz&foo%5B%5D=boz'
-
-    querystring.stringify({foo: {bar: 'baz'}})
-    // returns
-    'foo%5Bbar%5D=baz'
-
-If you wish to disable the array munging (e.g. when generating parameters for a Java servlet), you
-can set the `munge` argument to `false`.
-Example:
-
-    querystring.stringify({foo: ['bar', 'baz', 'boz']}, '&', '=', false)
-    // returns
-    'foo=bar&foo=baz&foo=boz'
-
-Note that when `munge` is `false`, parameter names with object values will still be munged.
-
 ### querystring.parse(str, sep='&', eq='=')
 
 Deserialize a query string to an object.  Optionally override the default separator and assignment characters.
 
+Example:
+
     querystring.parse('a=b&b=c')
     // returns
     { 'a': 'b'
     , 'b': 'c'
     }
 
-This function can parse both munged and unmunged query strings (see `stringify` for details).
-
 ### querystring.escape
 
 The escape function used by `querystring.stringify`, provided so that it could be overridden if necessary.

diff --git a/lib/querystring.js b/lib/querystring.js
@@ -10,8 +10,22 @@ QueryString.escape = function (str) {
   return encodeURIComponent(str);
 };
 
+var stringifyPrimitive = function(v) {
+  switch (typeof v) {
+    case "string":
+      return v;
+
+    case "boolean":
+      return v ? "true" : "false";
+
+    case "number":
+      return isFinite(v) ? v : "";
+
+    default:
+      return "";
+  }
+};
 
-var stack = [];
 /**
  * <p>Converts an arbitrary value to a Query String representation.</p>
  *
@@ -21,92 +35,61 @@ var stack = [];
  * @param obj {Variant} any arbitrary value to convert to query string
  * @param sep {String} (optional) Character that should join param k=v pairs together. Default: "&"
  * @param eq  {String} (optional) Character that should join keys to their values. Default: "="
- * @param munge {Boolean} (optional) Indicate whether array/object params should be munged, PHP/Rails-style. Default: true
  * @param name {String} (optional) Name of the current key, for handling children recursively.
  * @static
  */
-QueryString.stringify = QueryString.encode = function (obj, sep, eq, munge, name) {
-  munge = typeof munge == "undefined" || munge;
+QueryString.stringify = QueryString.encode = function (obj, sep, eq, name) {
   sep = sep || "&";
   eq = eq || "=";
-  var type = Object.prototype.toString.call(obj);
-  if (obj == null || type == "[object Function]" || type == "[object Number]" && !isFinite(obj)) {
-    return name ? QueryString.escape(name) + eq : "";
-  }
-
-  switch (type) {
-    case '[object Boolean]':
-      obj = +obj; // fall through
-    case '[object Number]':
-    case '[object String]':
-      return QueryString.escape(name) + eq + QueryString.escape(obj);
-    case '[object Array]':
-      name = name + (munge ? "[]" : "");
-      return obj.map(function (item) {
-        return QueryString.stringify(item, sep, eq, munge, name);
-      }).join(sep);
-  }
-  // now we know it's an object.
-
-  // Check for cyclical references in nested objects
-  for (var i = stack.length - 1; i >= 0; --i) if (stack[i] === obj) {
-    throw new Error("querystring.stringify. Cyclical reference");
-  }
-
-  stack.push(obj);
+  obj = (obj === null) ? undefined : obj;
 
-  var begin = name ? name + "[" : "",
-      end = name ? "]" : "",
-      keys = Object.keys(obj),
-      n,
-      s = Object.keys(obj).map(function (key) {
-        n = begin + key + end;
-        return QueryString.stringify(obj[key], sep, eq, munge, n);
+  switch (typeof obj) {
+    case "object":
+      return Object.keys(obj).map(function(k) {
+        if (Array.isArray(obj[k])) {
+          return obj[k].map(function(v) {
+            return QueryString.escape(stringifyPrimitive(k)) +
+                   eq +
+                   QueryString.escape(stringifyPrimitive(v));
+          }).join(sep);
+        } else {
+          return QueryString.escape(stringifyPrimitive(k)) + 
+                 eq +
+                 QueryString.escape(stringifyPrimitive(obj[k]));
+        }
       }).join(sep);
 
-  stack.pop();
-
-  if (!s && name) {
-    return name + "=";
+    default:
+      return (name) ?
+        QueryString.escape(stringifyPrimitive(name)) + eq +
+          QueryString.escape(stringifyPrimitive(obj)) :
+        "";
   }
-  return s;
 };
 
-// matches .xxxxx or [xxxxx] or ['xxxxx'] or ["xxxxx"] with optional [] at the end
-var chunks = /(?:(?:^|\.)([^\[\(\.]+)(?=\[|\.|$|\()|\[([^"'][^\]]*?)\]|\["([^\]"]*?)"\]|\['([^\]']*?)'\])(\[\])?/g;
 // Parse a key=val string.
 QueryString.parse = QueryString.decode = function (qs, sep, eq) {
+  sep = sep || "&";
+  eq = eq || "=";
   var obj = {};
-  if (qs === undefined) { return {} }
-  String(qs).split(sep || "&").map(function (keyValue) {
-    var res = obj,
-      next,
-      kv = keyValue.split(eq || "="),
-      key = QueryString.unescape(kv.shift(), true),
-      value = QueryString.unescape(kv.join(eq || "="), true);
-    key.replace(chunks, function (all, name, nameInBrackets, nameIn2Quotes, nameIn1Quotes, isArray, offset) {
-      var end = offset + all.length == key.length;
-      name = name || nameInBrackets || nameIn2Quotes || nameIn1Quotes;
-      next = end ? value : {};
-      if (Array.isArray(res[name])) {
-        res[name].push(next);
-        res = next;
-      } else {
-        if (name in res) {
-          if (isArray || end) {
-            res = (res[name] = [res[name], next])[1];
-          } else {
-            res = res[name];
-          }
-        } else {
-          if (isArray) {
-            res = (res[name] = [next])[0];
-          } else {
-            res = res[name] = next;
-          }
-        }
-      }
-    });
+
+  if (typeof qs !== 'string') {
+    return obj;
+  }
+
+  qs.split(sep).forEach(function(kvp) {
+    var x = kvp.split(eq);
+    var k = QueryString.unescape(x[0], true);
+    var v = QueryString.unescape(x.slice(1).join(eq), true);
+
+    if (!(k in obj)) {
+        obj[k] = v;
+    } else if (!Array.isArray(obj[k])) {
+        obj[k] = [obj[k], v];
+    } else {
+        obj[k].push(v);
+    }
   });
+
   return obj;
 };
diff --git a/test/simple/test-querystring.js b/test/simple/test-querystring.js
@@ -10,35 +10,17 @@ var qs = require("querystring");
 var qsTestCases = [
   ["foo=918854443121279438895193", "foo=918854443121279438895193", {"foo": "918854443121279438895193"}],
   ["foo=bar",  "foo=bar", {"foo" : "bar"}],
-  ["foo=bar&foo=quux", "foo%5B%5D=bar&foo%5B%5D=quux", {"foo" : ["bar", "quux"]}],
+  ["foo=bar&foo=quux", "foo=bar&foo=quux", {"foo" : ["bar", "quux"]}],
   ["foo=1&bar=2", "foo=1&bar=2", {"foo" : "1", "bar" : "2"}],
   ["my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F", "my%20weird%20field=q1!2%22'w%245%267%2Fz8)%3F", {"my weird field" : "q1!2\"'w$5&7/z8)?" }],
   ["foo%3Dbaz=bar", "foo%3Dbaz=bar", {"foo=baz" : "bar"}],
   ["foo=baz=bar", "foo=baz%3Dbar", {"foo" : "baz=bar"}],
-    [ "str=foo&arr[]=1&arr[]=2&arr[]=3&obj[a]=bar&obj[b][]=4&obj[b][]=5&obj[b][]=6&obj[b][]=&obj[c][]=4&obj[c][]=5&obj[c][][somestr]=baz&obj[objobj][objobjstr]=blerg&somenull=&undef=", "str=foo&arr%5B%5D=1&arr%5B%5D=2&arr%5B%5D=3&obj%5Ba%5D=bar&obj%5Bb%5D%5B%5D=4&obj%5Bb%5D%5B%5D=5&obj%5Bb%5D%5B%5D=6&obj%5Bb%5D%5B%5D=&obj%5Bc%5D%5B%5D=4&obj%5Bc%5D%5B%5D=5&obj%5Bc%5D%5B%5D%5Bsomestr%5D=baz&obj%5Bobjobj%5D%5Bobjobjstr%5D=blerg&somenull=&undef=", {
+    [ "str=foo&arr=1&arr=2&arr=3&somenull=&undef=", "str=foo&arr=1&arr=2&arr=3&somenull=&undef=", {
     "str":"foo",
     "arr":["1","2","3"],
-    "obj":{
-      "a":"bar",
-      "b":["4","5","6",""],
-      "c":["4","5",{"somestr":"baz"}],
-      "objobj":{"objobjstr":"blerg"}
-    },
     "somenull":"",
     "undef":""
   }],
-  ["foo[bar][bla]=baz&foo[bar][bla]=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-  ["foo[bar][][bla]=baz&foo[bar][][bla]=blo", "foo%5Bbar%5D%5B%5D%5Bbla%5D=baz&foo%5Bbar%5D%5B%5D%5Bbla%5D=blo", {"foo":{"bar":[{"bla":"baz"},{"bla":"blo"}]}}],
-  ["foo[bar][bla][]=baz&foo[bar][bla][]=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-
-  ["foo.bar.bla=baz&foo.bar.bla=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-  ["foo.bar[].bla=baz&foo[bar][][bla]=blo", "foo%5Bbar%5D%5B%5D%5Bbla%5D=baz&foo%5Bbar%5D%5B%5D%5Bbla%5D=blo", {"foo":{"bar":[{"bla":"baz"},{"bla":"blo"}]}}],
-  ["foo[bar].bla[]=baz&foo.bar[bla][]=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-
-  ["foo['bar']['bla']=baz&foo[\"bar\"][\"bla\"]=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-  ["foo['bar'][]['bla']=baz&foo['bar'][][\"bla\"]=blo", "foo%5Bbar%5D%5B%5D%5Bbla%5D=baz&foo%5Bbar%5D%5B%5D%5Bbla%5D=blo", {"foo":{"bar":[{"bla":"baz"},{"bla":"blo"}]}}],
-  ["foo[bar][\"bla\"][]=baz&foo[\"bar\"][bla][]=blo", "foo%5Bbar%5D%5Bbla%5D%5B%5D=baz&foo%5Bbar%5D%5Bbla%5D%5B%5D=blo", {"foo":{"bar":{"bla":["baz","blo"]}}}],
-
   [" foo = bar ", "%20foo%20=%20bar%20", {" foo ":" bar "}],
   ["foo=%zx", "foo=%25zx", {"foo":"%zx"}],
   ["foo=%EF%BF%BD", "foo=%EF%BF%BD", {"foo" : "\ufffd" }]
@@ -47,7 +29,7 @@ var qsTestCases = [
 // [ wonkyQS, canonicalQS, obj ]
 var qsColonTestCases = [
   ["foo:bar", "foo:bar", {"foo":"bar"}],
-  ["foo:bar;foo:quux", "foo%5B%5D:bar;foo%5B%5D:quux", {"foo" : ["bar", "quux"]}],
+  ["foo:bar;foo:quux", "foo:bar;foo:quux", {"foo" : ["bar", "quux"]}],
   ["foo:1&bar:2;baz:quux", "foo:1%26bar%3A2;baz:quux", {"foo":"1&bar:2", "baz":"quux"}],
   ["foo%3Abaz:bar", "foo%3Abaz:bar", {"foo:baz":"bar"}],
   ["foo:baz:bar", "foo:baz%3Abar", {"foo":"baz:bar"}]
@@ -65,8 +47,8 @@ var qsWeirdObjects = [
   [ {e:extendedFunction}, "e=", {"e":""} ],
   [ {d:new Date()}, "d=", {"d":""} ],
   [ {d:Date}, "d=", {"d":""} ],
-  [ {f:new Boolean(false), t:new Boolean(true)}, "f=0&t=1", {"f":"0", "t":"1"} ],
-  [ {f:false, t:true}, "f=0&t=1", {"f":"0", "t":"1"} ],
+  [ {f:new Boolean(false), t:new Boolean(true)}, "f=&t=", {"f":"", "t":""} ],
+  [ {f:false, t:true}, "f=false&t=true", {"f":"false", "t":"true"} ],
   [ {n:null}, "n=", {"n":""} ],
   [ {nan:NaN}, "nan=", {"nan":""} ],
   [ {inf:Infinity}, "inf=", {"inf":""} ]
@@ -84,7 +66,7 @@ var qsNoMungeTestCases = [
   ["gragh=1&gragh=3&goo=2", {"gragh": ["1", "3"], "goo": "2"}],
   ["frappucino=muffin&goat%5B%5D=scone&pond=moose",
    {"frappucino": "muffin", "goat[]": "scone", "pond": "moose"}],
-  ["obj%5Btrololol%5D=yes&obj%5Blololo%5D=no", {"obj": {"trololol": "yes", "lololo": "no"}}]
+  ["trololol=yes&lololo=no", {"trololol": "yes", "lololo": "no"}]
 ];
 
 assert.strictEqual("918854443121279438895193", qs.parse("id=918854443121279438895193").id);
@@ -123,11 +105,6 @@ qsNoMungeTestCases.forEach(function (testCase) {
 })();
 
 // now test stringifying
-assert.throws(function () {
-  var f = {};
-  f.f = f;
-  qs.stringify(f);
-});
 
 // basic
 qsTestCases.forEach(function (testCase) {