ProxyTunnel: close-delimited responses via proxy cause ECONNRESET

[avarayr]

fixes: #23717

What does this PR do?

• Align ProxyTunnel.onClose with HTTPClient.onClose: when a tunneled HTTPS response is in-progress and either
  • parsing chunked trailers (trailer-line states), or
  • transfer-encoding is identity with content_length == null while in .body,
    treat EOF as end-of-message and complete the request, rather than ECONNRESET.
• Schedule proxy deref instead of deref inside callbacks to avoid lifetime hazards.

How did you verify your code works?

• test/js/bun/http/proxy.test.ts: raw TLS origin returns close-delimited 200 OK; verified no ECONNRESET and body delivered.
• Test suite passes under bun bd test.

Risk/compat

• Only affects CONNECT/TLS path. Direct HTTP/HTTPS unchanged. Behavior mirrors existing HTTPClient.onClose.

Repro (minimal)

See issue; core condition is no Content-Length and no Transfer-Encoding (close-delimited).

[coderabbitai]

Walkthrough

Handle proxy socket close when an HTTP response body is in-progress (close-delimited or chunked-with-trailers): mark last-chunk received, emit a proxy-specific progress update, adjust teardown to schedule deferred proxy derefs and avoid lifetime hazards, and add a test exercising HTTPS origin close-delimited responses through an HTTP proxy.

Changes

Cohort / File(s)	Summary of Changes
Proxy tunnel close semantics and progress updates src/http/ProxyTunnel.zig	Update onClose to detect in-progress responses (no Content-Length or chunked-with-trailers), set received_last_chunk, call new progressUpdateForProxySocket, schedule an asynchronous proxy-deref and return early; otherwise continue shutdown. Add progressUpdateForProxySocket helper and refactor proxy deref/teardown ordering to avoid lifetime hazards (remove unconditional defer, add deferred deref after teardown and an extra deref after detaching the proxy socket).
Test: HTTPS over HTTP proxy close-delimited response test/js/bun/http/proxy.test.ts	Add test "HTTPS origin close-delimited body via HTTP proxy does not ECONNRESET": spins up an inline TLS origin that replies close-delimited (no Content-Length), an HTTP proxy, issues a proxied POST, asserts 200 and body "ok", and ensures servers are cleaned up in a finally block.

Suggested reviewers

• cirospaciari
• Jarred-Sumner

Pre-merge checks

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "ProxyTunnel: close-delimited responses via proxy cause ECONNRESET" directly and specifically describes the main change in the pull request. It identifies the component being modified (ProxyTunnel), the problem being addressed (close-delimited responses causing ECONNRESET), and aligns with the changes visible in the file-level summary. The title is concise, clear, and provides meaningful context about the fix without extraneous details or vague language.
Linked Issues Check	✅ Passed	The PR successfully addresses all coding-related objectives from linked issue #23717. The implementation modifies ProxyTunnel.onClose to treat server-initiated connection close as end-of-message rather than ECONNRESET when close-delimited responses occur (matching Node.js and Deno behavior). A new test "HTTPS origin close-delimited body via HTTP proxy does not ECONNRESET" validates that requests to close-delimited origins succeed without raising ECONNRESET. The changes also schedule proxy deref asynchronously to avoid lifetime hazards, and the PR references the minimal repro from the linked issue.
Out of Scope Changes Check	✅ Passed	All changes in this PR are directly in-scope with the objectives from issue #23717. The modifications to ProxyTunnel.onClose specifically target close-delimited response handling via proxy, the new helper function progressUpdateForProxySocket supports the fix, and the test directly validates the fix for the reported issue. There are no incidental refactorings, formatting changes, or unrelated bug fixes that would suggest scope creep. The PR description explicitly notes that only the CONNECT/TLS path is affected, leaving direct HTTP/HTTPS unchanged.
Description Check	✅ Passed	The pull request description follows the repository's template by including both required sections: "What does this PR do?" and "How did you verify your code works?" The description is comprehensive, explaining the specific alignment with HTTPClient.onClose semantics, the conditions being handled (chunked trailers and identity transfer-encoding with null content-length), the test verification approach, and a risk assessment noting that only the CONNECT/TLS path is affected. The description also appropriately links to the issue being fixed.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3920f04 and f96540a.

📒 Files selected for processing (2)

• src/http/ProxyTunnel.zig (2 hunks)
• test/js/bun/http/proxy.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (10)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

• test/js/bun/http/proxy.test.ts

test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

• test/js/bun/http/proxy.test.ts

test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test for files ending with *.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests with describe blocks to group related tests
Use utilities like describe.each, toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach) and track resources for cleanup

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

For large/repetitive strings, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count)

Files:

• test/js/bun/http/proxy.test.ts

test/js/{bun,node}/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests by module under /test/js/bun/ and /test/js/node/

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must be created under test/ and end with .test.ts or .test.tsx
In tests that open network ports, always use port: 0; do not hardcode port numbers or roll your own random port
Use normalizeBunSnapshot to normalize snapshot output instead of comparing raw stdout
Do not write tests that merely assert absence of strings like "panic" or "uncaught exception"
Use tempDir from "harness" to create temporary directories; do not use fs.mkdtempSync or tmpdirSync
When spawning processes in tests, assert output before asserting the exit code
Avoid shell commands in tests (e.g., find, grep); prefer Bun's Glob and built-in tools
Prefer snapshot tests over direct string equality assertions on stdout

Files:

• test/js/bun/http/proxy.test.ts

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

• src/http/ProxyTunnel.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use private fields in Zig with the # prefix (e.g., struct { #foo: u32 };)
Prefer decl literals in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing @import statements at the bottom of the Zig file (formatter may reorder automatically)
Prefer @import("bun") rather than @import("root").bun or @import("../bun.zig")

In Zig code, be careful with allocators and use defer for cleanup

Files:

• src/http/ProxyTunnel.zig

🔇 Additional comments (3)

test/js/bun/http/proxy.test.ts (1)

305-339: LGTM! Test correctly validates the close-delimited proxy fix.

The test properly exercises the scenario from issue #23717: an HTTPS origin returning a close-delimited response (no Content-Length, connection: close) via HTTP proxy. The implementation follows all coding guidelines: uses port: 0, properly manages server lifecycle with once(), and ensures cleanup in the finally block.

src/http/ProxyTunnel.zig (2)

219-242: Correctly handles close-delimited responses in onClose.

The refactored logic properly mirrors HTTPClient.onClose semantics by treating connection close as EOF when:

1. Response is in progress (not done/fail/redirecting)
2. Either chunked encoding in trailer states OR identity transfer with no content-length in body stage

The ref-counting is balanced across all paths: the ref taken at line 218 is matched by scheduled async deref in early returns (lines 230, 240) or normal path (line 257). Scheduling deref asynchronously avoids lifetime hazards by ensuring the proxy isn't freed within the callback.

---

256-259: Proper async deref after detachment prevents lifetime issues.

Scheduling the deref asynchronously after socket detachment ensures the proxy isn't freed while still in use, avoiding potential use-after-free or double-free scenarios. This pattern is consistent with the early return paths above.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[avarayr]

cc: proxies codeowner @cirospaciari

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6197586 and 3920f04.

📒 Files selected for processing (2)

• src/http/ProxyTunnel.zig (2 hunks)
• test/js/bun/http/proxy.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (11)

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

• test/js/bun/http/proxy.test.ts

test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

• test/js/bun/http/proxy.test.ts

test/js/bun/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests

Files:

• test/js/bun/http/proxy.test.ts

test/js/bun/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place Bun-specific API tests under test/js/bun/

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test for files ending with *.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests with describe blocks to group related tests
Use utilities like describe.each, toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach) and track resources for cleanup

Files:

• test/js/bun/http/proxy.test.ts

test/**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

For large/repetitive strings, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count)

Files:

• test/js/bun/http/proxy.test.ts

test/js/{bun,node}/**

📄 CodeRabbit inference engine (test/CLAUDE.md)

Organize unit tests by module under /test/js/bun/ and /test/js/node/

Files:

• test/js/bun/http/proxy.test.ts

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

• src/http/ProxyTunnel.zig

src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use private fields in Zig with the # prefix (e.g., struct { #foo: u32 };)
Prefer decl literals in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing @import statements at the bottom of the Zig file (formatter may reorder automatically)
Prefer @import("bun") rather than @import("root").bun or @import("../bun.zig")

Files:

• src/http/ProxyTunnel.zig

🔇 Additional comments (3)

test/js/bun/http/proxy.test.ts (1)

305-339: Good regression coverage for close-delimited TLS via HTTP proxy

Test reliably reproduces and asserts the non-ECONNRESET behavior; lifecycle is properly cleaned up. LGTM.

src/http/ProxyTunnel.zig (2)

256-259: Async deref on close verified scheduleProxyDeref is implemented on HTTPThread and safe to call here, and onClose’s two exclusive branches each schedule exactly once before returning.

---

220-242: EOF-as-EOM logic covers all trailer states – picohttpparser only defines CHUNKED_IN_TRAILERS_LINE_HEAD and CHUNKED_IN_TRAILERS_LINE_MIDDLE, and ProxyTunnel mirrors HTTPClient’s handling.