Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
members = [
"web-transport",
"web-transport-proto",
"web-transport-quiche",
"web-transport-quinn",
"web-transport-trait",
"web-transport-wasm",
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
75 changes: 75 additions & 0 deletions web-demo/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// @ts-expect-error embed the certificate fingerprint using bundler
import fingerprintHex from "bundle-text:../dev/localhost.hex";

// Convert the hex to binary.
const fingerprint = [];
for (let c = 0; c < fingerprintHex.length - 1; c += 2) {
fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16));
}

const params = new URLSearchParams(window.location.search);

const url = params.get("url") || "https://localhost:4443";
const datagram = params.get("datagram") || false;
Comment on lines +10 to +13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix datagram parameter parsing logic.

Line 13 has a logic error: params.get("datagram") returns either a string or null, so || false will evaluate to the string value (e.g., "false") rather than a boolean. Any non-empty string (including "false") is truthy in JavaScript.

Apply this fix:

-const datagram = params.get("datagram") || false;
+const datagram = params.get("datagram") === "true";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const params = new URLSearchParams(window.location.search);
const url = params.get("url") || "https://localhost:4443";
const datagram = params.get("datagram") || false;
const params = new URLSearchParams(window.location.search);
const url = params.get("url") || "https://localhost:4443";
const datagram = params.get("datagram") === "true";
🤖 Prompt for AI Agents
In web-demo/client.js around lines 10 to 13 the datagram parsing uses
`params.get("datagram") || false` which preserves string values like "false" as
truthy; change the logic to explicitly convert the query value to a boolean,
e.g., replace that line with a comparison that yields true only when the
parameter equals "true" (or use params.has("datagram") if presence-only
semantics are desired), ensuring the result is a real boolean.


function log(msg) {
const element = document.createElement("div");
element.innerText = msg;

document.body.appendChild(element);
}

async function run() {
// Connect using the hex fingerprint in the cert folder.
const transport = new WebTransport(url, {
serverCertificateHashes: [
{
algorithm: "sha-256",
value: new Uint8Array(fingerprint),
},
],
});
await transport.ready;

log("connected");

let writer;
let reader;

if (!datagram) {
// Create a bidirectional stream
const stream = await transport.createBidirectionalStream();
log("created stream");

writer = stream.writable.getWriter();
reader = stream.readable.getReader();
} else {
log("using datagram");

// Create a datagram
writer = transport.datagrams.writable.getWriter();
reader = transport.datagrams.readable.getReader();
}

// Create a message
const msg = "Hello, world!";
const encoded = new TextEncoder().encode(msg);

await writer.write(encoded);
await writer.close();
writer.releaseLock();

log("send: " + msg);

// Read a message from it
// TODO handle partial reads
const { value } = await reader.read();

const recv = new TextDecoder().decode(value);
log("recv: " + recv);

transport.close();
log("closed");
}

run();
File renamed without changes.
File renamed without changes.
3 changes: 3 additions & 0 deletions web-transport-proto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ categories = ["network-programming", "web-programming"]
bytes = "1"
http = "1"
thiserror = "2"

# Just for AsyncRead and AsyncWrite traits
tokio = { version = "1", default-features = false }
url = "2"
29 changes: 28 additions & 1 deletion web-transport-proto/src/capsule.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use bytes::{Buf, BufMut, Bytes};
use bytes::{Buf, BufMut, Bytes, BytesMut};
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};

use crate::{VarInt, VarIntUnexpectedEnd};

Expand Down Expand Up @@ -68,6 +69,22 @@ impl Capsule {
}
}

pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> {
let mut buf = Vec::new();
loop {
stream
.read_buf(&mut buf)
.await
.map_err(|_| CapsuleError::UnexpectedEnd)?;
let mut limit = std::io::Cursor::new(&buf);
match Self::decode(&mut limit) {
Ok(capsule) => return Ok(capsule),
Err(CapsuleError::UnexpectedEnd) => continue,
Err(e) => return Err(e),
}
}
}
Comment on lines 74 to 88
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider limiting buffer growth to prevent memory exhaustion.

The read method uses an unbounded Vec that grows with each iteration until decode succeeds. If the stream continuously sends data without forming a valid capsule, this could lead to excessive memory consumption.

Consider adding a maximum buffer size check:

 pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, CapsuleError> {
     let mut buf = Vec::new();
     loop {
+        if buf.len() > MAX_MESSAGE_SIZE {
+            return Err(CapsuleError::MessageTooLong);
+        }
         stream
             .read_buf(&mut buf)
             .await

Note: This pattern appears in other files (connect.rs, settings.rs) and should be addressed consistently across the codebase.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In web-transport-proto/src/capsule.rs around lines 72 to 86, the read method
uses an unbounded Vec that can grow indefinitely; add a maximum buffer size
constant (e.g. MAX_CAPSULE_BUF_BYTES) and enforce it by checking buffer length
before/after each read and returning a CapsuleError::PayloadTooLarge (or a new
suitable error variant) when exceeded; ensure you check prior to continuing the
loop to avoid attempting decode on oversized buffers, and apply the same pattern
to connect.rs and settings.rs for consistency.


pub fn encode<B: BufMut>(&self, buf: &mut B) {
match self {
Self::CloseWebTransportSession {
Expand Down Expand Up @@ -101,6 +118,16 @@ impl Capsule {
}
}
}

pub async fn write<S: AsyncWrite + Unpin>(&self, stream: &mut S) -> Result<(), CapsuleError> {
let mut buf = BytesMut::new();
self.encode(&mut buf);
stream
.write_all_buf(&mut buf)
.await
.map_err(|_| CapsuleError::UnexpectedEnd)?;
Ok(())
}
}

fn is_grease(val: u64) -> bool {
Expand Down
55 changes: 54 additions & 1 deletion web-transport-proto/src/connect.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::str::FromStr;

use bytes::{Buf, BufMut};
use bytes::{Buf, BufMut, BytesMut};
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use url::Url;

use super::{qpack, Frame, VarInt};
Expand Down Expand Up @@ -97,6 +98,22 @@ impl ConnectRequest {
Ok(Self { url })
}

pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, ConnectError> {
let mut buf = Vec::new();
loop {
stream
.read_buf(&mut buf)
.await
.map_err(|_| ConnectError::UnexpectedEnd)?;
let mut limit = std::io::Cursor::new(&buf);
match Self::decode(&mut limit) {
Ok(request) => return Ok(request),
Err(ConnectError::UnexpectedEnd) => continue,
Err(e) => return Err(e),
}
}
}

pub fn encode<B: BufMut>(&self, buf: &mut B) {
let mut headers = qpack::Headers::default();
headers.set(":method", "CONNECT");
Expand All @@ -118,6 +135,16 @@ impl ConnectRequest {
size.encode(buf);
buf.put_slice(&tmp);
}

pub async fn write<S: AsyncWrite + Unpin>(&self, stream: &mut S) -> Result<(), ConnectError> {
let mut buf = BytesMut::new();
self.encode(&mut buf);
stream
.write_all_buf(&mut buf)
.await
.map_err(|_| ConnectError::UnexpectedEnd)?;
Ok(())
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -148,6 +175,22 @@ impl ConnectResponse {
Ok(Self { status })
}

pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, ConnectError> {
let mut buf = Vec::new();
loop {
stream
.read_buf(&mut buf)
.await
.map_err(|_| ConnectError::UnexpectedEnd)?;
let mut limit = std::io::Cursor::new(&buf);
match Self::decode(&mut limit) {
Ok(response) => return Ok(response),
Err(ConnectError::UnexpectedEnd) => continue,
Err(e) => return Err(e),
}
}
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

Propagate EOF when reading the response

Same issue on the response side: a zero-length read_buf signals EOF, so the loop should fail fast instead of hanging.(tokio.rs)(tokio.rs)

-            stream
-                .read_buf(&mut buf)
-                .await
-                .map_err(|_| ConnectError::UnexpectedEnd)?;
+            let read = stream
+                .read_buf(&mut buf)
+                .await
+                .map_err(|_| ConnectError::UnexpectedEnd)?;
+            if read == 0 {
+                return Err(ConnectError::UnexpectedEnd);
+            }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In web-transport-proto/src/connect.rs around lines 182 to 191, the loop treats
any Ok read as progress but doesn't handle a zero-length read (EOF), which
causes the loop to hang; change the read handling to capture the number of bytes
read, return Err(ConnectError::UnexpectedEnd) immediately when that number is 0,
and only proceed to construct the Cursor and decode when bytes_read > 0. Ensure
errors from the await are still mapped to ConnectError::UnexpectedEnd and that
the EOF case is detected and propagated before attempting to decode.

}

pub fn encode<B: BufMut>(&self, buf: &mut B) {
let mut headers = qpack::Headers::default();
headers.set(":status", self.status.as_str());
Expand All @@ -162,4 +205,14 @@ impl ConnectResponse {
size.encode(buf);
buf.put_slice(&tmp);
}

pub async fn write<S: AsyncWrite + Unpin>(&self, stream: &mut S) -> Result<(), ConnectError> {
let mut buf = BytesMut::new();
self.encode(&mut buf);
stream
.write_all_buf(&mut buf)
.await
.map_err(|_| ConnectError::UnexpectedEnd)?;
Ok(())
}
}
10 changes: 5 additions & 5 deletions web-transport-proto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
const ERROR_FIRST: u64 = 0x52e4a40fa8db;
const ERROR_LAST: u64 = 0x52e5ac983162;

pub fn error_from_http3(code: u64) -> Option<u32> {
if !(ERROR_FIRST..=ERROR_LAST).contains(&code) {
pub const fn error_from_http3(code: u64) -> Option<u32> {
if code < ERROR_FIRST || code > ERROR_LAST {
return None;
}

let code = code - ERROR_FIRST;
let code = code / 0x1f;
let code = code - code / 0x1f;

Some(code.try_into().unwrap())
Some(code as u32)
}

pub fn error_to_http3(code: u32) -> u64 {
pub const fn error_to_http3(code: u32) -> u64 {
ERROR_FIRST + code as u64 + code as u64 / 0x1e
}
38 changes: 37 additions & 1 deletion web-transport-proto/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use std::{
ops::{Deref, DerefMut},
};

use bytes::{Buf, BufMut};
use bytes::{Buf, BufMut, BytesMut};

use thiserror::Error;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};

use super::{Frame, StreamUni, VarInt, VarIntUnexpectedEnd};

Expand Down Expand Up @@ -96,6 +97,9 @@ pub enum SettingsError {

#[error("invalid size")]
InvalidSize,

#[error("unsupported")]
Unsupported,
}

// A map of settings to values.
Expand Down Expand Up @@ -128,11 +132,32 @@ impl Settings {
Ok(settings)
}

pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, SettingsError> {
let mut buf = Vec::new();

loop {
stream
.read_buf(&mut buf)
.await
.map_err(|_| SettingsError::UnexpectedEnd)?;

// Look at the buffer we've already read.
let mut limit = std::io::Cursor::new(&buf);

match Settings::decode(&mut limit) {
Ok(settings) => return Ok(settings),
Err(SettingsError::UnexpectedEnd) => continue, // More data needed.
Err(e) => return Err(e),
};
}
}
Comment on lines 142 to 159
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider limiting buffer growth to prevent memory exhaustion.

Similar to the issue in capsule.rs, the read method uses an unbounded Vec that could grow excessively if the stream sends continuous data without forming valid settings. This pattern appears in multiple files and should be addressed consistently.

Consider adding a buffer size limit before the read operation or after accumulating data.

🤖 Prompt for AI Agents
In web-transport-proto/src/settings.rs around lines 135 to 153, the read method
appends into an unbounded Vec which can grow indefinitely; add a fixed maximum
buffer size (e.g. MAX_SETTINGS_BUF_BYTES) and check buf.len() (or buf.len() plus
incoming read size) before/after appending and return a specific error (e.g.
SettingsError::BufferOverflow or reuse an appropriate existing variant) when the
limit is exceeded; also avoid repeated growth by discarding bytes already
consumed by the decoder (use the Cursor position to drain or split_off the
consumed prefix) so only unread bytes are retained.


pub fn encode<B: BufMut>(&self, buf: &mut B) {
StreamUni::CONTROL.encode(buf);
Frame::SETTINGS.encode(buf);

// Encode to a temporary buffer so we can learn the length.
// TODO avoid doing this, just use a fixed size varint.
let mut tmp = Vec::new();
for (id, value) in &self.0 {
id.encode(&mut tmp);
Expand All @@ -143,6 +168,17 @@ impl Settings {
buf.put_slice(&tmp);
}

pub async fn write<S: AsyncWrite + Unpin>(&self, stream: &mut S) -> Result<(), SettingsError> {
// TODO avoid allocating to the heap
let mut buf = BytesMut::new();
self.encode(&mut buf);
stream
.write_all_buf(&mut buf)
.await
.map_err(|_| SettingsError::UnexpectedEnd)?;
Ok(())
}

pub fn enable_webtransport(&mut self, max_sessions: u32) {
let max = VarInt::from_u32(max_sessions);

Expand Down
47 changes: 46 additions & 1 deletion web-transport-proto/src/varint.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Based on Quinn: https://github.com/quinn-rs/quinn/tree/main/quinn-proto/src
// Licensed under Apache-2.0 OR MIT

use std::{convert::TryInto, fmt};
use std::{convert::TryInto, fmt, io::Cursor};

use bytes::{Buf, BufMut};
use thiserror::Error;
use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};

/// An integer less than 2^62
///
Expand Down Expand Up @@ -162,6 +163,31 @@ impl VarInt {
Ok(Self(x))
}

// Read a varint from the stream.
pub async fn read<S: AsyncRead + Unpin>(stream: &mut S) -> Result<Self, VarIntUnexpectedEnd> {
// 8 bytes is the max size of a varint
let mut buf = [0; 8];

// Read the first byte because it includes the length.
stream
.read_exact(&mut buf[0..1])
.await
.map_err(|_| VarIntUnexpectedEnd)?;

// 0b00 = 1, 0b01 = 2, 0b10 = 4, 0b11 = 8
let size = 1 << (buf[0] >> 6);
stream
.read_exact(&mut buf[1..size])
.await
.map_err(|_| VarIntUnexpectedEnd)?;

// Use a cursor to read the varint on the stack.
let mut cursor = Cursor::new(&buf[..size]);
let v = VarInt::decode(&mut cursor).unwrap();

Ok(v)
}

pub fn encode<B: BufMut>(&self, w: &mut B) {
let x = self.0;
if x < 2u64.pow(6) {
Expand All @@ -176,6 +202,25 @@ impl VarInt {
unreachable!("malformed VarInt")
}
}

pub async fn write<S: AsyncWrite + Unpin>(
&self,
stream: &mut S,
) -> Result<(), VarIntUnexpectedEnd> {
// Super jaink but keeps everything on the stack.
let mut buf = [0u8; 8];
let mut cursor: &mut [u8] = &mut buf;
self.encode(&mut cursor);
let size = 8 - cursor.len();

let mut cursor = &buf[..size];
stream
.write_all_buf(&mut cursor)
.await
.map_err(|_| VarIntUnexpectedEnd)?;

Ok(())
}
}

/// Error returned when constructing a `VarInt` from a value >= 2^62
Expand Down
Loading
Loading