Skip to content

Close the CONNECT tunnel when a proxied socket is closed - #253

Open
ekmartin wants to merge 2 commits into
socketry:mainfrom
ekmartin:ek-conductor/async-http-proxy-fix
Open

ekmartin wants to merge 2 commits into
socketry:mainfrom
ekmartin:ek-conductor/async-http-proxy-fix

Conversation

@ekmartin

@ekmartin ekmartin commented Sep 30, 2026 •

Copy link
Copy Markdown

Problem

When Proxy#connect is called without a block, it returns pipe.to_io and drops the Body::Pipe. Proxied endpoints always connect this way, through SSLEndpoint#connect and the client connection pool, so Pipe#close never runs. Closing the returned socket only half-closes the CONNECT request body. The pipe's reader keeps reading the CONNECT response until the proxy closes its end. Until then the proxy client's connection stays leased, and Client#close blocks with "Waiting for … pool to drain".

Changes

  • Closing the returned socket closes the tunnel. Proxy#connect without a block now returns the pipe's socket extended with Proxy::Tunnel. Its close calls the new Body::Pipe#finish, which stops the reader (closing the CONNECT response) and closes the socket. The pipe's writer then forwards any data already written to the socket and ends the CONNECT request normally. HTTP/2 then resets the stream with RST_STREAM(NO_ERROR), via the orderly-shutdown path from Preserve orderly HTTP/2 duplex shutdown. #248, and HTTP/1 closes the connection, which can't be reused after a tunnel anyway. It stays a real Socket because OpenSSL::SSL::SSLSocket requires one.
  • The block form uses Pipe#finish too. Previously it called Pipe#close, which stopped the writer and discarded data written just before the block exited.
  • ConnectFailure#body exposes up to 8 KiB of the proxy's response body, since proxies such as smokescreen put the rejection reason there. I/O and protocol errors while reading it are ignored. Anything else, such as Async::TimeoutError, propagates to the caller.

Notes for review

  • Closing now completes a scheduler tick or so later. The pipe's writer, then the HTTP/1 tunnel task or HTTP/2 output task, have to forward the remaining data before the connection is released. A Client#close on the proxy client immediately afterwards may briefly log "Waiting for … pool to drain", as it already does on main, but it no longer waits for the proxy to close its end.
  • A proxy that stops reading can delay the release. If the proxy stops accepting data (HTTP/2 flow control, or TCP backpressure on HTTP/1), forwarding the pending data waits until it does.
  • Reading ConnectFailure#body can block. For example, an HTTP/1 error response with no content-length on a connection the proxy keeps open. It is bounded only by the caller's own timeout, and this is documented on ConnectFailure#body.
  • Not addressed: after a rejected CONNECT, the HTTP/1 pool reports busy for one scheduler tick. This already happens on main.

🤖 Generated with Claude Code

ekmartin and others added 2 commits September 30, 2026 00:20
Closing the socket returned by `Proxy#connect` without a block only
half-closed the CONNECT request body, so the proxy client's connection
stayed leased (and `Client#close` blocked) until the proxy closed its end
of the tunnel. Closing it now stops the pipe and closes the CONNECT request
and response with an error. The block form closes the same way.

`ConnectFailure#body` also exposes up to 8 KiB of the proxy's response body.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Closing the tunnel stopped the pipe's writer and closed the CONNECT request
with an error, discarding any data written to the socket that hadn't been
forwarded yet (e.g. `QUIT\r\n` followed immediately by `close`). Instead,
stop only the reader and close the socket, so the writer forwards the
remaining data and ends the request normally. HTTP/2 then resets the stream
with `NO_ERROR` and HTTP/1 closes the connection. The block form closes the
same way.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ekmartin

Copy link
Copy Markdown
Author

🤖 This comment was written by Claude (Claude Code) on behalf of @ekmartin.

Fixed in 351ef6d: closing a tunnel no longer drops pending writes.

Problem: writing QUIT\r\n and closing the socket straight away delivered zero bytes on HTTP/1.1 and HTTP/2, while the base revision delivered all six. Tunnel#close stopped the pipe's writer and closed the CONNECT request with an error. That discarded anything still sitting in the socket, and anything already queued in the request body, because Writable#read raises the stored error before returning queued chunks. The block form's pipe.close on main had the same problem for data written just before the block exited.

What changed:

  • New Body::Pipe#finish: it stops only the reader, which closes the CONNECT response, and then closes the socket. The writer forwards whatever is left and ends the CONNECT request normally. After that, HTTP/2 resets the stream with RST_STREAM(NO_ERROR), using the orderly-shutdown path from Preserve orderly HTTP/2 duplex shutdown. #248, and HTTP/1 closes the connection.
  • Both forms use it: Tunnel#close and the block form of Proxy#connect both call finish.
  • The error-based close is gone: Tunnel::Closed and Pipe#close(error) are removed, so HTTP/2 proxies no longer see an INTERNAL_ERROR reset. Pipe#close is unchanged from main.
  • Tests:
    • a new test writes QUIT\r\n and closes immediately, for both forms on HTTP/1.0, HTTP/1.1 and HTTP/2 (it failed 6 of 6 before the fix);
    • a new unit test checks that Pipe#finish forwards buffered data.

Trade-offs:

  • The connection is released a tick or two later, once the pending data has been forwarded. A Client#close on the proxy client straight afterwards may briefly log "Waiting for … pool to drain", as it already does on main, but it no longer waits for the proxy to close its end.
  • A proxy that stops reading can delay the release. With HTTP/2 flow control or TCP backpressure, the release waits until the proxy reads the pending data. The previous version avoided this by discarding the data.

I've updated the PR description to match.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant