Ioxide engine: ioxide 0.7.211, HTTP/1.1 to HTTP/3, native TLS with SNI, mutual TLS and live certificate rotation - #887
Draft
MDA2AV wants to merge 63 commits into
Draft
Ioxide engine: ioxide 0.7.211, HTTP/1.1 to HTTP/3, native TLS with SNI, mutual TLS and live certificate rotation#887MDA2AV wants to merge 63 commits into
MDA2AV wants to merge 63 commits into
Conversation
MDA2AV
changed the title
MDA2AV
marked this pull request as draft
August 10, 2026 09:39
MDA2AV
changed the title
MDA2AV
changed the title
…nation - ioxide 0.1.1 -> 0.4.161; the separate ioxide.tls package is folded into core - migrate renamed APIs (TcpConnection, TcpHandle, TcpConnectionDualPipe, ServerConfig.Tcp) - serve every configured endpoint (primary port + ExtraPorts) instead of the first only - endpoints bound with a certificate are TLS-terminated ring-natively (per-port contexts, certificate exported as PEM); client cert validation and SNI report as unsupported - replace the hand-rolled TlsDuplexPipe with ioxide's TlsConnectionDualPipe - release the connection when the handshake or connection factory faults
… one The eager Provide(null) in the constructor threw for SNI-only certificate providers (SecurityTests' PickyCertificateProvider), failing host startup for the secure-upgrade redirect cases that never actually handshake. Certificates are now resolved per reactor in OnStart. A secure port whose provider yields no default certificate stays advertised (so redirects derive the https port) but its handshakes are refused with a FIN, so a client sees a fast connection failure instead of a plaintext response on an https port.
…rd Information log)
ioxide.file 0.4.167 became io_uring reads only - it hands out a descriptor and a length, bakes no HTTP responses and caches no bytes. So Asset.Response, Asset.ResponseLength and AssetCache.IsFresh are all gone, and this module could not merely be re-pinned; the bump from 0.1.1 to 0.4.169 crosses that redesign. The engine goes 0.4.165 -> 0.4.169 with it. The baked-response branch is gone: the body is always read off the ring through the per-reactor AssetReader pool, which this class already used for assets too large to bake. The freshness check moves here rather than disappearing. The package dropped per-request statx deliberately - it trusts a snapshot's descriptors and expects Reload() on deploy - but this module's documented behaviour is that an edited file is served, and TestChangedFileServesUpdatedContent asserts it. Adopting the package's model silently would have changed GenHTTP's contract under its users, so AssetFreshness reproduces the size comparison the package used to do. It matters beyond freshness: the handler's length becomes Content-Length, so the body writer must agree with it or the response is malformed - which is exactly how the built-in Files module misbehaves when a file changes under it, serving new content at the old length. Acceptance suite: 2044 (net11) + 1442 (net10) pass, including all 16 Ioxide tests. Playground gains /ring and /disk over one directory to price the two against each other; that file also carries unrelated in-progress work, so it is left uncommitted deliberately.
/ring mounts IoxideFiles and /disk GenHTTP's built-in Files module over the
SAME directory, on the same engine, so the module is the only variable.
GENHTTP_STATIC picks the directory and neither route mounts without it.
Measured here with wrk -t8 -c64, best of two interleaved passes:
/ring /disk
4 KiB 835409 1041891
64 KiB 365531 509255
The built-in module is ahead, but part of that is work it does not do: edit a
file while it runs and it serves the new content at the old Content-Length,
truncating the response, where IoxideFiles serves it whole. That check is
what AssetFreshness restored.
One tuning note for later: IoxideAssetContent flushes every 12 KiB to stay
under the 16 KiB write slab, and at 64 KiB that costs about 19% - raising the
chunk to 64 KiB measured 433924 against 365531, content verified identical.
Left alone because a bigger chunk grows every connection's slab, which is a
memory tradeoff worth deciding rather than slipping in.
Added on a wrong assumption that 0.4.169 was unpublished. It is, so the local feed was both unnecessary and a hazard - it pinned an absolute path that only exists on one machine, and it shadowed the published package with a locally built one of the same version. Restore now resolves from nuget.org (verified via .nupkg.metadata source), and the acceptance suite passes against the published package: 2044 on net11, 1442 on net10.
The engine served HTTP/1.1 only. It now serves HTTP/2 - by ALPN on a TLS port, by the connection preface on a plaintext one - and HTTP/3 on the endpoint bound with enableQuic, carried by ngtcp2 and nghttp3. Streamed in both directions on both protocols. A handler starts once the request headers have arrived and pulls the body as it is delivered, paced by flow control so an upload cannot outrun it; the response goes out through the protocol's own writer, where each flush parks until the peer's window allows more. Serving a large file therefore costs the send-retention high-water rather than the size of the file. HTTP/2 and HTTP/3 differ only in transport, so the bridge between them and the handler chain is written once in Protocol/Mux and the two drivers are thin. The server splits along the same line: hosting, TLS termination and the QUIC listener are three partial files rather than one growing class. Certificates come from the caller. ngtcp2 loads PEM from disk rather than taking a certificate object, so Http3CertificatePath and Http3KeyPath name the files directly and nothing is written. Without them the endpoint's certificate is exported to a temporary directory created owner-only before anything is written to it, and removed on shutdown - which is worth avoiding, and the log says so. Mutual TLS across all three protocols, enforced where the connection is terminated: OpenSSL for HTTP/1.1 and HTTP/2, ngtcp2 for HTTP/3. An endpoint bound with a certificateValidator asks for a client certificate; ClientCaPath is what the offered one is validated against. Verified per protocol - a client signed by the configured CA is served, one offering nothing is refused, and one signed by another CA is refused. Engine options move to an IoxideOptions record rather than growing Create's parameter list, and HTTP/3 keys off the enableQuic flag GenHTTP's endpoint model already carries instead of a second switch. Acceptance suite 1442/1442.
Http2 was a flag in the engine options while HTTP/3 was enableQuic on the
endpoint, so the two protocols were configured in different places and neither
could be given a port of its own.
Protocols are a property of an endpoint, so they are set per endpoint now:
Protocols = IoxideProtocols.Http1, // what a port serves by default
ProtocolsByPort =
{
[8081] = IoxideProtocols.Http2, // h2c only, no HTTP/1.1 here
[8443] = IoxideProtocols.All, // h1 + h2 over TCP, h3 over UDP
}
HTTP/1.1 and HTTP/2 share the TCP socket - ALPN decides on a secure endpoint, the
connection preface on a plaintext one - and HTTP/3 is a UDP socket on the same
port number, so one port can serve all three or each can have its own. A port
that serves neither HTTP/1.1 nor HTTP/2 would otherwise accept TCP connections
and answer nothing, so HTTP/1.1 is served there instead.
The set is now honoured rather than advisory: an HTTP/2-only port closes a
connection that is not HTTP/2, where before it quietly answered HTTP/1.1.
HTTP/3 in the DEFAULT set applies only to endpoints that can serve it, since QUIC
carries TLS 1.3 and a plaintext port cannot - so Protocols = All reads as
"everything each port supports" rather than failing over the plaintext one. Named
explicitly for a port it is taken literally. Asking two endpoints for HTTP/3 is
still refused, but the message now names the ports and what to do about it.
enableQuic on Bind keeps working and still means HTTP/3 for that endpoint.
Acceptance suite 1442/1442; mutual TLS still enforced on all three protocols.
The sample showed 8081 as HTTP/2 only without saying that was a choice, so it read as a limitation. Both protocols share a port when the port is given Http1AndHttp2 - ALPN decides on a secure endpoint, the connection preface on a plaintext one.
Http1AndHttp3 and Http2AndHttp3 join the named combinations, and both describe
real deployments. HTTP/1.1 with HTTP/3 skips HTTP/2 entirely while still serving
every client - one that speaks neither gets HTTP/1.1, and a browser told about
the QUIC port by Alt-Svc moves itself there. HTTP/2 with HTTP/3 drops HTTP/1.1,
which suits somewhere the clients are known, gRPC being the obvious one.
An endpoint given only HTTP/3 now opens no TCP listener at all, where before it
was quietly given HTTP/1.1 on the grounds that the socket existed anyway. It does
not have to: the transport takes a null TCP configuration, so the endpoint binds
its UDP socket and nothing else, and a server made entirely of such endpoints
opens no TCP listener either. A port left with no protocols is a configuration
error rather than something to paper over.
Verified per combination, asserting the protocol actually negotiated rather than
that a request succeeded - a client asking for HTTP/2 against a port that does not
serve it falls back to HTTP/1.1 and answers 200, which reads as success:
Http1 h1 only, tcp
Http2 h2 only, tcp
Http3 h3 only, udp and no tcp listener
Http1AndHttp2 h1 h2, tcp
Http1AndHttp3 h1 h3, tcp + udp
Http2AndHttp3 h2 h3, tcp + udp
All h1 h2 h3, tcp + udp
Acceptance suite 1442/1442.
The sample bound three ports and described the rest in a comment. It now binds
one per combination that can coexist:
8080 Http1 HTTP/1.1 only
8081 Http2 HTTP/2 only - an HTTP/1.1 client is turned away
8082 Http1AndHttp2 both on one plaintext socket, the preface decides
8443 the HTTP/3 case
The four combinations carrying HTTP/3 cannot run together: the transport binds
one QUIC listener per server, so a second endpoint asking for HTTP/3 is refused
at startup. They take turns on 8443 instead, chosen by GENHTTP_H3 - All (the
default), Http1AndHttp3, Http2AndHttp3, or Http3 alone, which leaves that port
with a UDP socket and no TCP listener at all.
Each mode verified by asserting the protocol negotiated rather than that a
request succeeded, and by the listener counts: under GENHTTP_H3=Http3 port 8443
reports tcp=0 udp=32 while the others are TCP only.
The sample rotated the four HTTP/3 combinations through one port with an
environment variable, so six of the seven were only ever described. All seven run
at once now, each on its own port:
8080 Http1 8443 All
8081 Http2 8444 Http1AndHttp3
8082 Http1AndHttp2 8445 Http2AndHttp3
8446 Http3
A server binds one QUIC listener, so the four carrying HTTP/3 need a host each -
which costs nothing worth avoiding, since a host is a handler and a few reactors.
The three plaintext combinations share one host, having no QUIC listener to
contend over. Reactors are held at two apiece rather than one per core: six hosts
on one machine, and a sample is not where throughput is measured.
Verified per port by the protocol actually negotiated, not by a request
succeeding - a client asking for HTTP/2 where it is not served falls back to
HTTP/1.1 and answers 200:
8080 h1 8443 h1 h2 h3
8081 h2 8444 h1 h3
8082 h1 h2 8445 h2 h3
8446 h3
Port 8446 reports tcp=0 udp=2: an HTTP/3-only endpoint opens no TCP listener.
Acceptance suite 1442/1442.
Seven hosts to demonstrate seven combinations was more machinery than the point
deserved. One host binds all of them except a second HTTP/3 port, so the sample
is one host again:
8080 Http1 HTTP/1.1 only
8081 Http2 HTTP/2 only - an HTTP/1.1 client is turned away
8082 Http1AndHttp2 both on one socket, the preface decides
8443 All HTTP/1.1 + HTTP/2 over TCP, HTTP/3 over UDP
Http1AndHttp3, Http2AndHttp3 and Http3-alone are named in the header rather than
bound, because only one endpoint per server can carry HTTP/3 - the transport
binds a single QUIC listener - and changing what 8443 serves is how to try them.
Verified by the protocol negotiated on each port rather than by a request
succeeding, and by the listener counts.
IoxideOptions was a flat list of nine properties from three unrelated concerns.
The two that belong together are grouped now:
options.Http3.CertificatePath options.MutualTls.ClientCaPath
options.Http3.KeyPath options.MutualTls.ClientCaPem
options.Http3.QpackDynamicTableCapacity
options.Http3.QpackBlockedStreams
Protocols and ProtocolsByPort stay at the top, being what the engine is mostly
configured through. QUIC's certificate and HTTP/3's QPACK share a group despite
belonging to different layers, because they configure the same endpoint.
The sample gains a port serving HTTP/1.1 behind mutual TLS, which also shows that
requiring a client certificate is decided per endpoint: 8444 is bound with a
certificateValidator and demands one, while 8443 alongside it stays open. The CA
they are validated against is shared by the server, since that is what the
transport takes.
An endpoint like that cannot be tried without a client certificate, so the sample
writes a CA, one certificate signed by it and one signed by nobody into ./certs
on startup, and the header carries the curl commands. Verified: the signed client
gets 200, no certificate is refused, the impostor is refused, and 8443 answers
both HTTP/1.1 and HTTP/3 without a certificate throughout.
Every certificate there shares one validity window. Reading the clock per
certificate put the leaf a second beyond its issuer, which is refused outright -
the sample crashed on startup until they were pinned.
The comment explained that ngtcp2 loads PEM from disk without saying whose PEM, which reads as though HTTP/3 needed a certificate of its own. It serves the one bound to its endpoint, the same as HTTP/1.1 and HTTP/2; the paths only change whether that certificate reaches ngtcp2 from files that already exist or from one written out for it, because ngtcp2 has no in-memory alternative and OpenSSL does.
Http3.CertificatePath exists to hand ngtcp2 a file, since it loads PEM from disk
and has no in-memory alternative - unlike OpenSSL, which terminates the TCP
protocols and takes the PEM text directly. Nothing stopped those paths naming a
DIFFERENT certificate, and then the same port answered as one host over TCP and
another over QUIC, silently. Confirmed on one endpoint:
h1/h2: subject=CN = localhost
h3: subject: CN=DIFFERENT-h3-identity
That breaks the reason the two share a port. A browser moving from HTTP/1.1 to
HTTP/3 by an Alt-Svc header expects the alternative to present a certificate
valid for the ORIGIN (RFC 7838 3.1), so it would refuse the upgrade - or not
notice.
Compared by leaf thumbprint, so a file carrying a fuller chain than the bound
certificate is not flagged. A warning rather than a refusal: someone may be doing
it deliberately, and this is not the place to decide they cannot.
The comparison reads the PEM text rather than calling CreateFromPemFile, which
wants a private key beside the certificate and throws on the certificate-only
file this usually is - the first version of this check threw every time and
logged it at Debug, so it looked like the warning simply never fired.
The sample wrote every generated key with File.WriteAllText, which takes the
umask - so client.key and impostor.key landed world-readable. Throwaways, but a
sample is read as an example of how to do it, and the engine's own export next to
them was already 0600.
It also left the HTTP/3 certificate paths unset, so the engine exported the bound
certificate to a temporary directory. That works and is owner-only, but the copy
outlives a SIGKILL - repeated restarts leave a private key per run under /tmp.
The sample now writes its certificate to ./certs and names it, which removes the
export entirely and demonstrates the option worth using in a deployment.
/tmp/genhttp-ioxide-* gone, 0 export log lines
certs/*.key -rw-------
certs/*.crt -rw-rw-r-- (public, unchanged)
All four still answer: h1 1.1, h2 2, h3 3, and mutual TLS on 8444 with the signed
client.
ngtcp2 loads PEM by path, which is the C layer's contract and fine. Working
around it was not: an endpoint serving HTTP/3 without configured paths had the
bound certificate exported to a temporary directory, so the engine chose a
location and a lifetime for someone else's private key. Owner-only, deleted on
shutdown - and still there after any shutdown that skips cleanup, one directory
per run.
Http3.CertificatePath and Http3.KeyPath are required to serve HTTP/3 now.
Without them the endpoint is a configuration error, named and explained, rather
than a key appearing under /tmp:
Port 8443 serves HTTP/3, which needs a PEM certificate and key on disk -
ngtcp2 loads them by path. Set IoxideOptions.Http3.CertificatePath and
Http3.KeyPath to the same certificate bound to that endpoint.
That removes the export, the owner-only temp directory, the writer that made it
and the cleanup that chased it - about sixty lines. The check that the configured
PEM is actually the endpoint's certificate stays, since naming the wrong one is
still possible and still leaves a port answering as two hosts.
The sample writes its own throwaway certificate to ./certs and names it, which is
what a deployment does with the PEM it already has.
"Mux" was jargon for the one thing HTTP/2 and HTTP/3 have in common, and it read as though the folder were a protocol of its own. Splitting it into Http2 and Http3 folders was the obvious alternative and does not work: 648 of those lines are used verbatim by both protocols against 179 in the drivers, which themselves differ by 39 lines once the protocol names are normalised. Splitting would either duplicate the 648 or leave a third shared folder anyway - the same shape under another name. So the folder is Multiplexed, which says why the code is shared, and the two drivers move up beside ConnectionDriver, the HTTP/1.1 one. Each protocol now has its driver in Protocol/ and the request and response bridge they share sits in Protocol/Multiplexed/. Types renamed to match. No behaviour change: h1 1.1, h2c 2, h2 2, h3 3, mutual TLS 200, acceptance 1442.
…op the duplicated StatusLine ConnectionDriver had grown into two unrelated jobs: deciding what protocol a TCP connection speaks, and then serving it when the answer was HTTP/1.1. The second half moves to Http1Driver, alongside Http2Driver and Http3Driver - so each protocol is one file, and ConnectionDriver is only the transport plus the ALPN/preface decision that routes to them. The engine also carried its own copy of StatusLine, byte-identical to the one in GenHTTP.Engine.Shared.Types. Use the shared one; the Ioxide engine gets the same InternalsVisibleTo the acceptance tests already have. DateHeader stays duplicated on purpose - the engine's is [ThreadStatic] so each reactor owns its buffer, which the shared static cannot be.
The comment blocks had grown to the point of hiding the code they explained - 590 of 2540 lines, with whole paragraphs restating what the next statement says. Trimmed to what is not derivable from reading it: the traps, the RFC references, and the reasons a line is the way it is. Public XML docs keep their summaries. No code changed - the diff is comment-only, verified by comparing both revisions with every comment line stripped.
Fourteen files sat flat in Protocol/, and nothing in the listing said which
belonged to which protocol. The dependency graph already answered it: the six
response-writing files are reachable only from Http1Driver, and the six
Multiplexed ones only from Http2Driver and Http3Driver.
Protocol/
ConnectionDriver.cs the TCP entry point, and the only fork between them
Http1/ Http1Driver + its response writing and sinks
Multiplexed/ Http2Driver, Http3Driver + what the two share
Namespaces follow the folders, so the moved types are now under .Protocol.Http1
and .Protocol.Multiplexed. Both nest inside .Protocol, which is how the drivers
still reach ConnectionDriver without importing anything.
kernelTx/kernelRx were loose booleans on Host.Create, next to the delegates,
saying neither what they switch nor where they apply. They are now grouped like
Http3 and MutualTls already were:
options: new IoxideOptions
{
Tcp = new IoxideTcpOptions { TxKernelTls = true, RxKernelTls = true },
}
Tcp is the honest group for them. kTLS offloads the record layer OpenSSL owns,
which terminates HTTP/1.1 and HTTP/2 only - HTTP/3 carries TLS 1.3 inside ngtcp2
and can never use it. The old names said "kernel" without saying kernel WHAT, and
sat where nothing marked that boundary.
Host.Create drops both parameters; nothing outside the engine passed them.
The kernel TLS knobs moved into options and the sample had no example of the group. Shipped off: the tls ULP is absent on most machines, and a sample that needs modprobe to serve anything is not a sample.
Matches Engine/Internal/Host.cs and Engine/Kestrel/Host.cs, which hold the same entry point under the same name.
IoxideServer and IoxideServerHost sat in GenHTTP.Engine.Ioxide.Infrastructure, where the namespace already says whose they are. They are now Server and ServerHost, and the files match. ServerHost shadows the GenHTTP base class it derives from, so the base is qualified as Shared.Hosting.ServerHost - the same shape Server.Quic.cs already uses for Shared.Infrastructure.SecurityConfiguration. The sibling engines avoid this by prefixing instead (ThreadedServerHost, KestrelServerHost).
…groups IoxideOptions and its four groups all carried a prefix the namespace already supplies. They are now EngineOptions, ReactorOptions, Http3Options, MutualTlsOptions - and TcpTransportOptions. That last one is not TcpOptions on purpose. ioxide has a TcpOptions of its own, and reaching ours means importing both namespaces: WriteOverflow and Incremental are ioxide's types, so anyone tuning the TCP transport writes `using ioxide;` and gets CS0104 on a name that ordinary. The Playground proved it before the rename was a minute old. Where the engine builds ioxide's, it now says ioxide.TcpOptions outright rather than relying on which namespace wins.
… folder IoxideTls was a folder holding one four-line method with one caller, under the prefix every other type has now shed. Its two halves belonged in different places, which is why neither fit where it was: AcceptWithAlpnAsync is per-connection work on the reactor thread, and establishing a connection's transport is exactly what ConnectionDriver is for. It is now a private AcceptTlsAsync there, beside the plaintext branch it is the alternative to. TlsRegistry joins Server.Tcp.Tls.cs, where ResolveTls produces what fills it. Not Server.Tcp.Tls for the handshake: that file is a partial of Server holding startup configuration - instance members that run once per reactor in OnStart. Terminating a connection has no server, and putting it there would have the connection driver calling into Server for a transport primitive.
…ing QUIC twice
_tcpRequested reads like a boolean - it was named for symmetry with
_quicRequested - but it holds the ports WithTcp binds, so it is _tcpPorts.
The symmetry it was named for turned out to be a duplicate anyway. _quicRequested
was resolved in the constructor and _quicEndPoint assigned the same value again
inside WithQuic, so the two always agreed and only one was needed. The field now
lives with the rest of the QUIC state, is readonly, and WithQuic reads it instead
of being handed it - the mirror of WithTcp reading _tcpPorts.
if (_tcpPorts.Length > 0) serverConfig = WithTcp(serverConfig);
if (_quicEndPoint is not null) serverConfig = WithQuic(serverConfig);
…agreed The check reads as an arbitrary refusal without the mismatch behind it: GenHTTP takes dual-stack per endpoint on Bind, ioxide takes one flag for the whole server, and the engine honours the first endpoint's. Endpoints that disagree would otherwise be served a mode they did not ask for, silently. The comparison value is hoisted, so it reads as all-against-the-first rather than something pairwise, and the message now names the first endpoint, the mode taken from it, and the ports that wanted the other: The ioxide engine binds every endpoint with one dual-stack mode, taken from the first one bound (port 8080, DualStack = True). These ask for the other: 8081, 8082.
EndPoint held a `secure` bool while the server kept a second table with the SecurityConfiguration behind it, keyed by port - two representations of one fact, one of them carrying the payload. The endpoint now holds the configuration and Secure is derived from it, so _secure is gone. WithQuic is the clearest gain: it already held the endpoint and was looking its own security up by port. It reads quicEndPoint.Security now. Not SecureEndPoint/InsecureEndPoint as the Internal engine has them - there the subclasses do the work (SecureEndPoint owns the SslStream handshake and the validation callback), here the endpoint is passive and TLS happens in the connection driver, so subclassing would add two types with no behaviour.
859fab4 dropped the Ioxide prefix from the infrastructure types, but two callers still named the old one. Playground/Program.cs set Protocols and ProtocolsByPort against IoxideProtocols, so the playground did not compile at all from that commit onward - the rename was verified against the engine, which built fine, and not against the sample that consumes it. The ProtocolsByPort summary carried the same stale name in its example. Nothing reads a doc comment, so it built either way and pointed at a type that no longer exists.
… one _primary was a second reference to an endpoint the port table already had, kept only because nothing else remembered which one came first. Same shape as the _secure bool this series just removed: one fact stored twice, and in principle able to disagree. The endpoints are now an array in bind order, and _endPointByPort is derived from it in the constructor rather than built alongside it, so the two cannot drift. The first endpoint is the first element, which is all _primary ever meant. The array also serves the places that were filtering the dictionary's values - SecureEndPoints and the HTTP/3 resolution - and ResolveQuicEndPoint no longer takes the mapped list as a parameter, since it can read the field. DualStack comes out as its own field. It is one mode for the whole server, and reading it off _primary made it look like the first endpoint's opinion when MapEndPoints has already refused any endpoint that disagrees.
… layout _config and _options named the kind of thing rather than which one. The server takes a ServerConfiguration from GenHTTP and an EngineOptions of its own, and a read of either had to be traced back to its field to tell the two apart. They are _serverConfiguration and _engineOptions now, with the constructor parameter matching. The file also picks up the layout the rest of GenHTTP uses: a Get-/Setters region over the interface members and a Constructors region around the constructor. MapEndPoints and BuildServerConfig move below StartAsync, both being setup detail the constructor calls once - sitting between the constructor and StartAsync they put half a file between the class and the thing it actually does.
…swers it _endPointByPort was a second collection over the same objects, built from _endPoints in the constructor to serve exactly one caller - the TcpHandle lookup that turns a connection's listener port into the endpoint it arrived on. A whole dictionary for one lookup, and another thing to keep in step with the array beside it. EndPointFor scans instead, and sits next to ProtocolsFor which answers the same question about the same port. A server binds a handful of endpoints, so walking a contiguous array is no worse than hashing a ushort, and the endpoints are in one place. A port with no endpoint now throws with the port in the message rather than a bare KeyNotFoundException from the indexer. The duplicate-port guard survives the removal: _protocols is still built with ToDictionary over the same key, so two endpoints on one port still fail in the constructor rather than silently keeping one.
_protocols was a Dictionary<ushort, Protocols> beside the endpoints, keyed by the port that already identifies them - the same second table 30d2ad5 removed for SecurityConfiguration, and the last one left. An endpoint is unique per port, which is what that dictionary was quietly asserting, so what a port serves is a fact about the endpoint and now lives on it. ResolveProtocols stops hunting. Given only a port, it went back to config.EndPoints twice per endpoint to ask whether that port had a certificate and whether it had enabled QUIC. It takes the binding itself now, and both scans become endPoint.Security is null and endPoint.EnableQuic. The accept path does one lookup instead of two. TcpHandle was calling EndPointFor and ProtocolsFor with the same port; it now reads the protocols off the endpoint it has already found. ResolveQuicEndPoint and ResolveTcpPorts filter the array directly rather than indexing a table alongside it. One thing had to be replaced rather than deleted. Building _protocols with ToDictionary was, by accident, the check that no port was bound twice - with it gone MapEndPoints refuses duplicates itself, naming the port and how often it was bound instead of raising a bare ArgumentException from the dictionary. ProtocolsFor's fallback to Http1 for an unknown port is not carried over: it became unreachable in 5b7a6fe, when EndPointFor started throwing for a port no endpoint is bound to.
The field was declared in Server.cs while the only things that write and read it - ResolveTcpPorts and WithTcp - live in Server.Tcp.cs. It moves to the partial that owns it, which is what Server.Quic.cs already does with _quicEndPoint.
…arry its TLS EndPoint answered for both kinds at once: a nullable Security that half the engine null-checked and the other half dereferenced with a !, and a Secure derived from whether it was set. It is abstract now, with InsecureEndPoint and SecureEndPoint under it, so which kind an endpoint is became its type. SecureEndPoints is _endPoints.OfType<SecureEndPoint>(), and every null-forgiving operator on Security went with it. The mutual-TLS settings move onto the secure endpoint. RequiresClientCertificate was ORing the engine's flag with the binding's own validator at each use - once building the TLS options, again creating the QUIC engine - the same question answered twice about the same endpoint. SecureEndPoint settles it in its constructor and both transports read RequireClientCertificate, so neither reaches back into EngineOptions for it. WithQuic's check that HTTP/3 was not asked for on a plaintext port is a type test now rather than a null test. The trust anchors stay configured on EngineOptions, since GenHTTP's Bind takes no bundle per endpoint. What moves onto the endpoint is the resolved answer, the same way protocols did in ae3050e. One behaviour change: MutualTlsConfigured is per endpoint rather than server-wide, so it reads false where the engine names a CA bundle but nothing is bound to serve it. It decides only whether the startup line says mTLS.
…rust different issuers MutualTlsOptions was engine-wide and had no per-endpoint form, because GenHTTP's Bind carries a certificate provider, protocols and a validator but no trust bundle. That left one set of anchors for every secure endpoint: a server fronting two audiences on two ports had to validate both against the same issuers, or bind two hosts. MutualTlsByPort is the override, shaped exactly like ProtocolsByPort - name the port, give it its own MutualTlsOptions, and the engine-wide MutualTls covers the rest. Taken whole rather than merged, for the same reason ProtocolsByPort is: a named port that inherited the halves it left unset would make a bundle appear on an endpoint that named none. It resolves in Map, so SecureEndPoint carries its own anchors and the transports go on reading the endpoint rather than the options. 0816304 moved the answer onto the endpoint; this gives the answer somewhere per-endpoint to come from. Untested by the acceptance suite: the client-certificate tests are skipped for this engine by a guard in their helper that still says TLS termination is not implemented, which stopped being true.
…tions The engine held what client certificates are validated against while the endpoint held whether one was asked for, and the two were ORed at each use. Both are facts about a single binding, and IServerHost already carries them there: Bind takes a certificateValidator, which is GenHTTP's own per-endpoint client-certificate hook. What that hook could not carry is the trust anchors. ICertificateValidator is handed a chain that has already been built, which suits an engine validating in managed code; ioxide validates in OpenSSL, and in ngtcp2 for HTTP/3, both of which need the anchors before the handshake starts. IMutualTlsValidator adds them to the validator, so an endpoint that wants mutual TLS names its issuers on the binding that asked for it. EngineOptions.MutualTls, MutualTlsByPort and MutualTlsOptions are gone. Per-port anchors were the whole point of MutualTlsByPort one commit ago; they now fall out of where the settings live, with no second table keyed by port to resolve against. RequireClientCertificate stops being an OR of two sources, there being one now, and MutualTls collapses to Security.CertificateValidator is not null - everything mutual TLS needs arrives on a validator, so having one is what it means to want it. One behaviour change beyond the move: a secure endpoint bound without a validator used to inherit the engine-wide client CA, handing OpenSSL a trust store for a port that never asked for client certificates. It gets none now. In the playground that is 8443, which the comment there already described as staying open while 8444 requires one. Breaking for anyone setting EngineOptions.MutualTls: the CA moves onto the validator passed to Bind. Still untested by the acceptance suite, whose client-certificate tests are skipped for this engine.
ResolveTls handed ioxide certificate.ExportCertificatePem(), which exports one certificate. Anything issued by a real CA is signed by an intermediate, and a client that does not already hold that intermediate cannot build a path to a root it trusts - so the handshake fails, or the certificate is reported untrusted, for every client without it cached. Nothing caught it because a self-signed certificate is leaf and root at once, which is what the playground and the tests use. It would have shown up the first time someone pointed this engine at a certificate from an actual issuer. The Internal engine never had the bug: SslStream assembles the chain itself. This engine terminates TLS on its own, so ExportChainPem assembles it here, leaf first, with the root left off - a client that does not already trust the root will not start because the server sent it, and it is bytes on every handshake. Two limits worth knowing. ICertificateProvider hands back a single X509Certificate2, which cannot carry a chain at all, so the intermediate has to be findable in the machine store; when it is not, the leaf goes out alone as before, but a warning now names the port and subject rather than saying nothing. And certificate downloads are off, because fetching a missing intermediate over AIA would put a network call on the startup path - an unreachable host there is a hung server, not a slow one.
…t trusts nothing ICertificateValidator.RequireCertificate defaults to TRUE, so any validator that does not override it asks for a client certificate. Since 4fea67c the trust anchors travel on the validator too, which means a plain ICertificateValidator - not an IMutualTlsValidator - now means "require a certificate, validate it against nothing". ioxide refuses that combination, correctly: TlsService throws when RequireClientCertificate is set with no anchors. But it throws where it is built, on a reactor thread, part-way through StartAsync - so a configuration mistake surfaced as a crash from inside the engine rather than as an answer about the binding. MapEndPoints refuses it up front instead, naming the port and what to do about it, alongside the duplicate-port and dual-stack checks that were already there.
…re they differ The two halves of this engine accept different things in different forms, for a reason that is invisible from either one alone: OpenSSL is handed the certificate as data, ngtcp2 loads it by path. So the server certificate arrives as an X509Certificate2 on TCP and as a file path on HTTP/3, and the HTTP/3 one has to be named a second time on EngineOptions.Http3 rather than being taken from the binding. SecureEndPoint is where both transports read from, so the table belongs on it. It also records the one place they disagree on the same setting: ClientCaPem reaches OpenSSL and is dropped on the way to ngtcp2, so an endpoint serving both validates clients on TCP and not on QUIC. ioxide 0.5.192 takes PEM text for QUIC; this engine references 0.4.186 and closes it at that bump.
The engine offered ClientCaPath and ClientCaPem alike, but only the path survived the trip to QUIC: ngtcp2 took a path and nothing else, so WithQuic had nothing to hand it the text form through. An endpoint serving Protocols.All therefore validated client certificates over HTTP/1.1 and HTTP/2 and let every client through unvalidated over HTTP/3 - the same origin, two answers, and no warning either way, since from the QUIC side it looked like an endpoint that had asked for no client verification at all. ioxide 0.5.192 takes the anchors as text for QUIC as well, so WithQuic passes ClientCaPem and the setting means one thing on both transports. The reference moves for ioxide.file too, which was still on 0.4.186 alongside the rest.
… has The HTTP/3 certificate was configured twice: once on Bind as an X509Certificate2, and again on EngineOptions.Http3 as a pair of paths, because ngtcp2 is a C library that loads PEM by path and takes nothing else. Two places naming one certificate, so the engine had to compare thumbprints and warn when they disagreed - a check that only existed because the disagreement was possible. IFileCertificateProvider is an ICertificateProvider that can also answer with files. A binding passes one to Bind and both transports read from it, so there is one source and nothing left to disagree with. EngineOptions.Http3 keeps QPACK and gives up the certificate; TryResolveQuicCertificate and WarnIfNotTheBoundCertificate go with it. Files are preferred wherever both forms are on offer, which also settles the chain. OpenSSL loads a chain file whole through SSL_CTX_use_certificate_chain_file, so the intermediates come from the file the binding named rather than being recovered from the machine store, and the private key never enters managed memory. The rebuilt chain from 64f8022 stays for the in-memory form, which carries no chain and has nowhere else to get one. A plain ICertificateProvider still serves HTTP/1.1 and HTTP/2 exactly as before. It is HTTP/3 that is now refused outright on such a binding, when the server is built and naming the port, rather than starting a server without the listener it was asked for. This needs no change to GenHTTP's own API: the ICertificateProvider Bind overload already existed, and SecurityConfiguration carries the provider through to the engine.
…ing callers write one Every deployment binding a certificate from disk needs the same IFileCertificateProvider, and the playground had written it inline as though it were sample-specific. It is not - it is the ioxide counterpart of Engine/Shared/Security/SimpleCertificateProvider, which is what the X509Certificate2 Bind overload has always wrapped. Two constructors, because callers arrive with different things in hand. From files alone is the ACME case, where a client has left a certificate and key on disk and nothing has been loaded; the object form is then read from those files only if something asks for it, so on this engine - which prefers the files - the private key never enters managed memory. From a certificate and its files is for a caller already holding both, and saves reading them back. The two are not checked against each other in that second form. Naming a certificate and files that disagree is naming two certificates, and the engine no longer has any way to notice, having just deleted the thumbprint check that existed for exactly that reason.
One port, several sites. Bind an IHostCertificateProvider and the endpoint names the hosts it answers for; the engine asks each for its certificate when the server starts and serves it to clients asking for that name. ICertificateProvider already took the host, but a callback alone was never enough here: both transports settle certificates at startup rather than per handshake, so the engine has to know the names up front. OpenSSL gets a context per name, ngtcp2 gets each registered before it accepts anything - which ioxide now refuses to allow later, since that table is read from handshakes on reactor threads without a lock. The binding's own certificate stays the default. A client that sends no name, or asks for one nobody registered, is answered with it rather than refused, so the port is still reachable by address. Files are preferred per host for the same reason as the default: OpenSSL reads a chain file whole, so the intermediates come from the file instead of being rebuilt from the machine store. A host with only an X509Certificate2 serves HTTP/1.1 and HTTP/2 and is left out of the QUIC listener, with a warning saying so - ngtcp2 loads PEM by path and the engine still will not write a private key out on anyone's behalf. Client verification is not repeated per host and does not need to be: OpenSSL fixes the verify mode on a connection when it is created, from the default context, so a name cannot select its way out of the mutual TLS the endpoint was bound with. HostCertificateProvider ships the ordinary case, so a deployment with a certificate per site on disk does not have to write a provider. Verified against both transports with curl validating the chain and the name: alpha and beta each get their own certificate over HTTP/1.1, HTTP/2 and HTTP/3, each is refused when checked against the other's, and an unregistered name falls back to the default.
A renewed certificate used to mean a restart, which is a dropped connection for every client on the server at the time. ReloadCertificates asks every bound provider again and installs what it answers with, across both transports, without closing anything - what an ACME hook calls once its client has rewritten the PEM. Connections already established keep the certificate they authenticated with, since that is what their peer verified. Only the material changes. Trust anchors, RequireCertificate, ALPN, the TLS floor and the kTLS pin stay as the binding set them, and no name can be added: both stacks settle their SNI tables at startup, so a reload replaces the certificates behind the names rather than the set of them. Everything is resolved and checked before anything is published, because a half-rotated server is worse than one still serving yesterday's certificate. A provider that throws, or a path an ACME client has not finished writing, leaves the server exactly as it was. HTTP/3 goes first of the two: ngtcp2 publishes a generation with one atomic store, so bad material fails there having changed nothing at all. The TCP reactors then take theirs one at a time, and a service that refuses the new pair keeps the old - still a certificate the endpoint was bound with, so the server serves throughout, it just briefly serves two vintages. What refused is named in the error and logged against its reactor and port. The bump to 0.7.211 is what makes that possible, and brings the transport settings that were missing alongside it. TCP gains a handshake timeout, so a client that opens a connection and never finishes is swept rather than held; the TLS 1.3 ciphersuites and the 1.2 cipher list; and a floor taken from the SslProtocols the binding already named. That last one cannot be honoured exactly - SslProtocols is a set and OpenSSL takes a minimum with no maximum - so a set that is contiguous and open at the top is served as asked, and the two that are not warn and widen rather than throw. A defensible default elsewhere should not be a dead deployment here, but serving more than was asked for is something an operator needs told. QUIC gains its own handshake timeout, an idle backstop for a connection whose engine went quiet, the choice between forwarding a migrated client's datagrams and routing them by connection id in a BPF program, whether to claim a migrated peer's new address, and the requested UDP socket buffer. That last is worth measuring rather than maximising: the kernel clamps it to net.core.rmem_max, and granting the full 8 MiB cost ~45% of throughput at saturation on ioxide's own benchmark, because a deep standing queue replaced the early drops congestion control reads. Two gaps in what a bound ICertificateValidator can expect are now said out loud at startup instead of left to be inferred from a client that was let in: revocation checking is not performed, neither stack being given a CRL or an OCSP responder here, and Validate does not run over QUIC, where ngtcp2 exposes no peer certificate to hand it. Verified on a server with a certificate per name and a mutual TLS port: SIGHUP replaces all three certificates, each name serving a new serial afterwards, with requests in flight throughout and the client CA still admitting its own client and refusing the impostor.
The sample bound one certificate it loaded from a PKCS#12 bundle, which showed neither of the two things this engine actually asks a deployment to decide: that a port can answer for several names, and that renewing one needs no restart. It now mints its own throwaway PEM into ./certs on startup - a server certificate per name, plus a client CA, a client it signs and an impostor it does not - so every port can be tried without generating anything by hand. The names are bound through a HostCertificateProvider, so 8443 serves alpha.localhost and beta.localhost their own certificates and answers everyone else with the default, over all three protocols. SIGHUP rewrites the PEM and calls ReloadCertificates, which is the shape an ACME renewal hook takes: rewrite the files the providers already name, then ask the server to install them. Everything is named as files, which is what lets HTTP/3 serve the same three names - ngtcp2 loads PEM by path and takes nothing else. Keys are written with an explicit mode rather than through the umask: they are throwaways, but a sample is read as an example of how to do it. Verified with curl over HTTP/1.1 and HTTP/2 and with openssl s_client per name: each name serves its own subject, an unregistered name falls back to the default, the mutual TLS port admits the signed client and refuses the impostor, and after SIGHUP all three names serve a new serial with no connection dropped.
Nothing covered the Connection header being compared case-insensitively, on any engine - both fixes for it shipped without a test, and a refactor that moves the comparison somewhere else takes the fix with it and stays green. This pipelines two requests onto one connection, the first asking to keep it alive in lowercase and the second to close it. Honouring the first is exactly what lets the second be answered, so a case-sensitive comparison comes back with one response instead of two. Checked by reverting the fix, where it fails, rather than only by passing. The ioxide skips said the engine had no TLS termination and no SNI provider, which stopped being true. What is true is narrower, and differs per test: the security tests bind a provider answering only a named host, and this engine asks once at startup with no name, so the port ends up with no default certificate; the client-certificate tests require a certificate from a validator naming no anchors, which the engine refuses when the server starts. Each now says its own reason. The optional case needs neither and no longer skips.
MDA2AV
changed the title
…ry to TCP A bound ICertificateValidator was never asked anything. Its anchors travel with it and OpenSSL settles the chain before the handshake completes, so Validate had nothing left to decide and simply was not called - an application pinning a certificate, holding a subject allow-list or checking its own revocation source had its answer ignored. It now runs on the TCP transports against the peer certificate ioxide hands over as DER, and a refusal throws, so the session is dropped before any protocol driver sees it rather than after. The chain it is given is built for its elements rather than its verdict, with revocation off and certificate downloads disabled. A privately issued client certificate does not validate against this machine's store and is exactly what mutual TLS usually carries, so reporting that as a chain error would reject the clients the endpoint was bound to accept. A port that asks for a certificate and lets the client decline reaches the validator with RemoteCertificateNotAvailable rather than not reaching it at all; one that demands a certificate never gets there, because OpenSSL has already refused the handshake. Over QUIC it still does not run, ngtcp2 exposing the peer's subject and common name but no certificate to hand it, and the startup warning now says which transports differ rather than naming versions. The TLS registry was built on every reactor whenever any endpoint was secure. On an HTTP/3-only server that is an OpenSSL context per name per reactor, plus its chains and a handshake sweep, for a port that binds no TCP listener at all - ResolveTcpPorts already left it out, and QUIC terminates its own TLS in ngtcp2 from the paths the binding named. ResolveTls now filters on the same predicate, and the registry is created only where there is something for it to hold. It keeps its empty form where a secure TCP port produced no certificate: Reactor.GetService throws rather than answering null, so without it the driver drops such a connection with no FIN and the client hangs instead of failing fast. Renamed TcpTlsRegistry, since that is what it always was, and the setup moved next to the rest of the TCP TLS resolution. That left ReloadCertificates deriving its liveness from a TCP structure, which would have refused to rotate an HTTP/3-only server. It guards on Running and the presence of a secure endpoint instead, reads the registries once inside the lock so a stop landing mid-rotation cannot swap them, and rotates QUIC from the endpoint it finds either way. Its log line counted only TCP rotations, so a successful QUIC-only rotation reported zero; the two are counted apart rather than summed, a port serving HTTP/3 alongside TCP being in both. Verified on a server bound HTTP/3-only: no TCP listener, a connect to the port refused, and ReloadCertificates rotating the QUIC certificate with no registry present. A secure TCP port whose provider answers with nothing still closes the connection in a couple of milliseconds rather than hanging, and a server with no secure endpoint still refuses to rotate. The new acceptance test binds a validator that answers every peer the same way, so only the verdict is under test: the client it rejects does not reach the handler, and the one it accepts does. Kestrel is inconclusive there - it consults the validator only once a certificate arrives, so under AllowCertificate a client offering none is admitted without being asked about.
…tself The engine carried 726 lines of comment against 2,633 of code - every public member documented at length, and most method bodies annotated with the reasoning behind them. Removed: the XML docs in full, and the inline commentary that restated what the line below it already said. Seventeen lines survive, each stating something no reader could derive from the code. Why Flush is deliberately a no-op, so it is not "fixed" into a reactor deadlock. Why Tcp is explicitly null, ioxide otherwise defaulting to a live listener on 8080 that an HTTP/3-only server would inherit. Why the reactors are stopped and then joined, a single-issuer ring having to be disposed on its own thread or it leaks one per host. Why a chain is built for its elements and its verdict thrown away. Why QUIC rotates first, being the transport that publishes atomically. That GENHTTP_IOXIDE_PARSER=pico turns off the smuggling hardening and is for benchmarking, not for untrusted traffic. Why an HTTP/3 request carries no remote address. And the consumed-versus-examined subtlety in the HTTP/2 preface peek, where marking the bytes examined would park the next protocol on data already in hand. CS1591 is already suppressed in Directory.Build.props, so the documentation file still generates and the build stays clean at both target frameworks. Nothing here changes behaviour: comparing the two trees line by line with comments and blank lines removed reports no difference.
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
1 participant
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.
Brings the ioxide engine up to
0.7.211and finishes the transport story: every protocol combination on any port, TLS terminated natively with SNI and mutual TLS, and certificates that can be replaced without restarting.ioxide
0.1.1→0.7.211, with the separateioxide.tlspackage folded into core,ioxide.filefollowing for the IoxideFiles module, and the renamed APIs migrated (TcpConnection,TcpHandle,TcpConnectionDualPipe,ServerConfig.Tcp).Protocols, per port. HTTP/1.1, HTTP/2 (framing, HPACK and flow control in C#) and HTTP/3 (ngtcp2 for QUIC, nghttp3 for H3 and QPACK), in any combination.
Protocolssets the default andProtocolsByPortoverrides it, so one host can serve h2c on one port and all three on another. HTTP/1.1 and HTTP/2 share a TCP socket - ALPN decides on a secure port, the connection preface on a plaintext one - and HTTP/3 takes a UDP socket of the same number. HTTP/3 without TCP works too. Every configured endpoint is served, rather than the first only.TLS, terminated in the engine. Certificates come off the binding in whichever form the provider has: an
IFileCertificateProvidernames PEM paths and serves all three protocols, a plainICertificateProviderhands over anX509Certificate2and serves the TCP transports. Files are preferred where both are offered, because OpenSSL reads a chain file whole - the intermediates then come from the file rather than being rebuilt from the machine store, and the private key never enters managed memory. The engine will not write a key out on anyone's behalf, which is why HTTP/3 needs files: ngtcp2 loads PEM by path and takes nothing else.SNI.
IHostCertificateProvidernames the hosts a port answers for, and each gets its own certificate over all three protocols. The binding's certificate stays the default, answering a client that sent no name or asked for one nobody registered, so the port is still reachable by address. Both stacks settle their tables at startup, so the names are read once.Mutual TLS, per endpoint. An
IMutualTlsValidatorcarries the trust anchors alongside the validator, as a path or as PEM text, and both reach OpenSSL and ngtcp2 - so two secure ports can trust different issuers. An endpoint that requires a client certificate while naming nothing to validate it against is refused when the server starts, rather than throwing from a reactor thread mid-startup.Certificate rotation without a restart.
ReloadCertificates()asks every bound provider again and installs the result across both transports without closing a connection - what an ACME hook calls once its client has rewritten the PEM. Everything is resolved and checked before anything is published, so a provider that throws or a half-written file leaves the server serving what it had. HTTP/3 goes first, since ngtcp2 publishes a generation atomically. Established connections keep the certificate they authenticated with.Transport settings. TCP gets a handshake timeout, the TLS 1.3 ciphersuites and 1.2 cipher list, and a TLS floor taken from the
SslProtocolsthe binding already named - honoured exactly where the set is contiguous and open at the top, and warned about where OpenSSL's minimum-only model cannot express it. QUIC gets a handshake timeout, an idle backstop, the choice between forwarding a migrated client's datagrams and routing them by connection id in BPF, whether to claim a migrated peer's address, the UDP socket buffer and QPACK. kTLS stays available on both directions.Two gaps stated at startup rather than left to be inferred from a client that was let in: revocation checking is not performed, and
ICertificateValidator.Validatedoes not run over QUIC, where ngtcp2 exposes no peer certificate to hand it.Verified
Full acceptance suite green on both target frameworks (1444 net10.0, 2048 net11.0). The sample host was exercised end to end: each SNI name serves its own certificate over HTTP/1.1 and HTTP/2, an unregistered name falls back to the default, the mutual TLS port admits the client its CA signed and refuses the impostor, and
SIGHUPreplaces every certificate on a live server with requests in flight throughout.HTTP/3 is covered by the engine and the sample but was not exercised end to end locally - the available curl has no HTTP/3 support.
Also adds a regression test for the
Connectionheader being compared case-insensitively, across all three engines. Both earlier fixes for that shipped without one, and it is the kind of bug a refactor carries off silently: it was verified by reverting the fix and watching the test fail, not only by watching it pass.