test: handle plain HTTP proxy requests in the PROXY auth mock#869
Open
GiHoon1123 wants to merge 1 commit into
Open
test: handle plain HTTP proxy requests in the PROXY auth mock#869GiHoon1123 wants to merge 1 commit into
GiHoon1123 wants to merge 1 commit into
Conversation
Node 26.5.0 bundles Undici 8.7.0, which changed how it proxies
plain http:// requests: instead of always tunneling through CONNECT,
it now sends them directly to the proxy with an absolute-URI request
line, per the standard HTTP proxying behavior.
The AUTH_TYPE=PROXY test mock only ever handled CONNECT (used for the
tunneling case) and unconditionally replied "okay" to any other
request. Under Node 26.5.0 this "okay" body got parsed as registry
JSON/tarball data, failing with "Unexpected token 'o', "okay" is not
valid JSON" or TAR_BAD_ARCHIVE.
Parse the absolute-URI request line, and for requests to the test's
mock host forward them into the existing mock registry server's own
request handler (reusing its logic via `server.emit('request', ...)`
rather than duplicating it) instead of replying with a stub. The
existing CONNECT handling is untouched.
Verified: reverting this fix and running under Node 26.5.0 reproduces
the exact reported failures (TAR_BAD_ARCHIVE / JSON parse error) in
all 4 PROXY-mode tests; with the fix, all 4 pass. Full main.test.ts
suite (114 tests) passes identically under both Node 22 and 26.5.0.
Fixes: nodejs#868
Contributor
|
Thanks for submitting this PR to resolve the issue #868 I submitted! 👍🏻 I successfully ran the workflow in my fork, so I expect if a maintainer approves the workflow from your PR, it will also succeed and be ready to merge. |
This file contains hidden or 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
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.
Fixes #868.
Problem
The
AUTH_TYPE=PROXYtest suite fails on Node.js 26.5.0 with errors like:and, for tarball downloads:
Root cause
Node 26.5.0 bundles Undici 8.7.0, which changed how
http://(non-TLS) requests are proxied: instead of always tunneling throughCONNECT, they're now sent directly to the proxy with an absolute-URI request line (GET http://example.com/... HTTP/1.1) — the standard way HTTP proxying works for plain HTTP.The test's mock proxy server (
tests/_registryServer.mjs) only ever implemented theCONNECTpath (for tunneling) and unconditionally replied"okay"to any other request it received. Once Undici started sending plain proxied HTTP requests for the mock registry, Corepack tried to parse that"okay"body as registry JSON or as a package tarball, producing the errors above.Fix
Parse the absolute-URI request line with
new URL(req.url). If it targets the test's mock registry host, rewritereq.urlto a relative path and forward the request into the existing mock registry server's own request handler viaserver.emit('request', req, res)— reusing its logic instead of duplicating it — rather than replying with the"okay"stub. Anything else gets a 404, matching the existingCONNECThandler's behavior of rejecting requests to any other host. TheCONNECThandling itself is untouched, since it's still exercised (e.g. by older Node/Undici versions, or wherever a real TLS tunnel is needed).Testing
process.versions.undiciis8.7.0, matching the issue), reverted this fix, and ran thePROXYtest group — got the exact sameTAR_BAD_ARCHIVE/ JSON parse errors described in the issue, across all 4 tests in that group.PROXYtests pass under Node 26.5.0.tests/main.test.tssuite (114 tests) under both Node 22 and Node 26.5.0 — identical results on both (113 passed, 1 expected-fail viait.fails, unrelated to this change), confirming no regressions to the otherAUTH_TYPEvariants or other tests.