Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I tried to implement a definitive, reliable URL query string parser that handles every corner case:

  • it tries to be efficient by avoiding regex
  • it takes full URLs or just query strings (as long as the query string begins with a question mark)
  • it ignores the hash value
  • it handles multiple equal parameter names
  • it handles parameter names that equal built-in JavaScript methods and keywords

What do you think - did I miss something? 

function parseURLParams(url) {
  if (url === null) return;

  var queryStart = url.indexOf("?") + 1,
      queryEnd   = url.indexOf("#") + 1 || url.length + 1,
      query      = url.slice(queryStart, queryEnd - 1);

  if (query === url || query === "") return;

  var params = {}, 
      nvPairs = query.replace(/\+/g, " ").split("&");

  for (var i=0; i<nvPairs.length; i++) {
    var nv = nvPairs[i],
        eq = nv.indexOf("=") + 1 || nv.length + 1,
        n  = decodeURIComponent( nv.slice(0, eq - 1) ),
        v  = decodeURIComponent( nv.slice(eq) );
    if ( n !== "" ) {
      if ( !Object.prototype.hasOwnProperty.call(params, n) ) {
        params[n] = [];
      }
      params[n].push(v);
    }
  }
  return params;
}

It returns an object of arrays for parsed URLs with query strings and undefined if a query string could not be identified.

I used this in an answer over at SO.

share|improve this question
Deleted my answer - I missed that line, sorry! :-) You might still want to take a look at my other answer over at SO. Interesting how those questions are so similar, posted less than a month apart and the votes are astoundingly different. I blame the [jquery] tag. – Andy E Jun 13 '11 at 11:53
1  
@Andy: Thanks for the cross-reference. I think the jQuery tag might have had an influence. ;) Especially since the 200+ votes answer is essentially flawed. – Tomalak Jun 13 '11 at 11:57
@Andy: And, I'm sorry to say, so is your's. ;) Here's an improved version in your spirit: jsfiddle.net/Tomalak/APXH3 :-) (It still has the problem of not handling multiple parameters of the same name) – Tomalak Jun 13 '11 at 12:27
@Tomalak: Now you have me intrigued :-P what is the flaw you spotted? Near as I can tell, the only difference is that you perform a global replace for + on the entire string (which may be more efficient), and you've allowed for a string to be passed instead of using the current URL, but I can't spot any differences beyond those. I decided not to bloat my answer by supporting dupe parameters, as that practice is a rare one. However, I did link to a proof-of-concept example that would parse the URL in a similar style to how PHP would handle it. – Andy E Jun 13 '11 at 12:41
@Andy Oh, you're right. If you just work on window.location.search and ignore duplicate params, your original version is fine, too. I was testing with full URLs; your key/value regex did not work with them. So, uhm... At least your approach could be more simplified (no nested function d()). :-P – Tomalak Jun 13 '11 at 13:06
show 1 more comment

3 Answers

up vote 1 down vote

Seems a tiny bit over-engineered. Something like this should work just as well, and addresses searlea's points in his answer:

function parseURLParams(url) {
  var out = {}; 
  (url.split('?')[1] || url).split('#')[0].split('&').forEach(function(p) { 
    var kv = p.match(/([^=]*)=?(.*)/), 
        k = decodeURIComponent(kv[1]), 
        v = decodeURIComponent(kv[2] || ''); 
    hasOwnProperty.call(out, k) ? out[k].push(v) : out[k] = [v]; 
  });
  return out;
}

The regex match is only needed if you want to support equals signs in values, otherwise you can use split and the indices 0 and 1.

The main (only?) difference is that pretty much any string will be treated as a viable query -- if there are no equals signs or ampersands, it's a query with just one key and no value.

share|improve this answer
1  
Yikes, just realized how old this question is =/ – Dagg Apr 25 at 2:52
Hm. That's a little too compressed for my tastes. I like less "clever" code. Oh and forEach() is not portable. – Tomalak Apr 25 at 3:01
Fair enough. I felt the same way about forEach and ES5-only features for a long time, but I figure we're at the point by now where ES5 is "normal" and anything else is "legacy," and can be shimmed or whatever if legacy support is needed. – Dagg Apr 25 at 3:46
Right, adding forEach() to the array prototype if necessary is not very difficult, you just have to remember to do it. I suppose most people use some sort of JS library anyway. – Tomalak Apr 25 at 3:52
up vote 2 down vote

you could do a null check on the url argument because the following will throw an exception.

parseURLParams(null);
share|improve this answer
Good point. Done. – Tomalak Jun 15 '11 at 6:32
up vote 2 down vote

Is it bug free? No.

These two corner-cases have been missed:

  1. parameter values containing '=', i.e. 'example.com?foo==bar' (double equals) or '?foo=k=v'
  2. cannot handle parameters called 'toString' and 'valueOf' (amongst others.)

The first may well count as malformed URL, but Chrome handles it and pass through == unencoded in location.search. To handle this, go back to basic indexOf usage.

The second problem's just pedantic really. You could try and work around it using !params.hasOwnProperty(n) instead of !(n in params), but you'll still get stuck if someone passes a parameter called hasOwnProperty. The only way I see around this is to fall back to some dire array-based collection populated something like:

var keys = [], params = [];
for (...) {
    var n = ..., v = ...;
    var i = keys.indexOf(n);
    if (i >= 0) {
        if (!(params[i] instanceof Array)) {
            params[i] = [params[i]];
        }
        params.push(v);
    } else {
        params[i] = v;
        keys.push(n);
    }
}

I guess you'd then have to resort to returning an array of arrays rather than an object. i.e. each element of the array returned would either be [key, value] or [key, [values]], although client might find it easier to work with if you returned something like [key, value1, value2, ...] (which caters nicely for properties without values.)

share|improve this answer
Wow, great answer. :-) I'll come up with an improved version. – Tomalak Jun 13 '11 at 19:30
See modified answer. I've decided not to go down the array-of-arrays route as this is highly impractical. – Tomalak Jun 13 '11 at 20:18
I like the way you handle and call hasOwnProperty - very nice. – searlea Jun 14 '11 at 6:28
Of course it would still be broken "downstream" - the object returned would have its hasOwnProperty overwritten. But the function would not break. Anything else that comes to mind? Efficiency-wise maybe? It seems a little bloated to me... – Tomalak Jun 14 '11 at 6:42
Try Object.prototype.hasOwnProperty.call(params, n) – orip Jan 16 '12 at 8:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.