Merge pull request #1600 from simov/qs-rfc3986

Extract the querystring logic into separate module

README.md

@@ -199,8 +199,7 @@ For advanced cases, you can access the form-data object itself via `r.form()`. T
 
 ```js
 // NOTE: Advanced use-case, for normal use see 'formData' usage above
-var r = request.post('http://service.com/upload', function optionalCallback(err, httpResponse, body) { // ...
-
+var r = request.post('http://service.com/upload', function optionalCallback(err, httpResponse, body) {...})
 var form = r.form();
 form.append('my_field', 'my_value');
 form.append('my_buffer', new Buffer([1, 2, 3]));
@@ -723,8 +722,8 @@ The first argument can be either a `url` or an `options` object. The only requir
 ---
 
 - `qs` - object containing querystring values to be appended to the `uri`
-- `qsParseOptions` - object containing options to pass to the [qs.parse](https://github.com/hapijs/qs#parsing-objects) method or [querystring.parse](https://nodejs.org/docs/v0.12.0/api/querystring.html#querystring_querystring_parse_str_sep_eq_options) method
-- `qsStringifyOptions` - object containing options to pass to the [qs.stringify](https://github.com/hapijs/qs#stringifying) method or to the [querystring.stringify](https://nodejs.org/docs/v0.12.0/api/querystring.html#querystring_querystring_stringify_obj_sep_eq_options) method. For example, to change the way arrays are converted to query strings pass the `arrayFormat` option with one of `indices|brackets|repeat`
+- `qsParseOptions` - object containing options to pass to the [qs.parse](https://github.com/hapijs/qs#parsing-objects) method. Alternatively pass options to the [querystring.parse](https://nodejs.org/docs/v0.12.0/api/querystring.html#querystring_querystring_parse_str_sep_eq_options) method using this format `{sep:';', eq:':', options:{}}`
+- `qsStringifyOptions` - object containing options to pass to the [qs.stringify](https://github.com/hapijs/qs#stringifying) method. Alternatively pass options to the  [querystring.stringify](https://nodejs.org/docs/v0.12.0/api/querystring.html#querystring_querystring_stringify_obj_sep_eq_options) method using this format `{sep:';', eq:':', options:{}}`. For example, to change the way arrays are converted to query strings using the `qs` module pass the `arrayFormat` option with one of `indices|brackets|repeat`
 - `useQuerystring` - If true, use `querystring` to stringify and parse
   querystrings, otherwise use `qs` (default: `false`).  Set this option to
   `true` if you need arrays to be serialized as `foo=bar&foo=baz` instead of the

lib/querystring.js

@@ -0,0 +1,51 @@
+'use strict'
+
+var qs = require('qs')
+  , querystring = require('querystring')
+
+
+function Querystring (request) {
+  this.request = request
+  this.lib = null
+  this.useQuerystring = null
+  this.parseOptions = null
+  this.stringifyOptions = null
+}
+
+Querystring.prototype.init = function (options) {
+  if (this.lib) {return}
+
+  this.useQuerystring = options.useQuerystring
+  this.lib = (this.useQuerystring ? querystring : qs)
+
+  this.parseOptions = options.qsParseOptions || {}
+  this.stringifyOptions = options.qsStringifyOptions || {}
+}
+
+Querystring.prototype.stringify = function (obj) {
+  return (this.useQuerystring)
+    ? this.rfc3986(this.lib.stringify(obj,
+      this.stringifyOptions.sep || null,
+      this.stringifyOptions.eq || null,
+      this.stringifyOptions))
+    : this.lib.stringify(obj, this.stringifyOptions)
+}
+
+Querystring.prototype.parse = function (str) {
+  return (this.useQuerystring)
+    ? this.lib.parse(str,
+      this.parseOptions.sep || null,
+      this.parseOptions.eq || null,
+      this.parseOptions)
+    : this.lib.parse(str, this.parseOptions)
+}
+
+Querystring.prototype.rfc3986 = function (str) {
+  return str.replace(/[!'()*]/g, function (c) {
+    return '%' + c.charCodeAt(0).toString(16).toUpperCase()
+  })
+}
+
+Querystring.prototype.unescape = querystring.unescape
+
+exports.Querystring = Querystring

package.json

@@ -29,7 +29,7 @@
     "json-stringify-safe": "~5.0.0",
     "mime-types": "~2.0.1",
     "node-uuid": "~1.4.0",
-    "qs": "~2.4.0",
+    "qs": "~3.0.0",
     "tunnel-agent": "~0.4.0",
     "tough-cookie": ">=0.12.0",
     "http-signature": "~0.10.0",

request.js

@@ -5,8 +5,6 @@ var http = require('http')
   , url = require('url')
   , util = require('util')
   , stream = require('stream')
-  , qs = require('qs')
-  , querystring = require('querystring')
   , zlib = require('zlib')
   , helpers = require('./lib/helpers')
   , bl = require('bl')
@@ -22,6 +20,7 @@ var http = require('http')
   , cookies = require('./lib/cookies')
   , copy = require('./lib/copy')
   , getProxyFromURI = require('./lib/getProxyFromURI')
+  , Querystring = require('./lib/querystring').Querystring
   , Har = require('./lib/har').Har
   , Auth = require('./lib/auth').Auth
   , OAuth = require('./lib/oauth').OAuth
@@ -229,13 +228,6 @@ function responseToJSON() {
   }
 }
 
-// encode rfc3986 characters
-function rfc3986 (str) {
-  return str.replace(/[!'()*]/g, function(c) {
-    return '%' + c.charCodeAt(0).toString(16).toUpperCase()
-  })
-}
-
 function Request (options) {
   // if given the method property in options, set property explicitMethod to true
 
@@ -265,6 +257,7 @@ function Request (options) {
   if (options.method) {
     self.explicitMethod = true
   }
+  self._qs = new Querystring(self)
   self._auth = new Auth(self)
   self._oauth = new OAuth(self)
   self._multipart = new Multipart(self)
@@ -341,15 +334,7 @@ Request.prototype.init = function (options) {
     self.localAddress = options.localAddress
   }
 
-  if (!self.qsLib) {
-    self.qsLib = (options.useQuerystring ? querystring : qs)
-  }
-  if (!self.qsParseOptions) {
-    self.qsParseOptions = options.qsParseOptions
-  }
-  if (!self.qsStringifyOptions) {
-    self.qsStringifyOptions = options.qsStringifyOptions
-  }
+  self._qs.init(options)
 
   debug(options)
   if (!self.pool && self.pool !== false) {
@@ -576,14 +561,12 @@ Request.prototype.init = function (options) {
   }
 
   if (self.uri.auth && !self.hasHeader('authorization')) {
-    var uriAuthPieces = self.uri.auth.split(':').map(function(item) { return querystring.unescape(item) })
+    var uriAuthPieces = self.uri.auth.split(':').map(function(item) {return self._qs.unescape(item)})
     self.auth(uriAuthPieces[0], uriAuthPieces.slice(1).join(':'), true)
   }
 
   if (!self.tunnel && self.proxy && self.proxy.auth && !self.hasHeader('proxy-authorization')) {
-    var proxyAuthPieces = self.proxy.auth.split(':').map(function(item) {
-      return querystring.unescape(item)
-    })
+    var proxyAuthPieces = self.proxy.auth.split(':').map(function(item) {return self._qs.unescape(item)})
     var authHeader = 'Basic ' + toBase64(proxyAuthPieces.join(':'))
     self.setHeader('proxy-authorization', authHeader)
   }
@@ -1295,7 +1278,7 @@ Request.prototype.qs = function (q, clobber) {
   var self = this
   var base
   if (!clobber && self.uri.query) {
-    base = self.qsLib.parse(self.uri.query, self.qsParseOptions)
+    base = self._qs.parse(self.uri.query)
   } else {
     base = {}
   }
@@ -1304,13 +1287,13 @@ Request.prototype.qs = function (q, clobber) {
     base[i] = q[i]
   }
 
-  if (self.qsLib.stringify(base, self.qsStringifyOptions) === '') {
+  if (self._qs.stringify(base) === '') {
     return self
   }
 
-  var qs = self.qsLib.stringify(base, self.qsStringifyOptions)
+  var qs = self._qs.stringify(base)
 
-  self.uri = url.parse(self.uri.href.split('?')[0] + '?' + rfc3986(qs))
+  self.uri = url.parse(self.uri.href.split('?')[0] + '?' + qs)
   self.url = self.uri
   self.path = self.uri.path
 
@@ -1321,9 +1304,8 @@ Request.prototype.form = function (form) {
   if (form) {
     self.setHeader('content-type', 'application/x-www-form-urlencoded')
     self.body = (typeof form === 'string')
-      ? form.toString('utf8')
-      : self.qsLib.stringify(form, self.qsStringifyOptions).toString('utf8')
-    self.body = rfc3986(self.body)
+      ? self._qs.rfc3986(form.toString('utf8'))
+      : self._qs.stringify(form).toString('utf8')
     return self
   }
   // create form-data object
@@ -1359,7 +1341,7 @@ Request.prototype.json = function (val) {
       if (!/^application\/x-www-form-urlencoded\b/.test(self.getHeader('content-type'))) {
         self.body = safeStringify(self.body)
       } else {
-        self.body = rfc3986(self.body)
+        self.body = self._qs.rfc3986(self.body)
       }
       if (!self.hasHeader('content-type')) {
         self.setHeader('content-type', 'application/json')

tests/test-qs.js

@@ -13,23 +13,18 @@ var request = require('../index')
 //   - expectedQuerystring : expected path when using the querystring library
 function runTest(name, options) {
   var uri = 'http://www.google.com' + (options.suffix || '')
-    , requestOptsQs = {
-      uri : uri,
-      qsParseOptions: options.qsParseOptions,
-      qsStringifyOptions: options.qsStringifyOptions
-    }
-    , requestOptsQuerystring = {
-      uri : uri,
-      useQuerystring : true
-    }
+  var opts = {
+    uri : uri,
+    qsParseOptions: options.qsParseOptions,
+    qsStringifyOptions: options.qsStringifyOptions
+  }
 
   if (options.qs) {
-    requestOptsQs.qs = options.qs
-    requestOptsQuerystring.qs = options.qs
+    opts.qs = options.qs
   }
 
-  tape(name + ' using qs', function(t) {
-    var r = request.get(requestOptsQs)
+  tape(name + ' - using qs', function(t) {
+    var r = request.get(opts)
     if (typeof options.afterRequest === 'function') {
       options.afterRequest(r)
     }
@@ -40,8 +35,9 @@ function runTest(name, options) {
     })
   })
 
-  tape(name + ' using querystring', function(t) {
-    var r = request.get(requestOptsQuerystring)
+  tape(name + ' - using querystring', function(t) {
+    opts.useQuerystring = true
+    var r = request.get(opts)
     if (typeof options.afterRequest === 'function') {
       options.afterRequest(r)
     }
@@ -121,3 +117,19 @@ runTest('pass options to the qs module via the qsStringifyOptions key', {
   expected : esc('/?order[]=bar&order[]=desc'),
   expectedQuerystring : '/?order=bar&order=desc'
 })
+
+runTest('pass options to the querystring module via the qsParseOptions key', {
+  suffix   : '?a=1;b=2',
+  qs: {},
+  qsParseOptions: { sep : ';' },
+  qsStringifyOptions: { sep : ';' },
+  expected : esc('/?a=1%3Bb%3D2'),
+  expectedQuerystring : '/?a=1;b=2'
+})
+
+runTest('pass options to the querystring module via the qsStringifyOptions key', {
+  qs       : { order : ['bar', 'desc'] },
+  qsStringifyOptions: { sep : ';' },
+  expected : esc('/?order[0]=bar&order[1]=desc'),
+  expectedQuerystring : '/?order=bar;order=desc'
+})

tests/test-rfc3986.js

@@ -97,8 +97,13 @@ var cases = [
   }
 ]
 
-cases.forEach(function (options) {
-  tape('rfc3986 ' + options._name, function(t) {
-    runTest(t, options)
+var libs = ['qs', 'querystring']
+
+libs.forEach(function (lib) {
+  cases.forEach(function (options) {
+    options.useQuerystring = (lib === 'querystring')
+    tape(lib + ' rfc3986 ' + options._name, function(t) {
+      runTest(t, options)
+    })
   })
 })