fix(quic): HTTP/3 stream-context validation + ReceiveBuffer perf cliff

Round 3 of the audit follow-ups.

* Http3FrameReader gains a [StreamContext] parameter that enforces
  RFC 9114 §7.2 per-stream rules:
   - CONTROL: first frame MUST be SETTINGS (else H3_MISSING_SETTINGS);
     duplicate SETTINGS, DATA, HEADERS, PUSH_PROMISE all forbidden.
   - REQUEST: SETTINGS / GOAWAY / MAX_PUSH_ID / CANCEL_PUSH forbidden.
   - PUSH: similar set including PUSH_PROMISE.
   - Reserved types 0x02 / 0x06 / 0x08 / 0x09 explicitly rejected.
   - WT_BIDI_DATA / WT_UNI_DATA: reader is the wrong tool, throw.
   - UNCHECKED preserves prior test behaviour and is the default.
  WtPeerStreamDemux's CONTROL drain now constructs the reader with
  StreamContext.CONTROL, so a buggy server can no longer slip a DATA
  frame into our SETTINGS expectations and silently confuse the
  parser. The validation throws QuicCodecException, which the
  drainControlStream catch records on a new peerH3ProtocolError
  field — the QUIC layer / application reads it to close with the
  proper diagnostic instead of having the route() catch swallow it.

* ReceiveBuffer no longer coalesces overlapping segments on insert.
  Pre-fix every reorder fill allocated a fresh merged ByteArray of
  size (hi - lo) and copyInto'd each existing segment — under a 200-
  chunk reorder burst that was O(N²) bytes. The new layout keeps
  segments as a sorted, non-overlapping list (binary-searched on
  insert) and only allocates at readContiguous time, where it walks
  consecutive segments and concats them in a single pass. Adjacent
  segments are not eagerly merged — the read-side concat is bounded
  by the contiguous prefix the consumer is about to drain anyway.
  bufferedAhead becomes O(1) (cached counter) instead of O(N) sum.

* New tests cover the per-context rejection paths (CONTROL-stream
  first-frame check, DATA-on-control, SETTINGS-on-request, all four
  reserved frame types).

All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL.

https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
This commit is contained in:
Claude
2026-05-08 23:30:17 +00:00
parent 5c534b1774
commit 392df0384b
4 changed files with 381 additions and 66 deletions
@@ -33,9 +33,20 @@ import com.vitorpamplona.quic.Varint
* Use one [Http3FrameReader] per HTTP/3 stream. Feed bytes via [push]; drain
* frames via [next] until it returns null.
*
* Unknown frame types (per RFC 9114 §9 — "frame types in the format and intent
* not recognized SHOULD be ignored") are surfaced as [Http3Frame.Unknown] so
* the caller can decide policy. SETTINGS, HEADERS, DATA are typed.
* Stream-context enforcement (RFC 9114 §7.2):
* * On the [StreamContext.CONTROL] stream, the FIRST frame MUST be SETTINGS
* (else H3_MISSING_SETTINGS); DATA / HEADERS / PUSH_PROMISE are not
* allowed (H3_FRAME_UNEXPECTED).
* * On a [StreamContext.REQUEST] stream, SETTINGS / GOAWAY / MAX_PUSH_ID /
* CANCEL_PUSH are not allowed (H3_FRAME_UNEXPECTED).
* * On a [StreamContext.PUSH] stream, SETTINGS / GOAWAY / MAX_PUSH_ID /
* PUSH_PROMISE / CANCEL_PUSH are not allowed.
* * Reserved frame types `0x02, 0x06, 0x08, 0x09` are explicitly rejected
* (H3_FRAME_UNEXPECTED) per RFC 9114 §7.2.8 — only the
* `0x21 + 0x1f * N` reserved-greasing pattern is ignored as Unknown.
* * On [StreamContext.WT_BIDI_DATA] / [StreamContext.WT_UNI_DATA]
* (post-prefix WebTransport stream payload), no HTTP/3 framing applies
* — the caller never feeds bytes through here. Provided for symmetry.
*
* Memory safety:
* * Pending buffered bytes are capped at [maxPendingBytes]; a peer that
@@ -48,15 +59,46 @@ import com.vitorpamplona.quic.Varint
* `ByteArray(len.toInt())` allocation.
*/
class Http3FrameReader(
private val context: StreamContext = StreamContext.UNCHECKED,
private val maxPendingBytes: Int = DEFAULT_MAX_PENDING_BYTES,
private val maxFrameBodyBytes: Int = DEFAULT_MAX_FRAME_BODY_BYTES,
) {
private var buf: ByteArray = ByteArray(0)
private var pos: Int = 0
private var framesEmitted: Int = 0
/** Bytes currently buffered awaiting a complete frame. Diagnostic; tests use this. */
val bufferedBytes: Int get() = buf.size - pos
/**
* Stream context for [Http3FrameReader] frame validation. The reader is
* stateless about which kind of stream it's running on unless the
* caller tells it. [UNCHECKED] preserves pre-fix behaviour for tests
* that intentionally feed mixed frames; production callers should
* specify the actual context.
*/
enum class StreamContext {
/** No per-context validation (pre-fix behaviour, tests). */
UNCHECKED,
/** RFC 9114 §6.2.1 server CONTROL unidi stream. */
CONTROL,
/** RFC 9114 §6.1 client/server REQUEST bidi stream. */
REQUEST,
/** RFC 9114 §6.2.2 PUSH unidi stream. */
PUSH,
/**
* Post-prefix WT bidi/uni stream payload. WebTransport bytes don't
* use HTTP/3 framing — this exists so the type system can prevent
* accidental use of [Http3FrameReader] on raw WT data.
*/
WT_BIDI_DATA,
WT_UNI_DATA,
}
fun push(bytes: ByteArray) {
if (bytes.isEmpty()) return
// Amortized compaction: only shift bytes down when the consumed
@@ -94,14 +136,101 @@ class Http3FrameReader(
}
val bodyEnd = bodyStart + len.toInt()
if (bodyEnd > buf.size) return null // not all body bytes present yet
val type = typeRes.value
validateFrameType(type)
val body = buf.copyOfRange(bodyStart, bodyEnd)
pos = bodyEnd
return when (typeRes.value) {
framesEmitted++
return when (type) {
Http3FrameType.DATA -> Http3Frame.Data(body)
Http3FrameType.HEADERS -> Http3Frame.Headers(body)
Http3FrameType.SETTINGS -> Http3Frame.Settings(Http3Settings.decodeBody(body))
Http3FrameType.GOAWAY -> Http3Frame.Goaway(body)
else -> Http3Frame.Unknown(typeRes.value, body)
else -> Http3Frame.Unknown(type, body)
}
}
/**
* Enforce RFC 9114 §7.2 per-stream-context rules. Throws
* [QuicCodecException] (mapped upstream to H3_FRAME_UNEXPECTED /
* H3_MISSING_SETTINGS) on violation.
*/
private fun validateFrameType(type: Long) {
// RFC 9114 §7.2.8: explicit reserved-and-forbidden frame types.
// Distinct from the reserved-greasing pattern `0x21 + 0x1f * N`
// (which is allowed and falls through as Unknown).
if (type == 0x02L || type == 0x06L || type == 0x08L || type == 0x09L) {
throw QuicCodecException(
"H3_FRAME_UNEXPECTED: reserved HTTP/3 frame type 0x${type.toString(16)}",
)
}
when (context) {
StreamContext.UNCHECKED -> {
Unit
}
StreamContext.CONTROL -> {
// §7.2.4: SETTINGS MUST be the first frame on the
// control stream; any non-SETTINGS first frame =>
// H3_MISSING_SETTINGS.
if (framesEmitted == 0 && type != Http3FrameType.SETTINGS) {
throw QuicCodecException(
"H3_MISSING_SETTINGS: first CONTROL-stream frame was 0x${type.toString(16)}",
)
}
// §7.2.4: a second SETTINGS is also forbidden.
if (framesEmitted > 0 && type == Http3FrameType.SETTINGS) {
throw QuicCodecException(
"H3_FRAME_UNEXPECTED: duplicate SETTINGS on CONTROL stream",
)
}
// §7.2.1, §7.2.2, §7.2.5: DATA / HEADERS / PUSH_PROMISE
// are forbidden on the control stream.
if (type == Http3FrameType.DATA ||
type == Http3FrameType.HEADERS ||
type == Http3FrameType.PUSH_PROMISE
) {
throw QuicCodecException(
"H3_FRAME_UNEXPECTED: 0x${type.toString(16)} on CONTROL stream",
)
}
}
StreamContext.REQUEST -> {
// §7.2.4 / §7.2.6 / §7.2.7: control-only frames are
// forbidden on request streams.
if (type == Http3FrameType.SETTINGS ||
type == Http3FrameType.GOAWAY ||
type == Http3FrameType.MAX_PUSH_ID ||
type == Http3FrameType.CANCEL_PUSH
) {
throw QuicCodecException(
"H3_FRAME_UNEXPECTED: 0x${type.toString(16)} on REQUEST stream",
)
}
}
StreamContext.PUSH -> {
if (type == Http3FrameType.SETTINGS ||
type == Http3FrameType.GOAWAY ||
type == Http3FrameType.MAX_PUSH_ID ||
type == Http3FrameType.PUSH_PROMISE ||
type == Http3FrameType.CANCEL_PUSH
) {
throw QuicCodecException(
"H3_FRAME_UNEXPECTED: 0x${type.toString(16)} on PUSH stream",
)
}
}
StreamContext.WT_BIDI_DATA, StreamContext.WT_UNI_DATA -> {
// WebTransport stream payload doesn't use HTTP/3 framing.
// The caller shouldn't be feeding bytes through this
// reader for those contexts; flag any attempt loudly.
throw QuicCodecException(
"WebTransport stream payload routed through Http3FrameReader",
)
}
}
}
@@ -25,19 +25,31 @@ package com.vitorpamplona.quic.stream
* (or for the per-encryption-level CRYPTO offset stream).
*
* Chunks may arrive in any order with possibly overlapping ranges. The buffer
* coalesces them into a single contiguous prefix that's available to the
* consumer via [readContiguous]. We never expose bytes past the contiguous
* frontier — gaps stall further reads until the missing offsets arrive.
* stores them as **non-overlapping, sorted-by-offset segments** without
* eager coalescing — the contiguous prefix is collected at [readContiguous]
* time. Pre-fix the buffer coalesced overlapping segments on EVERY [insert]
* by allocating a fresh merged `ByteArray((hi - lo).toInt())` and copying
* each existing segment into it; under a 200-chunk reorder burst that's
* O(N²) bytes copied. The new layout is O(1) amortized per [insert] for
* the common cases (adjacent or non-overlapping) and only allocates at
* read time, when the consumer is ready to pull the bytes.
*
* For a fully streamed CRYPTO transcript, the consumer just reads contiguous
* bytes whenever new data arrives; the lookup is O(log N) on the gap tree.
* Invariants on [chunks]:
* * Sorted strictly ascending by `offset`.
* * No two segments overlap, but adjacent segments (one ending at the
* next's start) are allowed.
* * No segment lives below [readOffset].
*
* Implementation: we store raw chunks sorted by offset. Calls to [insert]
* coalesce adjacent and overlapping ranges. [readContiguous] returns the
* range from the current cursor up to the first gap.
* For a fully streamed CRYPTO transcript the consumer just reads contiguous
* bytes whenever new data arrives.
*/
class ReceiveBuffer {
private val chunks = mutableListOf<Chunk>() // sorted by offset, non-overlapping after insert
/** Sorted ascending by [Chunk.offset]; non-overlapping. See class kdoc. */
private val chunks = ArrayDeque<Chunk>()
/** Cached `chunks.sumOf { data.size }` so [bufferedAhead] is O(1). */
private var bufferedAheadBytes: Long = 0L
var readOffset: Long = 0L
private set
@@ -88,9 +100,8 @@ class ReceiveBuffer {
}
if (data.isEmpty()) return InsertResult.OK
val end = offset + data.size
// Drop chunk parts already consumed.
if (end <= readOffset) return InsertResult.OK
// Drop chunk parts already consumed by the reader.
if (frameEnd <= readOffset) return InsertResult.OK
val effOffset: Long
val effData: ByteArray
if (offset < readOffset) {
@@ -102,46 +113,106 @@ class ReceiveBuffer {
effData = data
}
// Find the first chunk that's not strictly before the new range. The
// boundary is `<=` so a perfectly adjacent prior chunk (its endOffset
// equals our offset) is included in the merge — otherwise it would
// stay as a separate adjacent chunk and bufferedAhead() would
// overcount on perfectly-sequential receives starting at offset > 0.
var startIdx = 0
while (startIdx < chunks.size && chunks[startIdx].endOffset() < effOffset) startIdx++
var endIdx = startIdx
while (endIdx < chunks.size && chunks[endIdx].offset <= effOffset + effData.size) endIdx++
// Also pull in the prior chunk if it's exactly adjacent on the lower end.
if (startIdx > 0 && chunks[startIdx - 1].endOffset() == effOffset) startIdx -= 1
if (startIdx == endIdx) {
// No overlap — just insert.
chunks.add(startIdx, Chunk(effOffset, effData))
return InsertResult.OK
}
// Coalesce [startIdx, endIdx) plus the new chunk.
var lo = effOffset
var hi = effOffset + effData.size
for (i in startIdx until endIdx) {
lo = minOf(lo, chunks[i].offset)
hi = maxOf(hi, chunks[i].endOffset())
}
val merged = ByteArray((hi - lo).toInt())
for (i in startIdx until endIdx) {
chunks[i].data.copyInto(merged, (chunks[i].offset - lo).toInt())
}
effData.copyInto(merged, (effOffset - lo).toInt())
// Replace
for (i in 1..(endIdx - startIdx)) chunks.removeAt(startIdx)
chunks.add(startIdx, Chunk(lo, merged))
insertNonOverlapping(effOffset, effData)
return InsertResult.OK
}
/**
* Insert [data] starting at [start] into [chunks], trimming portions
* that already exist (retransmit / overlap). The result preserves the
* non-overlapping, sorted invariants. No allocation when the new
* range is fully covered (retransmit no-op) or when it doesn't
* touch any existing segment (single insert).
*/
private fun insertNonOverlapping(
start: Long,
data: ByteArray,
) {
val end = start + data.size
// Locate the first existing chunk whose `endOffset > start` —
// anything before it is strictly to the left of [start, end).
var idx = lowerBoundEnd(start)
var cursor = start
while (cursor < end) {
val existing = if (idx < chunks.size) chunks[idx] else null
val existingStart = existing?.offset ?: Long.MAX_VALUE
if (existingStart >= end) {
// Tail (or whole new range) sits past every existing
// segment that could overlap. Emit the remainder.
emitSegment(start, data, cursor, end)
cursor = end
} else if (existingStart > cursor) {
// Gap from [cursor, existingStart) — emit it, then skip
// over the existing segment (it covers some of [start,end)).
emitSegment(start, data, cursor, existingStart)
cursor = minOf(existing!!.endOffset, end)
idx++
} else {
// existingStart <= cursor — the existing segment covers
// [cursor, existing.endOffset). Skip ahead.
cursor = minOf(existing!!.endOffset, end)
idx++
}
}
}
/**
* Emit the slice of [src] covering absolute range `[segStart, segEnd)`
* as a new chunk, inserted in sorted order. [src] starts at absolute
* offset [srcStart], so the slice index is `(segStart - srcStart)`.
*/
private fun emitSegment(
srcStart: Long,
src: ByteArray,
segStart: Long,
segEnd: Long,
) {
if (segStart >= segEnd) return
val from = (segStart - srcStart).toInt()
val to = (segEnd - srcStart).toInt()
val piece =
if (from == 0 && to == src.size) {
// Whole src is the segment; avoid the copyOfRange.
src
} else {
src.copyOfRange(from, to)
}
// Insert in sorted position. The index from [lowerBoundOffset]
// is monotonically non-decreasing across the walk in
// [insertNonOverlapping] but we recompute defensively to keep
// this helper self-contained.
val insertAt = lowerBoundOffset(segStart)
chunks.add(insertAt, Chunk(segStart, piece))
bufferedAheadBytes += piece.size
}
/** Index of first chunk whose `endOffset > target`, or `chunks.size`. */
private fun lowerBoundEnd(target: Long): Int {
var lo = 0
var hi = chunks.size
while (lo < hi) {
val mid = (lo + hi) ushr 1
if (chunks[mid].endOffset > target) hi = mid else lo = mid + 1
}
return lo
}
/** Index of first chunk whose `offset >= target`, or `chunks.size`. */
private fun lowerBoundOffset(target: Long): Int {
var lo = 0
var hi = chunks.size
while (lo < hi) {
val mid = (lo + hi) ushr 1
if (chunks[mid].offset >= target) hi = mid else lo = mid + 1
}
return lo
}
/** Highest `offset + data.size` ever seen on this stream (whether or not contiguous). */
fun highestObservedOffset(): Long {
val finEnd = finOffset
val bufEnd = if (chunks.isEmpty()) readOffset else chunks.last().endOffset()
val bufEnd = if (chunks.isEmpty()) readOffset else chunks.last().endOffset
return if (finEnd != null) maxOf(finEnd, bufEnd) else bufEnd
}
@@ -163,19 +234,49 @@ class ReceiveBuffer {
FIN_CONFLICTS_WITH_PRIOR_FIN,
}
/** Returns and consumes the contiguous bytes available starting from [readOffset]. */
/**
* Returns and consumes the contiguous bytes available starting from
* [readOffset]. Walks every consecutive segment whose left edge meets
* the current read frontier and returns them as a single concatenated
* [ByteArray]. Returns an empty array if the next pending segment is
* beyond the current frontier (gap) or the buffer is empty.
*
* Allocation: zero allocations when only one segment is consecutive
* (the most common case — one inserted segment per call); a single
* concat allocation when multiple segments stack up after a gap fill.
*/
fun readContiguous(): ByteArray {
if (chunks.isEmpty()) return ByteArray(0)
val first = chunks[0]
if (first.offset != readOffset) return ByteArray(0)
val data = first.data
readOffset += data.size
chunks.removeAt(0)
return data
if (chunks.isEmpty() || chunks.first().offset != readOffset) return EMPTY
val first = chunks.removeFirst()
bufferedAheadBytes -= first.data.size
var cursor = first.endOffset
// Fast-path: only one consecutive segment.
if (chunks.isEmpty() || chunks.first().offset != cursor) {
readOffset = cursor
return first.data
}
// Multiple consecutive segments — collect, then concat once.
val collected = mutableListOf(first)
var total = first.data.size.toLong()
while (chunks.isNotEmpty() && chunks.first().offset == cursor) {
val next = chunks.removeFirst()
bufferedAheadBytes -= next.data.size
cursor = next.endOffset
collected += next
total += next.data.size
}
val out = ByteArray(total.toInt())
var pos = 0
for (c in collected) {
c.data.copyInto(out, pos)
pos += c.data.size
}
readOffset = cursor
return out
}
/** Bytes already buffered and held back due to gaps. */
fun bufferedAhead(): Long = chunks.sumOf { it.data.size.toLong() }
fun bufferedAhead(): Long = bufferedAheadBytes
/** Highest contiguous offset received so far. */
fun contiguousEnd(): Long = readOffset
@@ -192,6 +293,10 @@ class ReceiveBuffer {
val offset: Long,
val data: ByteArray,
) {
fun endOffset() = offset + data.size
val endOffset: Long get() = offset + data.size
}
private companion object {
private val EMPTY = ByteArray(0)
}
}
@@ -125,6 +125,21 @@ class WtPeerStreamDemux(
var peerGoawayProtocolError: String? = null
private set
/**
* Surface RFC 9114 §7.2 frame-validation violations the
* [Http3FrameReader] catches (H3_FRAME_UNEXPECTED, H3_MISSING_SETTINGS,
* forbidden reserved types). The route()-level `catch (_: Throwable)`
* cancels the per-stream collector but doesn't propagate the
* underlying [com.vitorpamplona.quic.QuicCodecException]'s message
* upstream — without this field a buggy server would just see
* "stream stops being read" with no diagnostic. Stays null until a
* violation is observed; the QUIC-layer caller polls it and closes
* the connection on non-null.
*/
@Volatile
var peerH3ProtocolError: String? = null
private set
val incomingStrippedStreams: Flow<StrippedWtStream> = readyStreams.consumeAsFlow()
/**
@@ -246,13 +261,25 @@ class WtPeerStreamDemux(
pending: ArrayDeque<ByteArray>,
chunkChannel: Channel<ByteArray>,
) {
val reader = Http3FrameReader()
val reader = Http3FrameReader(context = Http3FrameReader.StreamContext.CONTROL)
// Push whatever we already buffered.
while (pending.isNotEmpty()) reader.push(pending.removeFirst())
consumeFrames(reader)
for (chunk in chunkChannel) {
reader.push(chunk)
try {
while (pending.isNotEmpty()) reader.push(pending.removeFirst())
consumeFrames(reader)
for (chunk in chunkChannel) {
reader.push(chunk)
consumeFrames(reader)
}
} catch (e: com.vitorpamplona.quic.QuicCodecException) {
// RFC 9114 §7.2 frame-validation throws (H3_FRAME_UNEXPECTED /
// H3_MISSING_SETTINGS / reserved type) land here. Record the
// diagnostic so the QUIC layer / application can close the
// connection deliberately instead of having the route() catch
// silently swallow the message. Idempotent on duplicate hits.
if (peerH3ProtocolError == null) {
peerH3ProtocolError = e.message ?: "HTTP/3 protocol violation on CONTROL stream"
}
throw e
}
}
@@ -20,10 +20,13 @@
*/
package com.vitorpamplona.quic.http3
import com.vitorpamplona.quic.QuicCodecException
import com.vitorpamplona.quic.QuicWriter
import com.vitorpamplona.quic.webtransport.encodeHeadersFrame
import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNull
import kotlin.test.assertTrue
@@ -90,4 +93,55 @@ class Http3FrameReaderTest {
assertContentEquals(byteArrayOf(0x01, 0x02, 0x03, 0x04), second.body)
assertNull(r.next())
}
@Test
fun control_stream_first_frame_must_be_settings() {
// RFC 9114 §7.2.4: first CONTROL-stream frame is SETTINGS or
// H3_MISSING_SETTINGS. A peer that opens a CONTROL stream and
// immediately sends GOAWAY is malformed.
val r = Http3FrameReader(context = Http3FrameReader.StreamContext.CONTROL)
val w = QuicWriter()
w.writeVarint(Http3FrameType.GOAWAY)
w.writeVarint(0L)
r.push(w.toByteArray())
assertFailsWith<QuicCodecException> { r.next() }
}
@Test
fun control_stream_rejects_data_frame() {
// RFC 9114 §7.2.1: DATA frames are forbidden on the control stream.
val r = Http3FrameReader(context = Http3FrameReader.StreamContext.CONTROL)
// Settings first (legal), then DATA (illegal).
r.push(buildClientWebTransportSettings().encodeFrame())
assertTrue(r.next() is Http3Frame.Settings)
val w = QuicWriter()
w.writeVarint(Http3FrameType.DATA)
w.writeVarint(0L)
r.push(w.toByteArray())
assertFailsWith<QuicCodecException> { r.next() }
}
@Test
fun request_stream_rejects_settings_frame() {
// RFC 9114 §7.2.4: SETTINGS forbidden on request streams.
val r = Http3FrameReader(context = Http3FrameReader.StreamContext.REQUEST)
r.push(buildClientWebTransportSettings().encodeFrame())
assertFailsWith<QuicCodecException> { r.next() }
}
@Test
fun reserved_frame_types_are_rejected() {
// RFC 9114 §7.2.8: types 0x02, 0x06, 0x08, 0x09 are reserved
// and MUST be treated as connection error.
for (forbidden in longArrayOf(0x02L, 0x06L, 0x08L, 0x09L)) {
val r = Http3FrameReader(context = Http3FrameReader.StreamContext.REQUEST)
val w = QuicWriter()
w.writeVarint(forbidden)
w.writeVarint(0L)
r.push(w.toByteArray())
assertFailsWith<QuicCodecException>("type 0x${forbidden.toString(16)} must be rejected") {
r.next()
}
}
}
}