with http mocked out: Wrong host, port, and Host header extracted from concurrent requests to different hosts #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
papandreou
force-pushed
the
fail/concurrentRequestsToDifferentHosts
branch
from
645c4bf to
c411666
Compare
|
@alexjeffburke This is the fix I had in mind. |
Use req.connection._mitm.{client,opts} instead of trying to keep track of the last hijacked socket and options. Fixes failing test.
papandreou
force-pushed
the
fail/concurrentRequestsToDifferentHosts
branch
from
c411666 to
6c3c53c
Compare
|
@alexjeffburke This problem bit me again, so I forked mitm once more and rebased this branch. I would like to include this in 9.2.0. |
|
Should be ready to merge if CI is green. |
|
Like that this both fixes a nasty bug and very much cleans things up - |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the failing test.
It happens because
lastHijackedSocketandlastHijackedSocketOptions, which are saved in theconnecthandler here aren't necessarily paired up with the values that are used in therequesthandler here.This is somewhat predictable, and I raised the issue about a year and a half ago without it getting too much attention: moll/node-mitm#14
We'll probably need to get that implemented somehow so we can get rid of all the
lastHijackedSocket... hacks.