Commit 7074ae3
test: allow half-open CONNECT tunnel sockets
CONNECT tunnels are full-duplex. When the upstream socket receives a
FIN while the client-to-upstream pipe is still draining, the default
socket behavior can produce EPIPE or ECONNRESET errors.
Keep the upstream socket half-open so both directions can drain.
Separate request logs from transport errors to verify that teardown
completes without errors.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
PR-URL: #64973
Refs: https://github.com/nodejs/reliability/issues?q=%22test-https-proxy-request-invalid-char-in-url%22
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>1 parent 7f31f62 commit 7074ae3
2 files changed
Lines changed: 12 additions & 14 deletions
Lines changed: 4 additions & 13 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -82,19 +82,10 @@ for (const testCase of testCases) { | |||
| 82 | 82 | proxy.close(); | |
| 83 | 83 | server.close(); | |
| 84 | 84 | assert.deepStrictEqual(requests, expectedUrls); | |
| 85 | - const logSet = new Set(logs); | ||
| 86 | - for (const log of logSet) { | ||
| 87 | - if (log.source === 'proxy connect' && log.error?.code === 'EPIPE') { | ||
| 88 | - // There can be a race from eagerly shutting down the servers and severing | ||
| 89 | - // two pipes at the same time but for the purpose of this test, we only | ||
| 90 | - // care about whether the requests are initiated from the client as expected, | ||
| 91 | - // not how the upstream/proxy servers behave. Ignore EPIPE errors from them.. | ||
| 92 | - // Refs: https://github.com/nodejs/node/issues/59741 | ||
| 93 | - console.log('Ignoring EPIPE error from proxy connect', log.error); | ||
| 94 | - logSet.delete(log); | ||
| 95 | - } | ||
| 96 | - } | ||
| 97 | - assert.deepStrictEqual(logSet, expectedProxyLogs); | ||
| 85 | + const requestLogs = logs.filter((log) => !('error' in log)); | ||
| 86 | + const errors = logs.filter((log) => 'error' in log); | ||
| 87 | + assert.deepStrictEqual(new Set(requestLogs), expectedProxyLogs); | ||
| 88 | + assert.deepStrictEqual(errors, []); | ||
| 98 | 89 | })); | |
| 99 | 90 | } | |
| 100 | 91 | })); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -80,7 +80,14 @@ function createProxyServer(options = {}) { | |||
| 80 | 80 | ||
| 81 | 81 | const normalizedHostname = hostname.startsWith('[') && hostname.endsWith(']') ? | |
| 82 | 82 | hostname.slice(1, -1) : hostname; | |
| 83 | - const proxyReq = net.connect(port, normalizedHostname, () => { | ||
| 83 | + // A CONNECT tunnel is full-duplex. Keep the upstream socket writable after | ||
| 84 | + // receiving a FIN so that the client-to-upstream pipe can finish draining. | ||
| 85 | + // The reverse pipe will end `res`, and `res` will in turn end `proxyReq`. | ||
| 86 | + const proxyReq = net.connect({ | ||
| 87 | + port, | ||
| 88 | + host: normalizedHostname, | ||
| 89 | + allowHalfOpen: true, | ||
| 90 | + }, () => { | ||
| 84 | 91 | res.write( | |
| 85 | 92 | 'HTTP/1.1 200 Connection Established\r\n' + | |
| 86 | 93 | 'Proxy-agent: Node.js-Proxy\r\n' + | |
0 commit comments