Crash on malformed query arguments

[Mickael-van-der-Beek]

The library crashes when a querystring contains a value without a key or an empty key.

e.g:

url.parse('http://www.google.com/hello?=test', true)

will crash with the error:

TypeError: Cannot call method 'push' of null
    at QueryStringParser$placeValue [as placeValue] (/node_modules/querystringparser/js/querystringparser.js:267:23)
    at QueryStringParser$parseString [as parseString] /node_modules/querystringparser/js/querystringparser.js:373:26)
    at Function.QueryStringParser$Parse [as parse] (/node_modules/querystringparser/js/querystringparser.js:51:23)
    at Url$parse [as parse] (/node_modules/fast-url-parser/src/urlparser.js:107:38)
    at Function.Url$Parse [as parse] (/node_modules/fast-url-parser/src/urlparser.js:892:9)
    at repl:1:6
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)

In comparison Node.js (v0.10.29 for me), will return:

{ protocol: 'http:',
  slashes: true,
  auth: null,
  host: 'www.google.com',
  port: null,
  hostname: 'www.google.com',
  hash: null,
  search: '?=test',
  query: { '': 'test' },
  pathname: '/hello',
  path: '/hello?=test',
  href: 'http://www.google.com/hello?=test' }

and considers that the key is an empty string ''.

[petkaantonov]

Some notes:

• this module replaces the qs module, not querystring that comes with node. What does qs do?
• this module is expected to throw, so you will need a try catch anyway

[Mickael-van-der-Beek]

@petkaantonov : I'm using querystringparser as a drop-in replacement for qs inside fast-url-parser, like this:

var querystringParser = require('querystringparser');
var fastUrlParser = require('fast-url-parser');

fastUrlParser.replace();

url.parse('http://www.google.com/hello?=test', true)

I understand the part about using a try {} catch {} block but since fast-url-parser is supposed to be used as a drop-in replacement of the core url module I found it a little weird.

[Mickael-van-der-Beek]

The same code without querystringparser, so with qs will be something like this:

var fastUrlParser = require('fast-url-parser');

fastUrlParser.replace();

url.parse('http://www.google.com/hello?=test', true);

and return the following object:

{ _protocol: 'http',
  _href: '',
  _port: -1,
  _query: { '': 'test' },
  auth: null,
  slashes: true,
  host: 'www.google.com',
  hostname: 'www.google.com',
  hash: null,
  search: '?=test',
  pathname: '/hello',
  _prependSlash: false }

[petkaantonov]

querystringparser is not compatible with querystring but fast url parser is compatible with url

[Mickael-van-der-Beek]

@petkaantonov If querystringparser is compatible with qs and fast-url-parser with url. Wouldn't that make it necessary for querystringparser to be compatible with querystring at least in the context of a drop-in replacement of url ?

[tjconcept]

@Mickael-van-der-Beek qs is not compatible with nodes url :)

On another note I hit this same exception when passing { '': '' } as argument (result of a malformed request). Could there be exposed an Error class (for use as predicate) for operational errors? I'm not sure this one should happen though, as I guess there is no such thing as a "valid" query. Maybe it should always just do a best effort?