Repository navigation
[pull] master from eycorsican:master - #35
Merged
Merged
Conversation
Every accepted inbound socket was given SO_LINGER = 0, so closing it sent a reset. That discards whatever is still queued for the peer: a response whose end is marked by the close itself -- HTTP/1.0, a body with no length, anything read to EOF -- can be truncated, and a peer that has received every byte but not yet read them can still see the transfer fail with a reset. What it bought was the socket back at once, with no TIME_WAIT. But TIME_WAIT falls on whoever closes first, and on the common paths that is the client: leaf is the passive closer and has no TIME_WAIT to avoid. The reclaim is only ever paid for on the paths where leaf closes first -- the idle timeouts and the error teardowns -- and those are the paths where a reset is the right answer anyway. So it is now off by default and available as TCP_INBOUND_ABORT_ON_CLOSE, for devices that cannot afford the sockets. Sessions are still bounded by the existing uplink and downlink idle timeouts.
A client that has finished sending says so by closing its write side and goes on reading, and plenty of protocols end a request that way and no other. Over a websocket transport that signal went nowhere: `poll_shutdown` returned without sending anything, so the far end learned the request had ended only when the connection itself went away -- when the downlink grace expires, ten seconds later, by which time the reply has nowhere to go. The comment above it said as much. A Close frame is the obvious candidate and the wrong one. RFC 6455 makes it the start of closing the whole connection, and tungstenite enforces that at both ends: once a Close has been received the connection will not carry another data frame, so the reply still on its way back is lost. Measured, not assumed -- the server read the origin's answer and then failed to forward it. An empty binary frame says the same thing and costs nothing. It is an ordinary data frame carrying no bytes, so a peer that does not understand it hands nothing to its application and carries on, which is exactly today's behaviour. This transport's read side has always turned a frame with no bytes into an end of stream, so between two leaf nodes it is precisely the missing signal. A write of no bytes now returns early rather than sending one by accident. Still behind WS_HALF_CLOSE, off by default, for the peer that would rather error than ignore it. Booleans now also accept 1/yes/on: an option that is silently ignored, as WS_HALF_CLOSE=1 was, is worse than one that is missing.
A certificate and its key are configured the same way and were read two different ways. `certificate` was recognised inline -- if it contains "-----BEGIN", it is the PEM itself -- and `certificateKey` never was: it went straight to the path branch, so an inline key became a filename made of PEM, joined onto the asset directory. What the operator saw was "no private keys found" about a key that was right there in the configuration. It affected the quic and tls settings on both sides, all four with their own copy of the same twelve lines. There is one resolver now, used for both halves of every keypair, and three tests that say which of the three shapes -- inline, absolute, relative -- goes where. Found by an end-to-end matrix that configures every stack from one self-signed certificate held in memory: with `rawCertificateKey` it worked, with `certificateKey` no quic node would start. Also: the quic inbound now reads a SEC1 key -- "BEGIN EC PRIVATE KEY", what openssl hands out for an elliptic curve by default. The tls inbound has always taken one. That path has no test here; the harness's key is PKCS#8, and the asymmetry is plain enough between the two files.
A chain has to answer three questions about every actor -- which of its two handlers runs, what address it is told to reach, and which actor the caller should dial before any of it starts -- and it answered them in the middle of doing the I/O. `next_connect_addr` served two of those questions at once with subtly different rules on the stream and datagram sides, recomputed per actor per connection; the datagram fold then made the same decisions again in five branches with a lookahead. Nothing said that actor zero is the one nearest the wire, or why only the last one is shown the client's stream. The rules now live in `outbound/plan.rs` as a function from actors to stages: one backward pass for what each suffix names, one forward fold for which handler each actor runs. It touches no sockets, so the seven tests under it are the first statement anywhere of what a chain is supposed to do -- that a transport leaves the address to what is inside it, that every actor but the last is told to reach the next hop, that a datagram stays a stream until the actor that tunnels it. The handlers are left with the I/O and with naming the actor in any error, which they never did. Inbound had the same fold written four times -- two handlers and two functions for the actors behind a multiplexed transport -- agreeing on the interesting parts and differing in the rest: one panicked with `unimplemented!()` where another returned, one timed out each actor and the others did not. One fold now, and the case that used to panic ends the chain instead. Behaviour is unchanged with two exceptions, both where the old code was indefensible: a datagram handed to an actor that speaks only streams was silently dropped and reported as that actor being handed nothing, and is now an error naming it; and the datagram lookahead asked about the wrong suffix for every actor after the first, which no configuration here reaches but which was wrong on its own terms. Verified against 178 end-to-end cases that put traffic through chains -- every stack, every relay shape, quic and amux and ws among them -- plus leaf's own suite.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )