Merge pull request #78 from groupon/jk-fix-header-leak

Fix header option leak

lib/gofer.js

@@ -96,7 +96,7 @@ function preventComplexMerge(objValue, srcValue) {
     return srcValue || objValue;
   }
 
-  return mergeWith(objValue, srcValue, preventComplexMerge);
+  return mergeWith({}, objValue, srcValue, preventComplexMerge);
 }
 
 Gofer.prototype._prepareOptions =

package.json

@@ -45,6 +45,7 @@
     "mochify": "^2.17.0",
     "nlm": "^3.0.0",
     "promise": "^7.1.1",
+    "self-signed": "^1.3.1",
     "whatwg-fetch": "^0.11.0"
   },
   "author": {

test/fetch-https.test.js

@@ -15,10 +15,8 @@ describe('fetch: https', function () {
     // This is a remote call which isn't great but it means we get a valid
     // https certificate without having to pull any tricks.
     this.timeout(2000);
-    return fetch('https://api.reddit.com/r/javascript/about.json')
-      .then(function (res) {
-        assert.equal(200, res.statusCode);
-      });
+    return fetch('https://api.reddit.com/user/ageitgey/about.json')
+      .json();
   });
 
   it('fails with self-signed https', function () {
@@ -28,10 +26,10 @@ describe('fetch: https', function () {
         if (typeof document === 'undefined') {
           if (error.code) {
             // more recent node versions (e.g. 4+)
-            assert.equal('SELF_SIGNED_CERT_IN_CHAIN', error.code);
+            assert.match(/SELF_SIGNED/, error.code);
           } else {
             // old node versions (e.g. 0.10)
-            assert.equal('SELF_SIGNED_CERT_IN_CHAIN', error.message);
+            assert.match(/SELF_SIGNED/, error.message);
           }
         }
       });
@@ -55,9 +53,8 @@ describe('fetch: https', function () {
       // Browsers don't allow to side-step https
       return this.skip();
     }
-    var fs = require('fs');
     return fetch(options.baseUrlTls, {
-      ca: fs.readFileSync('test/certs/client/my-root-ca.crt.pem'),
+      ca: [options.certOptions.cert],
     })
       .then(function (res) {
         assert.equal(200, res.statusCode);

test/gofer.test.js

@@ -25,6 +25,27 @@ describe('gofer', function () {
     });
   });
 
+  describe('call that sets a header', function () {
+    var gofer;
+    before(function () {
+      gofer = new Gofer().with({
+        headers: { 'x-a': 'foo' },
+      }).with(options);
+
+      return gofer.fetch('/echo', {
+        headers: { 'x-b': 'should not leak' },
+      }).json();
+    });
+
+    it('does not affect defaults', function () {
+      return gofer.fetch('/echo').json()
+        .then(function (echo) {
+          assert.equal('foo', echo.headers['x-a']);
+          assert.equal(undefined, echo.headers['x-b']);
+        });
+    });
+  });
+
   describe('sub-class', function () {
     function SubGofer(config) {
       Gofer.call(this, config, 'sub', '1.2.3', 'my-sub-client');

test/mock-service.js

@@ -1,9 +1,11 @@
 'use strict';
-var fs = require('fs');
+
 var http = require('http');
 var https = require('https');
 var parseUrl = require('url').parse;
 
+var selfSigned = require('self-signed');
+
 var options = require('./mock-service.browser');
 
 var MOCK_SERVICE_PORT = +(options.baseUrl.match(/:(\d+)/)[1]);
@@ -12,6 +14,24 @@ var MOCK_SERVICE_PORT_TLS = +(options.baseUrlTls.match(/:(\d+)/)[1]);
 var server;
 var serverTls;
 
+function generateCertOptions() {
+  var keypair = selfSigned({
+    name: 'localhost',
+    city: 'Chicago',
+    state: 'Illinois',
+    organization: 'Test',
+    unit: 'Test',
+  }, {
+    alt: ['127.0.0.1', 'http://localhost'],
+    expire: 60 * 60 * 1000, // one hour
+  });
+  return {
+    cert: keypair.cert,
+    key: keypair.private,
+  };
+}
+var certOptions = generateCertOptions();
+
 function sendEcho(req, res) {
   var chunks = [];
   var query = parseUrl(req.url, true).query;
@@ -96,7 +116,7 @@ function handleRequest(req, res) {
     res.setHeader('Access-Control-Allow-Origin', req.headers.origin);
   }
   res.setHeader('Access-Control-Allow-Methods', 'GET, POST, PUT, HEAD, OPTIONS, DELETE, PATCH');
-  res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization');
+  res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization, x-a, x-b');
 
   // Preflight requests that return a 404 confuse Chrome
   if (req.method === 'OPTIONS') return res.end();
@@ -123,15 +143,10 @@ function bootupServers(done) {
   }
   server = http.createServer(handleRequest);
   server.on('error', done);
-  server.listen(MOCK_SERVICE_PORT, onListen);
-  var certOptions = {
-    key: fs.readFileSync('test/certs/server/my-server.key.pem'),
-    ca: [fs.readFileSync('test/certs/server/my-root-ca.crt.pem')],
-    cert: fs.readFileSync('test/certs/server/my-server.crt.pem'),
-  };
+  server.listen(MOCK_SERVICE_PORT, '127.0.0.1', onListen);
   serverTls = https.createServer(certOptions, handleRequest);
   serverTls.on('error', done);
-  serverTls.listen(MOCK_SERVICE_PORT_TLS, onListen);
+  serverTls.listen(MOCK_SERVICE_PORT_TLS, '127.0.0.1', onListen);
 }
 
 if (typeof before === 'function') {
@@ -163,4 +178,6 @@ if (process.mainModule === module) {
   });
 }
 
+// Exposed for the https test so we can set up a compatible client
+options.certOptions = certOptions;
 module.exports = options;