elazarl
Avoid creating bad URLs when a mitm request has the wrong scheme
If a mitm request came in like http://host/path this was getting mangled to https://hosthttp://host/path. Instead, we should just swap the scheme to https if the scheme is already defined in the url.
We're still investigating this & working on a test but have been observing that the npm cli is not always working with the proxy in mitm mode.
When running npm install [email protected] on a fresh project we've seen these requests to the proxy:
CONNECT registry.npmjs.org:443 HTTP/1.1
GET http://registry.npmjs.org:443/npm HTTP/1.1
This proxy then detects the url does not match the https regex and transforms it to https://registry.npmjs.org:443http://registry.npmjs.org:443/npm. This eventually fails dns resolution later on and closes the connection without returning an error.
Other requests from the npm cli use https instead of http so this doesn't seem to be consistent and we haven't looked to see what the cause is yet.
I'm not sure what the right behavior is in this case. I've observed that mitmproxy v8.1.1 seems to be allowing this by converting the url to https.
but it shouldn't use GET http://... should it? If it really uses goproxy as a standard HTTP proxy
but it shouldn't use GET http://... should it? If it really uses goproxy as a standard HTTP proxy
It does seem strange but I'm not positive it's not allowed. I found this request does work successfully when I configure npm to use a squid proxy without mitm handling so registry.npmjs.org is not rejecting the request.
@jeffwidman I think we need to add a test case for this before undrafting.
We could also followup with the npm team on this behavior. I have a vague memory of running into this before.
This issue states that in npm 10.8.3 the problem has been resolved: https://github.com/dependabot/dependabot-core/issues/10623
Can you confirm that now it works and, if so, close this pull request? If it was an NPM problem, we don't need to change anything
No updates from the pull request author. I decided to close it, since npm states that they corrected this issue. If the issue is still present, please reopen it!
This was an NPM problem. From my coworker @jakecoffman:
This appears to be a bug in NPM 10 up until 10.5.0. I was able to reproduce the issue but when I bumped to 10.5.0 it was fixed.