Repository navigation
In a terminated tunnel, an HTTP/1.0 client gets the whole answer - #704
Merged
Merged
Conversation
…S socket Under Bun the TLS socket runs on a stream over the tunnel's socket, and has finished once it has written there, long before the client has it all. Destroyed then, it took the stream and the socket with it, and what was queued was dropped. Bun 1.4.3's HTTP server does that after every HTTP/1.0 response (destroySoon): the client lost the tail of a long answer, with no error where there is no Content-Length. A destroy with no error, once the TLS socket has finished, now counts as the stream's end, and the client is given all it holds. And once the stream has handed the socket to closeAfterFlush, its own destruction leaves the socket alone: that cut the close short, and a client with bytes nobody had read lost what the kernel still held for it.
… cuts again The commit before this one counted every destroy() with no error, once the TLS socket had finished, as the end of the stream under it. That cured the lost tail of an HTTP/1.0 answer, but it softened the proxy's own back-stops, which destroy a tunnel a second after ending it: with no mux in front (srt proxy) a refused client that never closes was held for 5 to 6 s, not 1 s. The two calls mean different things. destroy() is to cut now. destroySoon() is to destroy once all that was written is out, and under Bun it believes that too early, because the TLS socket has finished as soon as it has written to the stream under it. So destroySoon() now ends that socket, which closes the tunnel after the flush, and the stream is as it was.
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.
In a TLS-terminated tunnel, an HTTP/1.0 client could lose the end of a long answer. With a
Content-Length,curlsays(18) transfer closed with N bytes remaining. Without one, the file is short and nothing says so.In no release: it came with in-process termination (#666). Bun 1.4.3 shows it. Bun 1.4.2, which CI runs, and Node do not.
Why
Under Bun the TLS socket runs on a stream of ours over the tunnel's socket. It has finished once it has written into that stream, long before the client has the bytes.
Bun 1.4.3's HTTP server ends an HTTP/1.0 response with
end(), thendestroySoon(), which is to destroy the socket once all that was written is out. Here it believes that at once. Traced, for one answer of 5 MiB: 311,738 bytes still queued at the destroy, and 4,915,200 of 5,242,880 delivered. After HTTP/1.1 withConnection: closethe server callsend()alone, and the close waits for the flush.What changes
One line, in
serverTlsOn: on that TLS socket,destroySoon()ends it. The tunnel then closes as it does afterConnection: close, once the client has everything.destroy()still cuts at once. The first commit here did it otherwise: it counted everydestroy()with no error as an end. That softened the proxy's own back-stops, which destroy a tunnel a second after ending it. With no mux in front (srt proxy), a refused client that never closes was held for 5 to 6 s, where main lets it go after 1 s. The second commit takes that back; the stream is main's again.What it costs
An HTTP/1.0 client is now treated as one of HTTP/1.1 with
Connection: closealready is:A client that closes when it has the answer, as
curldoes, sees no difference.Testing
curl --http1.0through the public export: 1, 5 and 20 MiB, with and without aContent-Length, 8 at once. Whole answers:Connection: closeand keep-alivemaxTunnels: 4: a new CONNECT gets 503, then 200 from 303 to 363 s on.serverTlsOn, so each runtime brings its own order of events.destroySoon(), with and without anend()first: the client gets every byte. Both fail on main under Bun 1.4.2 too.destroy(), at once and once the socket has finished: cut. The second fails on this PR's first commit.