Skip to content

Conversation

@kixelated
Copy link
Collaborator

@kixelated kixelated commented Feb 10, 2026

Summary

  • Adds an Incoming struct that wraps a QUIC connection before it's fully accepted
  • Allows inspecting the peer address before starting the post-handshake driver

Changes

  • Added ez::Incoming struct with peer_addr() and accept() methods
  • Changed ez::Server::accept() to return Incoming instead of Connection
  • Updated WebTransport server to call incoming.accept() to get the connection

Use Case

This enables servers to inspect the peer address (and potentially other connection metadata in the future) before accepting the connection and starting the driver.

🤖 Generated with Claude Code

@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

Walkthrough

This pull request introduces a new public Incoming wrapper type in the server module that represents pre-accepted QUIC connections. The type provides methods to access peer address, ALPN protocol, and materialize the underlying connection. The server's accept method signature is updated to return Option<Incoming> instead of Option<Connection>, and callers are updated to invoke incoming.accept() to obtain the connection. Minor code reformatting is applied to connection handling patterns for consistency. The Claude Code GitHub Actions workflow is removed.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately describes the main change: adding an Incoming struct to enable inspection of connections before accepting them.
Description check ✅ Passed The description is related to the changeset, explaining the Incoming struct addition and its use case for inspecting peer addresses.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-quiche

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
web-transport-quiche/src/ez/server.rs (1)

240-240: ⚠️ Potential issue | 🟡 Minor

Channel capacity equals the number of sockets (typically 1).

mpsc::channel(sockets.len()) creates a very small buffer. Under load, many connections may complete the TLS handshake concurrently but block on accept.send(...) waiting for Server::accept() to drain. During this wait, resume(running) hasn't been called yet, so the QUIC connection's post-handshake driver isn't running — potentially causing idle timeouts for connections that handshook successfully but aren't being serviced.

Consider using a larger fixed capacity or making it configurable.

🤖 Fix all issues with AI agents
In `@web-transport-quiche/src/h3/connect.rs`:
- Around line 160-163: The caller destructures the result of
connect.into_inner() as a 2-tuple but into_inner now returns three items
(ez::SendStream, ez::RecvStream, Vec<u8>), so update the pattern where
connect.into_inner() is used (e.g. the let (_send, mut recv) =
connect.into_inner() site) to unpack all three values and either bind or ignore
the leftover buffer (e.g. use a third binding like _buf) so the code compiles
and correctly captures the leftover bytes.
🧹 Nitpick comments (3)
web-transport-quiche/src/h3/connect.rs (2)

53-69: Unbounded read loop — consider a size cap to mitigate slow-loris-style abuse.

If a peer sends a never-completing CONNECT frame, buf grows without bound (limited only by QUIC flow-control windows). A reasonable upper-bound check (e.g. a few KB) with an error return would make this more robust against misbehaving peers.

Example guard
+const MAX_CONNECT_HEADER_SIZE: usize = 16 * 1024;
+
 let mut buf = Vec::new();
 let request = loop {
     if recv.read_buf(&mut buf).await?.is_none() {
         return Err(ConnectError::UnexpectedEnd);
     }
+    if buf.len() > MAX_CONNECT_HEADER_SIZE {
+        return Err(ConnectError::Proto(web_transport_proto::ConnectError::UnexpectedEnd));
+    }
 
     let mut cursor = std::io::Cursor::new(buf.as_slice());

113-129: Duplicate decode-loop pattern — consider extracting a helper.

The buffered decode loop in open (lines 113-129) is structurally identical to the one in accept (lines 53-69), differing only in the type being decoded. A generic helper (e.g. async fn decode_buffered<T: Decode>(recv, buf) -> Result<T, ConnectError>) would eliminate this duplication and ensure the same size-cap guard applies to both paths.

The same unbounded-growth concern from the accept loop applies here.

web-transport-quiche/src/ez/server.rs (1)

265-285: Handshake errors are silently swallowed.

When initial.handshake(driver).await? fails, the ? propagates into the tokio::spawn async block's Result, but since no one inspects the JoinHandle, the error is silently dropped. Adding a tracing::warn! for failed handshakes would improve operational visibility.

Suggested improvement
 tokio::spawn(async move {
     let accept_bi = flume::unbounded();
     let accept_uni = flume::unbounded();

     let state = Lock::new(DriverState::new(true));
     let driver = Driver::new(state.clone(), accept_bi.0, accept_uni.0);

-    // Wait for the TLS handshake to complete so ALPN is available.
-    let (qconn, running) = initial.handshake(driver).await?;
+    // Wait for the TLS handshake to complete so ALPN is available.
+    let (qconn, running) = match initial.handshake(driver).await {
+        Ok(v) => v,
+        Err(e) => {
+            tracing::debug!("TLS handshake failed: {e}");
+            return Ok(());
+        }
+    };
     let connection = Connection::new(qconn, state, accept_bi.1, accept_uni.1);

Comment on lines 160 to 163
/// Returns the inner streams and any leftover bytes from reading the CONNECT handshake.
pub fn into_inner(self) -> (ez::SendStream, ez::RecvStream, Vec<u8>) {
(self.send, self.recv, self.buf)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n --type rust 'into_inner' -C 3

Repository: moq-dev/web-transport

Length of output: 15632


🏁 Script executed:

cat -n web-transport-quiche/src/connection.rs | sed -n '110,120p'

Repository: moq-dev/web-transport

Length of output: 527


Breaking API change on into_inner — update caller to handle 3-tuple.

into_inner now returns (SendStream, RecvStream, Vec<u8>) instead of a 2-tuple. At web-transport-quiche/src/connection.rs:115, the destructuring pattern let (_send, mut recv) = connect.into_inner(); attempts to unpack only 2 values from a 3-tuple and will fail to compile. Update to unpack all 3 values:

let (_send, mut recv, _buf) = connect.into_inner();
🤖 Prompt for AI Agents
In `@web-transport-quiche/src/h3/connect.rs` around lines 160 - 163, The caller
destructures the result of connect.into_inner() as a 2-tuple but into_inner now
returns three items (ez::SendStream, ez::RecvStream, Vec<u8>), so update the
pattern where connect.into_inner() is used (e.g. the let (_send, mut recv) =
connect.into_inner() site) to unpack all three values and either bind or ignore
the leftover buffer (e.g. use a third binding like _buf) so the code compiles
and correctly captures the leftover bytes.

Adds an Incoming struct that wraps a QUIC connection before it's fully
accepted. This allows inspecting the peer address before starting the
post-handshake driver.

The Server::accept() method now returns Incoming instead of Connection,
and callers must call incoming.accept() to get the Connection.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Add Claude Code workflow and merge latest changes Add Incoming struct to inspect connections before accepting Feb 10, 2026
@kixelated kixelated merged commit 2559877 into main Feb 10, 2026
1 check passed
@kixelated kixelated deleted the fix-quiche branch February 10, 2026 16:07
This was referenced Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant