From 7a28bc71a157b25167b283f22b81e6024a54071d Mon Sep 17 00:00:00 2001 From: Andreas Lind Date: Sun, 28 Jun 2020 02:41:31 +0200 Subject: [PATCH 1/2] Switch back to the official mitm --- lib/UnexpectedMitmMocker.js | 2 +- lib/UnexpectedMitmRecorder.js | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/UnexpectedMitmMocker.js b/lib/UnexpectedMitmMocker.js index 8da920d..e6d6004 100644 --- a/lib/UnexpectedMitmMocker.js +++ b/lib/UnexpectedMitmMocker.js @@ -1,5 +1,5 @@ const _ = require('underscore'); -const createMitm = require('mitm-papandreou'); +const createMitm = require('mitm'); const http = require('http'); const https = require('https'); const messy = require('messy'); diff --git a/lib/UnexpectedMitmRecorder.js b/lib/UnexpectedMitmRecorder.js index b68c3ff..87115ab 100644 --- a/lib/UnexpectedMitmRecorder.js +++ b/lib/UnexpectedMitmRecorder.js @@ -1,6 +1,6 @@ const _ = require('underscore'); const messy = require('messy'); -const createMitm = require('mitm-papandreou'); +const createMitm = require('mitm'); const consumeReadableStream = require('./consumeReadableStream'); const createSerializedRequestHandler = require('./createSerializedRequestHandler'); diff --git a/package.json b/package.json index 6f35fe2..7e498ab 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "detect-indent": "^6.0.0", "memoizesync": "^1.1.1", "messy": "^6.16.0", - "mitm-papandreou": "^1.7.1-patch1", + "mitm": "^1.7.1", "underscore": "^1.8.3", "unexpected-messy": "^9.0.0" }, From 7aa1d9b143b88c84297b7dd6cc4609d095f08c95 Mon Sep 17 00:00:00 2001 From: Andreas Lind Date: Sun, 28 Jun 2020 02:49:03 +0200 Subject: [PATCH 2/2] Use @jazko's workaround from https://github.com/moll/node-mitm/issues/14#issuecomment-437388337 --- lib/UnexpectedMitmMocker.js | 415 +++++++++++++++++----------------- lib/UnexpectedMitmRecorder.js | 10 +- 2 files changed, 221 insertions(+), 204 deletions(-) diff --git a/lib/UnexpectedMitmMocker.js b/lib/UnexpectedMitmMocker.js index e6d6004..116bf71 100644 --- a/lib/UnexpectedMitmMocker.js +++ b/lib/UnexpectedMitmMocker.js @@ -292,223 +292,234 @@ class UnexpectedMitmMocker { } return new Promise((resolve, reject) => { - mitm.on( - 'request', - createSerializedRequestHandler((req, res) => { - if (!res.connection) { - // I've observed some cases where keep-alive is - // being used and we end up with an extra "phantom - // request event" even though only requeest is being - // issued. Seems like a bug in mitm. - // It goes without saying that we should try to get - // rid of this. Hopefully something will happen - // on https://github.com/moll/node-mitm/pull/36 - return; - } - const clientSocket = req.connection._mitm.client; - let 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 - clientSocketOptions = _.defaults( - { - port: parseInt(clientSocketOptions.port, 10), - }, - clientSocketOptions - ); - } - const agent = - clientSocketOptions.agent || - (res.connection.encrypted ? https : http).globalAgent; - if (seenAgents.indexOf(agent) === -1) { - seenAgents.push(agent); - } - - const metadata = _.defaults( - { encrypted: Boolean(res.connection.encrypted) }, - _.pick( + mitm + .on('connect', (socket, opts) => { + // Store the client socket and connect options so they can be read in the request handler: + // https://github.com/moll/node-mitm/issues/14#issuecomment-437388337 + socket._handle.remote._mitm = { + clientSocket: socket, + clientSocketOptions: opts, + }; + }) + .on( + 'request', + createSerializedRequestHandler((req, res) => { + if (!res.connection) { + // I've observed some cases where keep-alive is + // being used and we end up with an extra "phantom + // request event" even though only requeest is being + // issued. Seems like a bug in mitm. + // It goes without saying that we should try to get + // rid of this. Hopefully something will happen + // on https://github.com/moll/node-mitm/pull/36 + return; + } + let { + clientSocket, clientSocketOptions, - messy.HttpRequest.metadataPropertyNames - ), - _.pick( - clientSocketOptions && - clientSocketOptions.agent && - clientSocketOptions.agent.options, - messy.HttpRequest.metadataPropertyNames - ) - ); - - let requestStruct; - let responseProperties; - let responseStruct; - - let __earlyExit = null; - - Promise.resolve() - .then(() => handleRequest(req, metadata)) - .then((result) => { - // make available for use further down the promise chain - requestStruct = result; - - return this.strategy - .nextDescriptionForIncomingRequest(requestStruct) - .catch((err) => { - if (err.name === 'EarlyExitError') { - __earlyExit = err; - const requestDescription = err.data; - // update the request with the spec it needs to satisfy - requestStruct.spec = resolveExpectedRequestProperties( - requestDescription && requestDescription.request - ); - return requestDescription; - } + } = req.connection._handle._mitm; + 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 + clientSocketOptions = _.defaults( + { + port: parseInt(clientSocketOptions.port, 10), + }, + clientSocketOptions + ); + } + const agent = + clientSocketOptions.agent || + (res.connection.encrypted ? https : http).globalAgent; + if (seenAgents.indexOf(agent) === -1) { + seenAgents.push(agent); + } + + const metadata = _.defaults( + { encrypted: Boolean(res.connection.encrypted) }, + _.pick( + clientSocketOptions, + messy.HttpRequest.metadataPropertyNames + ), + _.pick( + clientSocketOptions && + clientSocketOptions.agent && + clientSocketOptions.agent.options, + messy.HttpRequest.metadataPropertyNames + ) + ); - throw err; - }); - }) - .then((requestDescription) => { - if (requestStruct.error) { - // TODO: Consider adding support for recording this (the request erroring out while we're consuming it) - throw requestStruct.error; - } + let requestStruct; + let responseProperties; + let responseStruct; + + let __earlyExit = null; + + Promise.resolve() + .then(() => handleRequest(req, metadata)) + .then((result) => { + // make available for use further down the promise chain + requestStruct = result; + + return this.strategy + .nextDescriptionForIncomingRequest(requestStruct) + .catch((err) => { + if (err.name === 'EarlyExitError') { + __earlyExit = err; + const requestDescription = err.data; + // update the request with the spec it needs to satisfy + requestStruct.spec = resolveExpectedRequestProperties( + requestDescription && requestDescription.request + ); + return requestDescription; + } + + throw err; + }); + }) + .then((requestDescription) => { + if (requestStruct.error) { + // TODO: Consider adding support for recording this (the request erroring out while we're consuming it) + throw requestStruct.error; + } - if (!requestDescription) { - // there was no mock so arrange "" - assertExchange(requestStruct, null); - - // We wish to cause the generation of a diff - // so we arrange to enter our 'success' path - // but ensure the delegated assertion fully - // completes by signalling an error condition. - // - // Note the use of the single reject/resolve - // behaviour of promises: while the delegated - // assertion is reject()ed, we have already - // resolve()d thus the rejection of the former - // is effectively ignored and we proceed with - // our output. + if (!requestDescription) { + // there was no mock so arrange "" + assertExchange(requestStruct, null); + + // We wish to cause the generation of a diff + // so we arrange to enter our 'success' path + // but ensure the delegated assertion fully + // completes by signalling an error condition. + // + // Note the use of the single reject/resolve + // behaviour of promises: while the delegated + // assertion is reject()ed, we have already + // resolve()d thus the rejection of the former + // is effectively ignored and we proceed with + // our output. + + if (__earlyExit) { + throw __earlyExit; + } - if (__earlyExit) { - throw __earlyExit; + // cancel the delegated assertion + throw new errors.SawUnexpectedRequestsError( + 'unexpected-mitm: Saw unexpected requests.' + ); } - // cancel the delegated assertion - throw new errors.SawUnexpectedRequestsError( - 'unexpected-mitm: Saw unexpected requests.' + // update the request with the spec it needs to satisfy + requestStruct.spec = resolveExpectedRequestProperties( + requestDescription && requestDescription.request ); - } - - // update the request with the spec it needs to satisfy - requestStruct.spec = resolveExpectedRequestProperties( - requestDescription && requestDescription.request - ); - // set the response to be constructed based on the strategy - responseProperties = requestDescription.response; - - if (typeof responseProperties === 'function') { - // reset the readable req stream state - stream.Readable.call(req); - - // read stream data from the buffered chunks - req._read = function () { - this.push(requestStruct.chunks.shift() || null); - }; - - // call response function inside a promise to catch exceptions - return Promise.resolve() - .then(() => { - responseProperties(req, res); - }) - .then(() => ({ - response: null, - error: null, - })) - .catch(( - err // ensure delivery of the caught error to the underlying socket - ) => ({ - response: null, - error: err, - })); - } else { - return getMockResponse(responseProperties); - } - }) - .then((result) => { - responseStruct = result; - - if (!(responseStruct.response || responseStruct.error)) { - return; - } + // set the response to be constructed based on the strategy + responseProperties = requestDescription.response; + + if (typeof responseProperties === 'function') { + // reset the readable req stream state + stream.Readable.call(req); + + // read stream data from the buffered chunks + req._read = function () { + this.push(requestStruct.chunks.shift() || null); + }; + + // call response function inside a promise to catch exceptions + return Promise.resolve() + .then(() => { + responseProperties(req, res); + }) + .then(() => ({ + response: null, + error: null, + })) + .catch(( + err // ensure delivery of the caught error to the underlying socket + ) => ({ + response: null, + error: err, + })); + } else { + return getMockResponse(responseProperties); + } + }) + .then((result) => { + responseStruct = result; - if (__earlyExit) { - assertExchange(requestStruct, responseStruct); - throw __earlyExit; - } + if (!(responseStruct.response || responseStruct.error)) { + return; + } - return deliverMockResponse(responseStruct); - }) - .catch((e) => { - // Given an error occurs, the deferred assertion - // will still be pending and must be completed. We - // do this by signalling the error on the socket. - try { - clientSocket.emit('error', e); - } catch (e) { - // If an something was thrown trying to signal - // an error we have little choice but to simply - // reject the assertion. This is only safe with - // Unexpected 10.15.x and above. - } + if (__earlyExit) { + assertExchange(requestStruct, responseStruct); + throw __earlyExit; + } - timeline.push(e); - reject(e); - }); + return deliverMockResponse(responseStruct); + }) + .catch((e) => { + // Given an error occurs, the deferred assertion + // will still be pending and must be completed. We + // do this by signalling the error on the socket. + try { + clientSocket.emit('error', e); + } catch (e) { + // If an something was thrown trying to signal + // an error we have little choice but to simply + // reject the assertion. This is only safe with + // Unexpected 10.15.x and above. + } - function deliverMockResponse(responseStruct) { - const mockResponse = responseStruct.response; - const mockResponseError = responseStruct.error; - setImmediate(() => { - let nonEmptyMockResponse = false; - if (mockResponse) { - res.statusCode = mockResponse.statusCode; - mockResponse.headers.getNames().forEach((headerName) => { - mockResponse.headers.getAll(headerName).forEach((value) => { + timeline.push(e); + reject(e); + }); + + function deliverMockResponse(responseStruct) { + const mockResponse = responseStruct.response; + const mockResponseError = responseStruct.error; + setImmediate(() => { + let nonEmptyMockResponse = false; + if (mockResponse) { + res.statusCode = mockResponse.statusCode; + mockResponse.headers.getNames().forEach((headerName) => { + mockResponse.headers.getAll(headerName).forEach((value) => { + nonEmptyMockResponse = true; + res.setHeader(headerName, value); + }); + }); + const unchunkedBody = mockResponse.unchunkedBody; + if (unchunkedBody === null) { nonEmptyMockResponse = true; - res.setHeader(headerName, value); + res.write(Buffer.from('null')); + } else if ( + typeof unchunkedBody !== 'undefined' && + unchunkedBody.length > 0 + ) { + nonEmptyMockResponse = true; + res.write(unchunkedBody); + } else if (nonEmptyMockResponse) { + res.writeHead(res.statusCode || 200); + } + } + if (mockResponseError) { + setImmediate(() => { + clientSocket.emit('error', mockResponseError); + assertExchange(requestStruct, responseStruct); }); - }); - const unchunkedBody = mockResponse.unchunkedBody; - if (unchunkedBody === null) { - nonEmptyMockResponse = true; - res.write(Buffer.from('null')); - } else if ( - typeof unchunkedBody !== 'undefined' && - unchunkedBody.length > 0 - ) { - nonEmptyMockResponse = true; - res.write(unchunkedBody); - } else if (nonEmptyMockResponse) { - res.writeHead(res.statusCode || 200); + } else { + res.end(); } - } - if (mockResponseError) { - setImmediate(() => { - clientSocket.emit('error', mockResponseError); - assertExchange(requestStruct, responseStruct); - }); - } else { - res.end(); - } + }); + } + + // Hook the final write and immediately assert the request. + // Note this occurs prior to it being written on the wire. + observeResponse(res).then((rawBuffer) => { + const response = createMessyResponse(rawBuffer); + assertExchange(requestStruct, { response }); }); - } - - // Hook the final write and immediately assert the request. - // Note this occurs prior to it being written on the wire. - observeResponse(res).then((rawBuffer) => { - const response = createMessyResponse(rawBuffer); - assertExchange(requestStruct, { response }); - }); - }) - ); + }) + ); // handle synchronous throws let consumer; diff --git a/lib/UnexpectedMitmRecorder.js b/lib/UnexpectedMitmRecorder.js index 87115ab..e8045ea 100644 --- a/lib/UnexpectedMitmRecorder.js +++ b/lib/UnexpectedMitmRecorder.js @@ -51,6 +51,10 @@ class UnexpectedMitmRecorder { mitm .on('connect', (socket, opts) => { + socket._handle.remote._mitm = { + clientSocket: socket, + clientSocketOptions: opts, + }; if (bypassNextConnect) { socket.bypass(); bypassNextConnect = false; @@ -59,8 +63,10 @@ class UnexpectedMitmRecorder { .on( 'request', createSerializedRequestHandler((req, res) => { - const clientSocket = req.connection._mitm.client; - const clientSocketOptions = req.connection._mitm.opts; + const { + clientSocket, + clientSocketOptions, + } = req.connection._handle._mitm; const metadata = Object.assign( {}, _.pick(