tests: land the review round's reproductions, with the refuted claims turned into what is true

src/ioxide/Tls/TlsService.cs

@@ -205,6 +205,29 @@ private static byte[] Fold(string host)
     /// </param>
     public static TlsService Start(Reactor reactor, TlsOptions options, bool register = true)
     {
+        // Scalars first, because the checks below reason about COMBINATIONS and a value outside its
+        // own domain makes that reasoning meaningless. Both of these resolve to something plausible
+        // rather than failing, which is the shape worth refusing: an undefined version maps to the
+        // TLS 1.2 floor through a not-Tls13 ternary, and a negative timeout disables the handshake
+        // sweep entirely because both readers guard on "> 0" - so the one bound on a peer that
+        // connects and then says nothing is silently off. No config binder validates an enum
+        // (Enum.Parse<TlsProtocolVersion>("3") succeeds), so neither value needs a cast to arrive.
+        if (!Enum.IsDefined(options.MinProtocolVersion))
+        {
+            throw new ArgumentException(
+                $"MinProtocolVersion is {(int)options.MinProtocolVersion}, which is not one of "
+                + "Default, Tls12 or Tls13. Name the floor you want rather than leaving it to be "
+                + "resolved.", nameof(options));
+        }
+
+        if (options.HandshakeTimeoutMs < 0)
+        {
+            throw new ArgumentException(
+                $"HandshakeTimeoutMs is {options.HandshakeTimeoutMs}. Zero disables the handshake "
+                + "sweep; a negative value would disable it too, which is worth saying rather than "
+                + "arriving at by accident.", nameof(options));
+        }
+
         // RX alone cannot be programmed: the handoff shares the TCP_ULP that EnableTx installs.
         // Refuse loudly rather than silently serving the userspace path the caller opted out of.
         if (options.KernelRx && !options.KernelTx)

src/protocols/ioxide.ngtcp2/Connection/QuicEngineConnection.cs

@@ -39,6 +39,11 @@ public unsafe partial class QuicEngineConnection : QuicConnection
     /// whose organisation is <c>Acme\/CN=admin.internal</c> satisfies a
     /// <c>Contains("/CN=admin.internal")</c> check while being a different principal.
     /// <see cref="PeerCommonName"/> is the value to compare instead.
+    ///
+    /// Null also when the DN does not fit the 1024 bytes the shim records, which is a refusal
+    /// rather than an omission: a truncated DN is plausible, comparable, and can equal a DIFFERENT
+    /// principal's prefix, so no name is reported instead of a partial one. Nothing here ever
+    /// hands back a shortened identity.
     /// </remarks>
     public string? PeerSubject
     {
@@ -65,8 +70,11 @@ public string? PeerSubject
     /// with <see cref="StringComparison.Ordinal"/>.
     ///
     /// Null when there was no validated certificate, when the subject carries no CN (legitimate:
-    /// modern certificates identify by subjectAltName), or when the CN is empty or contains an
-    /// embedded NUL, which is a name built to be read differently by different consumers.
+    /// modern certificates identify by subjectAltName), when the CN is empty or contains an
+    /// embedded NUL - a name built to be read differently by different consumers - or when it
+    /// exceeds the 256 bytes recorded for it, which is four times RFC 5280's ub-common-name of 64.
+    /// Every one of those is a refusal rather than an omission: the accessor never reports a name
+    /// it had to shorten, because a prefix can belong to someone else.
     /// </summary>
     public string? PeerCommonName
     {

tests/Ioxide.Tests.E2E/Protocols/H3AlpnTests.cs

@@ -1,22 +1,211 @@
 using ioxide;
+using ioxide.nghttp3;
 using ioxide.ngtcp2;
 
 namespace Ioxide.Tests;
 
 /// <summary>
-/// ALPN as HTTP/3 requires it, and what the negotiated protocol reads back as.
+/// ALPN as HTTP/3 requires it, and what the negotiated protocol reads back as. RFC 9001 section
+/// 8.1 makes ALPN mandatory for QUIC (no mutual protocol = no_application_protocol, during the
+/// handshake); RFC 9114 section 3.1 makes "h3" the token an HTTP/3 server may serve.
 /// </summary>
 /// <remarks>
-/// Reserved for a review pass whose deliverable is a FAILING test. See tests/README.md: a defect
-/// that has been reproduced is committed as <c>runner.Pending</c> - it reports PEND while it still
-/// fails, and fails the run the moment it starts passing.
+/// Two behaviours in this area were examined and could NOT be driven from this suite, so they are
+/// recorded here rather than half-tested:
 ///
-/// Empty is a legitimate outcome. It means the area was examined and nothing was found that could
-/// be made to fail, which is worth more than a test that passes for reasons nobody established.
+/// - Server preference order with a multi-token offer (the shim's iq_on_client_hello walks the
+///   allowlist on the outside, so the server's order decides, like the TCP side's
+///   AlpnNegotiationTests pins). The shim's client entry points hand picotls exactly ONE token -
+///   a single iovec, count = 1 - so no in-tree client can offer several protocols at once.
+///
+/// - The negotiated token reading back as nothing when it exceeds the 64-byte read-back buffer
+///   (iq_conn_get_alpn returns 0 when the token does not fit the buffer
+///   QuicEngineConnection.HandshakeCompletedOnce hands it). A legal ALPN token may be 255 bytes,
+///   but the shim's client stores its offer in a char[64] via snprintf, truncating it to 63 - so
+///   the shortest token that would trip the server's cap cannot be offered from here. The 63-byte
+///   test below pins the longest reachable token instead.
 /// </remarks>
 internal static class H3AlpnTests
 {
     public static void Register(Runner runner)
     {
+        runner.Test("quic/alpn: control - a pinned engine serves an h3 offer and the handler reads back 'h3'", () =>
+        {
+            // The control for every refusal below: the same engine shape, the same handler, the
+            // one offer an HTTP/3 server may accept - and it serves. Also the only place the
+            // SERVER-side NegotiatedProtocol value is asserted: the pure-C# stack consumes it in
+            // its backstop, but nothing else pins that the shim's read-back (iq_conn_get_alpn)
+            // surfaces the very token the client offered.
+            (string certPath, string keyPath) = TestCert.Ensure();
+            using var engine = new QuicEngine(certPath, keyPath, cidLength: 8, alpn: ["h3"]);
+
+            (_, int udpPort) = TestServer.StartDatagram(
+                onDatagram: null,
+                quicFactory: engine.CreateFactory(),
+                quicHandle: (_, conn) => new Nghttp3Connection(conn).RunBufferedAsync(
+                    _ => Nghttp3Response.Text($"alpn={conn.NegotiatedProtocol ?? "(none)"}")));
+
+            using var client = new H3TestClient("127.0.0.1", udpPort);
+            client.Connect();
+            Assert.True(client.CompleteHandshake(timeoutMs: 5000), "handshake did not complete");
+
+            (int status, string body) = client.Get("/alpn", timeoutMs: 5000);
+            Assert.Equal(200, status);
+            Assert.Equal("alpn=h3", body);
+        });
+
+        runner.Test("quic/alpn: a pinned engine refuses a no-overlap offer during the handshake, with a close", () =>
+        {
+            // RFC 9001 section 8.1: no mutual protocol fails the handshake with
+            // no_application_protocol. The engine-side allowlist is the real fix for serving h3
+            // to clients that never claimed it, and every h3 test site now pins ["h3"] - but the
+            // refusal itself was one test deep and asserted only "not served". This pins the
+            // stronger half: the handshake never completes, and the refusal ARRIVES as a close.
+            // PeerClosed is the load-bearing assert - a server that silently dropped the
+            // connection would also fail CompleteHandshake, by timeout, and a hang is not a
+            // refusal (it is also not the alert RFC 9001 requires).
+            (string certPath, string keyPath) = TestCert.Ensure();
+            using var engine = new QuicEngine(certPath, keyPath, cidLength: 8, alpn: ["h3"]);
+
+            (_, int udpPort) = TestServer.StartDatagram(
+                onDatagram: null,
+                quicFactory: engine.CreateFactory(),
+                quicHandle: static (_, conn) => new Nghttp3Connection(conn).RunBufferedAsync(
+                    static _ => Nghttp3Response.Text("must never serve")));
+
+            using var client = new H3TestClient("127.0.0.1", udpPort) { Alpn = "echo" };
+            client.Connect();
+            bool done = client.CompleteHandshake(timeoutMs: 5000);
+            Assert.True(!done, "a pinned engine must not complete a handshake with no ALPN overlap");
+            Assert.True(client.PeerClosed,
+                "the refusal must arrive as a close during the handshake - a timeout is a hang, not a refusal");
+        });
+
+        runner.Test("quic/alpn: a pinned engine refuses a client that offered no ALPN at all", () =>
+        {
+            // The other half of RFC 9001 section 8.1: a client that offers NOTHING. An empty Alpn
+            // makes the shim's client omit the extension entirely (it only hands picotls a list
+            // for a non-empty token), and a pinned server must treat that as no overlap - not as
+            // "nothing to check". The permissive engine's documented default is to confirm even
+            // this; pinning is what closes it, so the pinned refusal is the behaviour to hold.
+            (string certPath, string keyPath) = TestCert.Ensure();
+            using var engine = new QuicEngine(certPath, keyPath, cidLength: 8, alpn: ["h3"]);
+
+            (_, int udpPort) = TestServer.StartDatagram(
+                onDatagram: null,
+                quicFactory: engine.CreateFactory(),
+                quicHandle: static (_, conn) => new Nghttp3Connection(conn).RunBufferedAsync(
+                    static _ => Nghttp3Response.Text("must never serve")));
+
+            using var client = new H3TestClient("127.0.0.1", udpPort) { Alpn = "" };
+            client.Connect();
+            bool done = client.CompleteHandshake(timeoutMs: 5000);
+            Assert.True(!done, "RFC 9001 8.1: a pinned engine must refuse a client that offered no ALPN");
+            Assert.True(client.PeerClosed,
+                "the refusal must arrive as a close during the handshake - a timeout is a hang, not a refusal");
+        });
+
+        runner.Test("quic/alpn: a 63-byte token - the longest the harness can offer - reads back whole", () =>
+        {
+            // The negotiated token reads back through a 64-byte buffer, and iq_conn_get_alpn
+            // answers 0 - "no protocol" - for anything that does not fit, so a shrunk buffer
+            // would not fail loudly: it would report a legal negotiated token as none at all.
+            // 63 bytes is the longest offer the harness client can make (its own char[64] +
+            // snprintf truncation - see the file remarks), which makes it the boundary this
+            // suite can hold: the whole token, not empty, not clipped.
+            //
+            // Recorded at handshake completion rather than through a served response, so this
+            // stays true even once the nghttp3 layer learns to refuse non-h3 connections.
+            string big = new string('a', 63);
+            var recorded = new TaskCompletionSource<string?>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            (string certPath, string keyPath) = TestCert.Ensure();
+            using var engine = new QuicEngine(certPath, keyPath, cidLength: 8);   // permissive: confirms the offer
+
+            (_, int udpPort) = TestServer.StartDatagram(
+                onDatagram: null,
+                quicFactory: engine.CreateFactory(),
+                quicHandle: (_, conn) =>
+                {
+                    ((QuicEngineConnection)conn).HandshakeCompleted =
+                        () => recorded.TrySetResult(conn.NegotiatedProtocol);
+                    return new Nghttp3Connection(conn).RunBufferedAsync(
+                        static _ => Nghttp3Response.Text("ok"));
+                });
+
+            using var client = new H3TestClient("127.0.0.1", udpPort) { Alpn = big };
+            client.Connect();
+            Assert.True(client.CompleteHandshake(timeoutMs: 5000), "handshake did not complete");
+
+            Assert.True(recorded.Task.Wait(5000), "the server never reported handshake completion");
+            Assert.Equal(big, recorded.Task.Result);
+        });
+
+        runner.Pending("h3/nghttp3: a connection that did not negotiate h3 is not served", () =>
+        {
+            // The mirror of Http3Tests' "a connection that did not negotiate h3 is not served",
+            // on the OTHER stack. The backstop landed only in the pure-C# layer
+            // (Http3Connection.RunCoreAsync checks NegotiatedProtocol once the control stream is
+            // up); Nghttp3Connection never reads it, so on an engine built without an allowlist -
+            // the constructor's documented default and its own doc example - it answers HTTP/3 on
+            // a connection that negotiated "echo", a protocol the client actually asked for and
+            // is entitled to believe it got.
+            (string certPath, string keyPath) = TestCert.Ensure();
+            using var engine = new QuicEngine(certPath, keyPath, cidLength: 8);   // permissive, on purpose
+
+            (_, int udpPort) = TestServer.StartDatagram(
+                onDatagram: null,
+                quicFactory: engine.CreateFactory(),
+                quicHandle: static (_, conn) => new Nghttp3Connection(conn).RunBufferedAsync(
+                    static _ => Nghttp3Response.Text("served-by-nghttp3")));
+
+            using var client = new H3TestClient("127.0.0.1", udpPort) { Alpn = "echo" };
+            client.Connect();
+            Assert.True(client.CompleteHandshake(timeoutMs: 5000),
+                "the permissive engine should still complete the handshake - that is the point");
+
+            (int status, string body) = client.Get("/nope", timeoutMs: 3000);
+            Assert.True(status != 200, $"an h3 handler must not serve a non-h3 connection, got {status} '{body}'");
+        }, "the ALPN backstop landed only in the pure-C# stack; Nghttp3Connection never reads "
+         + "NegotiatedProtocol, so a permissive engine's 'echo' connection is answered 200");
+
+        runner.Pending("quic/alpn: a non-ascii allow-list token must not admit a protocol nobody configured", () =>
+        {
+            // QuicEngine.AlpnWire encodes each configured token with Encoding.ASCII, whose
+            // fallback substitutes '?' for anything non-ascii - so ["Ũ2"] goes on the wire as the
+            // allowlist entry "?2". Two consequences: the configured token itself can never
+            // negotiate (a client offering the actual bytes of "Ũ2" finds no match), and every
+            // non-ascii token collapses onto '?', so a client offering the literal "?2" is
+            // admitted, served, and reads back NegotiatedProtocol == "?2" - a protocol nobody
+            // configured. The TCP side's BuildAlpnWire has the same shape with a worse symptom
+            // (UTF-16 units cast to bytes turn "Ũ2" into the real "h2"); the fix on either side
+            // is to refuse a non-ascii token at construction, like the >255-byte one already is,
+            // or to encode it faithfully - both make this body pass.
+            (string certPath, string keyPath) = TestCert.Ensure();
+            QuicEngine engine;
+            try
+            {
+                engine = new QuicEngine(certPath, keyPath, cidLength: 8, alpn: ["Ũ2"]);
+            }
+            catch (ArgumentException)
+            {
+                return;   // refused at configuration - the defect is gone
+            }
+
+            using (engine)
+            {
+                (_, int udpPort) = TestServer.StartDatagram(
+                    onDatagram: null,
+                    quicFactory: engine.CreateFactory(),
+                    quicHandle: static (_, conn) => new Nghttp3Connection(conn).RunBufferedAsync(
+                        static _ => Nghttp3Response.Text("must never serve")));
+
+                using var client = new H3TestClient("127.0.0.1", udpPort) { Alpn = "?2" };
+                client.Connect();
+                Assert.True(!client.CompleteHandshake(timeoutMs: 5000),
+                    "a client offering '?2' completed the handshake against an allow list of 'Ũ2'");
+            }
+        }, "AlpnWire's ASCII '?' substitution puts \"?2\" on the wire for the configured \"Ũ2\", "
+         + "and a client offering the literal \"?2\" is admitted and served");
     }
 }