Merge pull request #19 from unexpectedjs/fail/concurrentRequestsToDif…

…ferentHosts with http mocked out: Wrong host, port, and Host header extracted from concurrent requests to different hosts
unexpectedjs · Aug 25, 2016 · ce1572b · ce1572b
2 parents e0b9eed + 6c3c53c
commit ce1572b
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 27 deletions.
diff --git a/lib/unexpectedMitm.js b/lib/unexpectedMitm.js
@@ -1,6 +1,6 @@
 /* global setImmediate, process, after, console */
 var messy = require('messy'),
-    createMitm = require('mitm'),
+    createMitm = require('mitm-papandreou'),
     _ = require('underscore'),
     http = require('http'),
     https = require('https'),
@@ -615,23 +615,20 @@ module.exports = {
                 recordedExchanges = [];
 
             return expect.promise(function (resolve, reject) {
-                var bypassNextConnect = false,
-                    lastHijackedSocket,
-                    lastHijackedSocketOptions;
+                var bypassNextConnect = false;
 
                 mitm.on('connect', function (socket, opts) {
                     if (bypassNextConnect) {
                         socket.bypass();
                         bypassNextConnect = false;
-                    } else {
-                        lastHijackedSocket = socket;
-                        lastHijackedSocketOptions = opts;
                     }
                 }).on('request', createSerializedRequestHandler(function (req, res) {
+                    var clientSocket = req.connection._mitm.client;
+                    var clientSocketOptions = req.connection._mitm.opts;
                     var metadata = _.extend(
                             {},
-                            _.pick(lastHijackedSocketOptions.agent && lastHijackedSocketOptions.agent.options, metadataPropertyNames),
-                            _.pick(lastHijackedSocketOptions, metadataPropertyNames)
+                            _.pick(clientSocketOptions.agent && clientSocketOptions.agent.options, metadataPropertyNames),
+                            _.pick(clientSocketOptions, metadataPropertyNames)
                         ),
                         recordedExchange = {
                             request: _.extend({
@@ -689,7 +686,7 @@ module.exports = {
                             });
                         }).caught(function (err) {
                             recordedExchange.response = err;
-                            lastHijackedSocket.emit('error', err);
+                            clientSocket.emit('error', err);
                         });
                     });
                 }));
@@ -762,29 +759,26 @@ module.exports = {
 
                 var assertionPromise = expect.promise(function (resolve, reject) {
                     var httpConversation = new messy.HttpConversation(),
-                        httpConversationSatisfySpec = {exchanges: []},
-                        lastHijackedSocket,
-                        lastHijackedSocketOptions;
+                        httpConversationSatisfySpec = {exchanges: []};
 
                     __lastError = null;
 
-                    mitm.on('connect', function (socket, opts) {
-                        lastHijackedSocket = socket;
-                        lastHijackedSocketOptions = opts;
-                        if (typeof lastHijackedSocketOptions.port === 'string') {
+                    mitm.on('request', createSerializedRequestHandler(function (req, res) {
+                        var clientSocket = req.connection._mitm.client;
+                        var clientSocketOptions = req.connection._mitm.opts;
+                        if (typeof clientSocketOptions.port === 'string') {
                             // The port could have been defined as a string in a 3rdparty library doing the http(s) call, and that seems to be valid use of the http(s) module
-                            lastHijackedSocketOptions = _.defaults({
-                                port: parseInt(lastHijackedSocketOptions.port, 10)
-                            }, lastHijackedSocketOptions);
+                            clientSocketOptions = _.defaults({
+                                port: parseInt(clientSocketOptions.port, 10)
+                            }, clientSocketOptions);
                         }
-                    }).on('request', createSerializedRequestHandler(function (req, res) {
                         var currentDescription = requestDescriptions.shift(),
                             hasRequestDescription = !!currentDescription,
                             metadata =
                                 _.defaults(
                                     { encrypted: Boolean(res.connection.encrypted) },
-                                    _.pick(lastHijackedSocketOptions, messy.HttpRequest.metadataPropertyNames),
-                                    _.pick(lastHijackedSocketOptions && lastHijackedSocketOptions.agent && lastHijackedSocketOptions.agent.options, messy.HttpRequest.metadataPropertyNames)
+                                    _.pick(clientSocketOptions, messy.HttpRequest.metadataPropertyNames),
+                                    _.pick(clientSocketOptions && clientSocketOptions.agent && clientSocketOptions.agent.options, messy.HttpRequest.metadataPropertyNames)
                                 ),
                             requestDescription = currentDescription,
                             requestProperties,
@@ -830,7 +824,7 @@ module.exports = {
                                  */
 
                                 // cancel the delegated assertion
-                                lastHijackedSocket.emit('error', new Error('unexpected-mitm: Saw unexpected requests.'));
+                                clientSocket.emit('error', new Error('unexpected-mitm: Saw unexpected requests.'));
                                 // continue with current assertion
                                 resolve([null, httpConversation, httpConversationSatisfySpec]);
                                 return [null, null];
@@ -895,7 +889,7 @@ module.exports = {
                             __lastError = e;
                             // cancel the delegated assertion
                             try {
-                                lastHijackedSocket.emit('error', e);
+                                clientSocket.emit('error', e);
                             } finally {
                                 /*
                                  * If an something was thrown trying to signal
@@ -949,7 +943,7 @@ module.exports = {
                                 }
                                 if (mockResponseError) {
                                     setImmediate(function () {
-                                        lastHijackedSocket.emit('error', mockResponseError);
+                                        clientSocket.emit('error', mockResponseError);
                                         assertMockResponse(mockResponse, mockResponseError);
                                     });
                                 } else {

diff --git a/package.json b/package.json
@@ -48,7 +48,7 @@
     "detect-indent": "3.0.0",
     "memoizesync": "0.5.0",
     "messy": "^6.6.1",
-    "mitm": "1.3.0",
+    "mitm-papandreou": "1.3.0-patch1",
     "underscore": "1.7.0",
     "unexpected-messy": "^6.0.0"
   },

diff --git a/test/unexpectedMitm.js b/test/unexpectedMitm.js
@@ -2012,4 +2012,25 @@ describe('unexpectedMitm', function () {
             );
         });
     });
+
+    it('should handle concurrent requests without confusing the Host headers', function () {
+        return expect(function () {
+            return expect.promise(function (resolve, reject) {
+                var urls = ['http://www.google.com/', 'http://www.bing.com/'];
+                var numInFlight = 0;
+                urls.forEach(function (url) {
+                    numInFlight += 1;
+                    issueGetAndConsume(url, function () {
+                        numInFlight -= 1;
+                        if (numInFlight === 0) {
+                            resolve();
+                        }
+                    });
+                });
+            });
+        }, 'with http mocked out', [
+            { request: { host: 'www.google.com', headers: { Host: 'www.google.com' } }, response: 200 },
+            { request: { host: 'www.bing.com', headers: { Host: 'www.bing.com' } }, response: 200 }
+        ], 'not to error');
+    });
 });