Replace http.request() with raw TCP/TLS sockets for CONNECT tunneling#663
Draft
jancurn wants to merge 2 commits into
Draft
Replace http.request() with raw TCP/TLS sockets for CONNECT tunneling#663jancurn wants to merge 2 commits into
jancurn wants to merge 2 commits into
Conversation
The previous self-signed cert expired 2018-11-10. Node tolerated it because the tests pass `rejectUnauthorized: false`, but Bun rejects expired certs strictly on the WebSocket path, failing four e2e tests with "certificate has expired". New cert: sha256, SAN covers localhost, localhost-test, 127.0.0.1 and ::1, valid until 2126.
Three changes that make proxy-chain work as a library under Bun 1.3, inspired by the approach in #649: 1. `chain.ts` no longer tunnels through `http.request().on('connect')`. Bun implements `http.request` on top of `fetch()`, which (a) rejects RFC 7230 authority-form CONNECT paths like `:443` with "fetch() URL is invalid" and (b) swallows 407/590-class upstream responses. Instead we open a raw `net`/`tls` socket, write the CONNECT request ourselves, parse the response headers, and `unshift` any remaining bytes back as tunnel payload. The synthetic response object keeps `rawHeaders` so `tunnelConnectResponded`/`tunnelConnectFailed` subscribers see the same shape as before. TLS options configured on `httpsAgent` (custom `ca`, client certs) are honored by re-using the agent's stored constructor options for the direct connection. 2. `Server.onRequest` routes `CONNECT` requests to `onConnect`: Bun delivers CONNECT through the generic 'request' event for some client shapes instead of the dedicated 'connect' event. 3. `Server.getConnectionStats` returns `undefined` under Bun. Bun does not populate `socket.bytesRead`/`bytesWritten` on http sockets, so the stats would silently read as zero — better no stats than wrong stats. (`connectionClosed` events consequently carry `stats: undefined` on Bun.) Node behavior is unchanged: full e2e suite produces an identical pass/fail set to master baseline (2012 passing, same 174 pre-existing environment failures) on Node 22.
ea65934 to
e76b96b
Compare
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.
Summary
Refactors the upstream proxy CONNECT tunnel implementation to use raw TCP/TLS sockets instead of
http.request()with the 'connect' event, making proxy-chain work as a library under Bun 1.3. Rebased onto latest master (pnpm migration, puppeteer 25).Key Changes
Chain handler refactoring (
src/chain.ts): Replacedhttp.request()/https.request()with directnet.createConnection()andtls.connect()callsrawHeaders, sotunnelConnectResponded/tunnelConnectFailedsubscribers see the same shape as beforehttpsAgent(customca, client certs, ...) are honored by re-using the agent's stored constructor options for the direct connection:443with "fetch() URL is invalid", and silent swallowing of 407/590-class upstream responsesServer request routing (
src/server.ts):onRequestroutesCONNECTrequests toonConnect()— Bun delivers CONNECT through the generic 'request' event for some client shapes instead of the dedicated 'connect' eventConnection stats on Bun (
src/server.ts):getConnectionStats()returnsundefinedunder Bun. Bun does not populatesocket.bytesRead/bytesWrittenon http sockets, so the numbers would silently read as zero — better no stats than wrong stats. (connectionClosedevents consequently carrystats: undefinedon Bun.) An earlier revision of this PR tried to reimplement byte counting via passive'data'/'pipe'listeners; that added per-connection listener overhead on all runtimes for little gain and was dropped in favor of this explicit guard.SSL certificate regeneration (
test/e2e/ssl.crt,ssl.key): The previous self-signed test cert expired 2018-11-10. Node tolerated it (rejectUnauthorized: false), but Bun rejects expired certs strictly on the WebSocket path. New cert: sha256, SAN coverslocalhost,localhost-test,127.0.0.1,::1, valid until 2126.Verification
compatiblee2e subset pass; full-suite pass count improves, with the remainder blocked on upstream Bunhttp.requestclient bugs (tracked separately)https://claude.ai/code/session_01Vn16UCyisJSwEaAoHLSNX6