test(rust): make hyper-util's tunnel defects a test rather than a paragraph

This crate tunnels `CONNECT` itself because `hyper_util`'s `Tunnel` gets two cases wrong: a
status line arriving split across reads falls into its catch-all refusal, and only `200` opens
the tunnel where RFC 9110 says any 2xx does. Both are on hyper-util's main branch today, not
only in the published 0.1.20.

Written down, that justification goes stale silently. As a test it expires by itself: the day
a release fixes either, this goes red and says what to delete. `tunnel_through` is also the
shape the replacement would take, so the work is half done when the notice arrives.

Measured against `Tunnel`, with an unsplit 200 as the control:

    plain 200: ACCEPTED, as it should be
    201: REFUSED with `tunnel error: unsuccessful`
    split: REFUSED with `tunnel error: unsuccessful`

The firing path was checked by making the 201 case answer 200:

    hyper-util now opens the tunnel on a 2xx other than 200. Good news: check the
    split-status-line case too, and if that is fixed as well, delete this crate's `tunnel` and
    use `hyper_util`'s. See #699.

Nagle is off on the fixture's socket and the halves are 50ms apart, so a coalesced read cannot
make the split case pass for the wrong reason; the message names that possibility anyway,
since a false "upstream fixed it" is the one failure worth guarding against.

`client-legacy` joins the declared features. This crate names `HttpConnector`, which lives
behind it, and until now the feature was only on because `hyper-rustls` turns it on.
`Cargo.lock` is unchanged: nothing new enters the graph.

cargo test -p armonik-transport --all-features: 75 passed, 0 failed, and the new target five
times over. clippy --all-targets --all-features -Dwarnings, cargo fmt --all --check, cargo
build --workspace --locked --all-features: clean.

Author: wkirschenmann

packages/rust/armonik-transport/Cargo.toml

@@ -20,9 +20,16 @@ tonic = { workspace = true, features = ["channel", "codegen"] }
 snafu.workspace = true
 tracing.workspace = true
 hyper = { workspace = true, features = ["client", "http1", "http2"] }
-# `client-proxy` for the proxy matcher: the `*_PROXY` convention, `NO_PROXY` on curl's rules, and
-# taking credentials out of a proxy URL, none of which is worth writing again here.
-hyper-util = { workspace = true, features = ["client", "client-proxy", "http1"] }
+# `client-legacy` for `HttpConnector`, which this crate names: `hyper-rustls` happens to turn it on, and
+# depending on that would break the day it stops. `client-proxy` for the proxy matcher: the `*_PROXY`
+# convention, `NO_PROXY` on curl's rules, and taking credentials out of a proxy URL, none of which is
+# worth writing again here.
+hyper-util = { workspace = true, features = [
+  "client",
+  "client-legacy",
+  "client-proxy",
+  "http1",
+] }
 hyper-rustls = { workspace = true, features = [
   "http1",
   "http2",

packages/rust/armonik-transport/tests/upstream_tunnel.rs

@@ -0,0 +1,137 @@
+//! Why this crate tunnels `CONNECT` itself instead of using `hyper_util`'s `Tunnel`.
+//!
+//! These tests assert that `Tunnel` still gets two cases wrong. They are meant to fail: when a
+//! `hyper-util` release fixes either, the failure is the notice that this crate's own tunnel can go,
+//! and `tunnel_through` below is the shape it would be replaced by. Deleting ours is tracked by #699,
+//! fixing them upstream by #702.
+//!
+//! A dependency bump turning CI red is the point. Read the failure before assuming it is a regression.
+
+use hyper::Uri;
+use hyper_util::client::legacy::connect::proxy::Tunnel;
+use hyper_util::client::legacy::connect::HttpConnector;
+use hyper_util::rt::TokioIo;
+use tokio::io::{AsyncReadExt, AsyncWriteExt};
+use tokio::net::{TcpListener, TcpStream};
+use tower_service::Service;
+
+/// How long the split case waits between the two halves of its status line.
+///
+/// Long enough that the reader, which is already waiting, takes the first half on its own rather than
+/// finding both halves in one read. Nagle is off on that socket for the same reason.
+const SPLIT_DELAY: std::time::Duration = std::time::Duration::from_millis(50);
+
+/// A proxy that answers any `CONNECT` with `head`, in one write or in two.
+///
+/// It never dials the target: what is under test is how the response is read, so there is nothing to
+/// tunnel to.
+async fn spawn_proxy(head: &'static str, split_at: Option<usize>) -> std::net::SocketAddr {
+    let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind proxy");
+    let address = listener.local_addr().expect("proxy address");
+
+    tokio::spawn(async move {
+        let Ok((mut client, _)) = listener.accept().await else {
+            return;
+        };
+        // So the first half of a split status line leaves on its own segment rather than waiting for
+        // the second and arriving as one read, which would make the split case pass for the wrong
+        // reason.
+        let _ = client.set_nodelay(true);
+
+        // Read to the blank line that ends the request head.
+        let mut seen = Vec::new();
+        loop {
+            let mut byte = [0u8; 1];
+            if client.read_exact(&mut byte).await.is_err() {
+                return;
+            }
+            seen.push(byte[0]);
+            if seen.ends_with(b"\r\n\r\n") {
+                break;
+            }
+        }
+
+        // A closed connection is a different failure from a rejected status line, so the writes are
+        // best effort and the socket is held open afterwards.
+        match split_at {
+            None => {
+                let _ = client.write_all(head.as_bytes()).await;
+            }
+            Some(at) => {
+                let _ = client.write_all(&head.as_bytes()[..at]).await;
+                let _ = client.flush().await;
+                tokio::time::sleep(SPLIT_DELAY).await;
+                let _ = client.write_all(&head.as_bytes()[at..]).await;
+            }
+        }
+        let _ = client.flush().await;
+        tokio::time::sleep(std::time::Duration::from_secs(2)).await;
+    });
+
+    address
+}
+
+/// Open a tunnel with `hyper_util`'s connector, which is what this crate would use if it could.
+async fn tunnel_through(proxy: std::net::SocketAddr) -> Result<TcpStream, String> {
+    let proxy_uri = Uri::try_from(format!("http://{proxy}")).expect("proxy uri");
+    let mut connector = HttpConnector::new();
+    connector.enforce_http(false);
+
+    Tunnel::new(proxy_uri, connector)
+        .call(Uri::try_from("https://example.invalid:443").expect("target uri"))
+        .await
+        .map(TokioIo::into_inner)
+        .map_err(|error| error.to_string())
+}
+
+/// The control. Without it, a broken fixture would read as upstream still being broken.
+#[tokio::test]
+async fn a_plain_200_opens_the_tunnel() {
+    let proxy = spawn_proxy("HTTP/1.1 200 Connection established\r\n\r\n", None).await;
+
+    tunnel_through(proxy)
+        .await
+        .expect("the fixture itself must let an ordinary 200 through");
+}
+
+#[tokio::test]
+async fn hyper_util_still_refuses_a_2xx_that_is_not_200() {
+    // RFC 9110: any 2xx switches a `CONNECT` connection to tunnel mode. `Tunnel` compares the status
+    // line against `200` alone, so a proxy answering 201 is treated as a refusal.
+    let proxy = spawn_proxy("HTTP/1.1 201 Connection established\r\n\r\n", None).await;
+
+    let error = tunnel_through(proxy).await.err().unwrap_or_else(|| {
+        panic!(
+            "hyper-util now opens the tunnel on a 2xx other than 200. \
+             Good news: check the split-status-line case too, and if that is fixed as well, \
+             delete this crate's `tunnel` and use `hyper_util`'s. See #699."
+        )
+    });
+
+    assert!(
+        error.contains("unsuccessful"),
+        "unexpected failure: {error}"
+    );
+}
+
+#[tokio::test]
+async fn hyper_util_still_refuses_a_status_line_split_across_reads() {
+    // Cut inside `HTTP/1.1 200`, so the first read carries `HTTP/1.1 2`, which matches neither prefix
+    // `Tunnel` looks for and falls into its catch-all refusal. Legal, and likelier with a slow proxy
+    // or a small MSS.
+    let proxy = spawn_proxy("HTTP/1.1 200 Connection established\r\n\r\n", Some(10)).await;
+
+    let error = tunnel_through(proxy).await.err().unwrap_or_else(|| {
+        panic!(
+            "hyper-util now reads a split status line correctly, or the two writes reached it as \
+             one read. Rule the second out before believing the first: the halves are 50ms apart \
+             on a socket with Nagle off. If it really is fixed, check the 2xx case too, and if that \
+             is fixed as well, delete this crate's `tunnel` and use `hyper_util`'s. See #699."
+        )
+    });
+
+    assert!(
+        error.contains("unsuccessful"),
+        "unexpected failure: {error}"
+    );
+}