fix: Encoding query parameters when recording to string. (#1015)

lib/common.js

@@ -327,42 +327,51 @@ function matchStringOrRegexp(target, pattern) {
   return pattern instanceof RegExp  ? str.match(pattern) : str === String(pattern);
 }
 
-// return [newKey, newValue]
-function formatQueryValue(key, value, options) {
+/**
+ * Formats a query parameter.
+ *
+ * @param key                The key of the query parameter to format.
+ * @param value              The value of the query parameter to format.
+ * @param stringFormattingFn The function used to format string values. Can
+ *                           be used to encode or decode the query value.
+ *
+ * @returns the formatted [key, value] pair.
+ */
+function formatQueryValue(key, value, stringFormattingFn) {
   switch (true) {
-    case _.isNumber(value): // fall-though
+    case _.isNumber(value): // fall-through
     case _.isBoolean(value):
       value = value.toString();
       break;
-    case _.isUndefined(value): // fall-though
+    case _.isUndefined(value): // fall-through
     case _.isNull(value):
       value = '';
       break;
     case _.isString(value):
-      if(options.encodedQueryParams) {
-        value = percentDecode(value);
+      if(stringFormattingFn) {
+        value = stringFormattingFn(value);
       }
       break;
     case (value instanceof RegExp):
       break;
     case _.isArray(value):
       var tmpArray = new Array(value.length);
       for (var i = 0; i < value.length; ++i) {
-        tmpArray[i] = formatQueryValue(i, value[i], options)[1];
+        tmpArray[i] = formatQueryValue(i, value[i], stringFormattingFn)[1];
       }
       value = tmpArray;
       break;
     case _.isObject(value):
       var tmpObj = {};
       _.forOwn(value, function(subVal, subKey){
-        var subPair = formatQueryValue(subKey, subVal, options);
+        var subPair = formatQueryValue(subKey, subVal, stringFormattingFn);
         tmpObj[subPair[0]] = subPair[1];
       });
       value = tmpObj;
       break;
   }
 
-  if (options.encodedQueryParams) key = percentDecode(key);
+  if (stringFormattingFn) key = stringFormattingFn(key);
   return [key, value];
 }
 

lib/interceptor.js

@@ -410,21 +410,27 @@ Interceptor.prototype.basicAuth = function basicAuth(options) {
  */
 Interceptor.prototype.query = function query(queries) {
     this.queries = this.queries || {};
+
     // Allow all query strings to match this route
     if (queries === true) {
         this.queries = queries;
+        return this;
     }
 
     if (_.isFunction(queries)) {
         this.queries = queries;
         return this;
     }
 
-    for (var q in queries) {
-        if (_.isUndefined(this.queries[q])) {
-            var value = queries[q];
-            var formatedPair = common.formatQueryValue(q, value, this.scope.scopeOptions);
-            this.queries[formatedPair[0]] = formatedPair[1];
+    var stringFormattingFn;
+    if (this.scope.scopeOptions.encodedQueryParams) {
+        stringFormattingFn = common.percentDecode;
+    }
+
+    for (var key in queries) {
+        if (_.isUndefined(this.queries[key])) {
+            var formattedPair = common.formatQueryValue(key, queries[key], stringFormattingFn);
+            this.queries[formattedPair[0]] = formattedPair[1];
         }
     }
 

lib/recorder.js

@@ -118,6 +118,12 @@ function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
     var queryStr = req.path.slice(queryIndex + 1);
     queryObj = qs.parse(queryStr);
   }
+  // Always encoding the query parameters when recording.
+  var encodedQueryObj = {};
+  for (var key in queryObj) {
+    var formattedPair = common.formatQueryValue(key, queryObj[key], common.percentEncode);
+    encodedQueryObj[formattedPair[0]] = formattedPair[1];
+  }
 
   var ret = [];
   ret.push('\nnock(\'');
@@ -143,7 +149,7 @@ function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
 
   if (queryIndex !== -1) {
     ret.push('  .query(');
-    ret.push(JSON.stringify(queryObj));
+    ret.push(JSON.stringify(encodedQueryObj));
     ret.push(')\n');
   }
 

tests/test_recorder.js

@@ -719,6 +719,29 @@ test('includes query parameters from superagent', {skip: process.env.AIRPLANE},
     });
 });
 
+test('encodes the query parameters when not outputing objects', {skip: process.env.AIRPLANE}, function(t) {
+
+  nock.restore();
+  nock.recorder.clear();
+  t.equal(nock.recorder.play().length, 0);
+
+  nock.recorder.rec({
+    dont_print: true,
+    output_objects: false
+  });
+
+  superagent.get('http://google.com')
+    .query({q: 'test search++' })
+    .end(function(res) {
+      nock.restore();
+      var recording = nock.recorder.play();
+      t.true(recording.length >= 1);
+      t.true(recording[0].indexOf('test%20search%2B%2B') !== -1);
+      t.end();
+    });
+
+});
+
 test('works with clients listening for readable', {skip: process.env.AIRPLANE}, function(t) {
   nock.restore();
   nock.recorder.clear();
@@ -838,7 +861,7 @@ test('outputs query string arrays correctly', {skip: process.env.AIRPLANE}, func
   });
 });
 
-test('removes query params from from that path and puts them in query()', {skip: process.env.AIRPLANE}, function(t) {
+test('removes query params from that path and puts them in query()', {skip: process.env.AIRPLANE}, function(t) {
   nock.restore();
   nock.recorder.clear();
   t.equal(nock.recorder.play().length, 0);