From 02015a7ce111e4d0cfd5b5d78711ad0db7586936 Mon Sep 17 00:00:00 2001 From: Richard Wheeler Date: Sun, 28 Jun 2026 08:23:18 -0400 Subject: [PATCH] =?UTF-8?q?net.http:=20validate=20HTTP/2=20server=20reques?= =?UTF-8?q?t=20headers=20(RFC=209113=20=C2=A78.1.2)=20(#27569)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * net.http: validate HTTP/2 server request headers (RFC 9113 §8.1.2) The minimal h2 server accepted any decoded header list. Add request-side validation so malformed requests are rejected as a stream error (RST_STREAM(PROTOCOL_ERROR)) per RFC 9113 §8.1.1: - field names must be lowercase (§8.1.2) and non-empty - connection-specific fields and TE != "trailers" are forbidden (§8.2.2) - request pseudo-headers: only the known set, no duplicates, all before any regular field, with :method/:path/:scheme present and non-empty (§8.1.2.1, §8.1.2.3, §8.3) - declared content-length must equal the DATA payload length (§8.1.2.6), with the value charset-checked before parsing (a successful .int() is not validation) - a trailing HEADERS block on an open stream is accepted as trailers (§8.1): decoded to keep HPACK in sync, validated (no pseudo-headers), then dispatched Validation runs at finalize_headers (after the HPACK decode, so the dynamic table stays in sync even when the request is rejected); the connection survives. The connection-specific header list is now a shared const reused by the response path. Verified with the h2spec conformance gate (#27563) in CI's configuration (v2.6.0, --timeout 5): the suite goes from 109 to 120 passing. This closes 12 baselined cases — the §8.1.2/§8.1.2.x/§8.3 request-validation set, generic/4 (POST with trailers), and 6.9.2 (negative window via SETTINGS, which the existing flow-control loop already handled correctly once the request is accepted) — all verified stable across repeated runs and removed from the known-failure baseline. Adds scripted-peer unit tests for each case. Co-Authored-By: WOZCODE * net.http: h2 server — RST malformed trailers; validate every content-length Address Codex review on #27569: - A trailer section (a 2nd HEADERS block on an open stream) that lacks END_STREAM is malformed. It was returned as an error, which bubbles through serve() and GOAWAYs the whole connection; make it a stream error (RST_STREAM(PROTOCOL_ERROR)) like the other request validation, keeping the connection alive. The trailer block is now always decoded before the well-formedness check, so rejecting a bad trailer section does not desync the HPACK dynamic table for later streams. - build_request validated only the last content-length field, so a conflicting duplicate (e.g. "content-length: 5" then "content-length: 0") was accepted. Reject differing duplicate content-length values (RFC 9110 §8.6). Adds unit tests for both: trailers without END_STREAM and conflicting content-length each get RST_STREAM(PROTOCOL_ERROR). Co-Authored-By: WOZCODE * net.http: h2 server — reject NUL/CR/LF octets in request field values RFC 9113 §8.2.1: a field value MUST NOT contain NUL (0x00), LF (0x0a), or CR (0x0d). The new request validation checked field names but not values, so a value with an embedded CR/LF was accepted and passed through into req.header — a malformed request and, if the request were forwarded to an HTTP/1.x peer, a header-injection / request-smuggling vector. Reject such values (and the same octets in pseudo-header values) as a stream error. Found by a requirements-driven review pass (RFC §8.2.1) — not covered by the h2spec suite. Adds a unit test (CR/LF in a field value -> RST_STREAM). Co-Authored-By: WOZCODE * net.http: h2 server — HPACK decode errors are connection-level COMPRESSION_ERROR Address Codex review on #27569: a trailing-HEADERS (and request-HEADERS) HPACK decode failure was answered with RST_STREAM. H2HpackDecoder.decode mutates the dynamic table as it processes a block, so a decode failure leaves the server's decoder permanently out of sync with the peer — every subsequent header block on the connection would be corrupted. RFC 9113 §4.3 requires a connection error of type COMPRESSION_ERROR. finalize_headers and finalize_trailers now send_goaway(.compression_error) and return on a decode failure (closing the connection), instead of resetting one stream. Well-decoded but malformed requests/trailers stay stream errors (RST_STREAM(PROTOCOL_ERROR)) — only the decode failure escalates. Verified with the h2spec gate (v2.6.0, --timeout 5): 120 -> 129 passing, no regressions. This closes the whole HPACK error-scope cluster — 9 cases (hpack/2.3.3 x2, 4.2, 5.2 x3, 6.1, 6.3, http2/4.3) — now removed from the baseline. Adds a unit test (an index-0 HPACK block -> GOAWAY(COMPRESSION_ERROR)). Co-Authored-By: WOZCODE * net.http: reuse h2_conn_specific_headers from h2_mux_conn.v Rebase onto master after #27413 (H2MuxConn) merged: that PR added a module-level `h2_conn_specific_headers` const (the RFC 9113 §8.2.2 connection-specific set) in h2_mux_conn.v. This branch had defined the same const in h2_server.v, which now collides (duplicate const). Drop the server copy and share the existing one. --------- Co-authored-by: WOZCODE --- vlib/net/http/h2_server.v | 264 ++++++++++++- vlib/net/http/h2_server_test.v | 350 ++++++++++++++++++ .../http/h2spec/h2spec_expected_failures.txt | 46 +-- 3 files changed, 608 insertions(+), 52 deletions(-) diff --git a/vlib/net/http/h2_server.v b/vlib/net/http/h2_server.v index 0b71d8fe8..68fddc242 100644 --- a/vlib/net/http/h2_server.v +++ b/vlib/net/http/h2_server.v @@ -22,17 +22,147 @@ const h2_server_default_window = u32(65535) // rejected with RST_STREAM(REFUSED_STREAM). const h2_server_max_request_body = 8 * 1024 * 1024 +// The connection-specific header fields RFC 9113 §8.2.2 forbids in HTTP/2 (a +// request carrying one is malformed; the server must never echo one in a +// response) are the module-level `h2_conn_specific_headers` const defined in +// h2_mux_conn.v — shared with the client path rather than redefined here. + +// h2_all_digits reports whether s is a non-empty run of ASCII digits. A value +// that parses with `.int()` is NOT proof of a valid number — `'12x'.int()` is 12 +// — so wire-derived numerics (content-length) must be charset-checked first. +fn h2_all_digits(s string) bool { + if s.len == 0 { + return false + } + for ch in s { + if ch < `0` || ch > `9` { + return false + } + } + return true +} + +// h2_field_value_has_forbidden_octet reports whether a field value contains an +// octet RFC 9113 §8.2.1 forbids: NUL (0x00), LF (0x0a), or CR (0x0d). Such a +// value makes the request malformed and, if the request were forwarded to an +// HTTP/1.x peer, would be a header-injection / request-smuggling vector. +fn h2_field_value_has_forbidden_octet(value string) bool { + for ch in value { + if ch == 0 || ch == 0x0a || ch == 0x0d { + return true + } + } + return false +} + +// h2_request_field_error returns a non-empty reason when a regular (non-pseudo) +// request header field is malformed per RFC 9113 §8.2: names must be lowercase +// and non-empty, values must not contain NUL/CR/LF, connection-specific fields +// are forbidden, and TE may only carry the value "trailers". An empty return +// means the field is valid. +fn h2_request_field_error(name string, value string) string { + if name.len == 0 { + return 'empty header field name' + } + if name != name.to_lower() { + return 'uppercase header field name "${name}"' + } + if h2_field_value_has_forbidden_octet(value) { + return 'forbidden NUL/CR/LF octet in value of "${name}"' + } + if name in h2_conn_specific_headers { + return 'connection-specific header field "${name}"' + } + if name == 'te' && value.trim_space().to_lower() != 'trailers' { + return 'TE header field with a value other than "trailers"' + } + return '' +} + +// h2_validate_request_pseudo enforces the RFC 9113 §8.1.2/§8.3 rules a request +// header list must satisfy: only the request pseudo-headers, each at most once, +// all appearing before any regular field, with :method, :path and :scheme +// present; every regular field must pass h2_request_field_error. A non-ok return +// means the request is malformed (a stream error at the call site). +fn h2_validate_request_pseudo(headers []H2HeaderField) ! { + mut seen_regular := false + mut has_method := false + mut has_path := false + mut has_scheme := false + mut has_authority := false + for f in headers { + if f.name.starts_with(':') { + if seen_regular { + return error('pseudo-header "${f.name}" after a regular field') + } + if h2_field_value_has_forbidden_octet(f.value) { + return error('forbidden NUL/CR/LF octet in pseudo-header "${f.name}"') + } + match f.name { + ':method' { + if has_method { + return error('duplicate :method pseudo-header') + } + if f.value == '' { + return error('empty :method pseudo-header') + } + has_method = true + } + ':path' { + if has_path { + return error('duplicate :path pseudo-header') + } + // RFC 9113 §8.3.1: :path must not be empty for http/https. + if f.value == '' { + return error('empty :path pseudo-header') + } + has_path = true + } + ':scheme' { + if has_scheme { + return error('duplicate :scheme pseudo-header') + } + if f.value == '' { + return error('empty :scheme pseudo-header') + } + has_scheme = true + } + ':authority' { + if has_authority { + return error('duplicate :authority pseudo-header') + } + has_authority = true + } + else { + return error('unknown request pseudo-header "${f.name}"') + } + } + } else { + seen_regular = true + reason := h2_request_field_error(f.name, f.value) + if reason != '' { + return error(reason) + } + } + } + if !has_method || !has_path || !has_scheme { + return error('request omits a mandatory pseudo-header (:method/:path/:scheme)') + } +} + // H2ServerStream holds the state of one in-flight server-side stream. struct H2ServerStream { mut: - id u32 - hpack_block []u8 // assembled HEADERS + CONTINUATION fragments - headers []H2HeaderField - body []u8 - end_headers bool - end_stream bool - headers_done bool // set once we've decoded the HPACK block - send_window i64 + id u32 + hpack_block []u8 // assembled HEADERS + CONTINUATION fragments + headers []H2HeaderField + body []u8 + end_headers bool + end_stream bool + headers_done bool // set once we've decoded the HPACK block + send_window i64 + trailer_block []u8 // assembled trailer HEADERS + CONTINUATION fragments + in_trailers bool // set once a trailer section (a 2nd HEADERS block) begins } // H2ServerConn drives one server-side HTTP/2 connection over a transport. @@ -264,6 +394,12 @@ fn (mut c H2ServerConn) apply_settings(settings []H2Setting) ! { } fn (mut c H2ServerConn) on_headers(frame H2HeadersFrame, mut handler Handler) ! { + // A HEADERS block for a stream that is already open is a trailer section + // (RFC 9113 §8.1), not a new request. + if mut existing := c.streams[frame.stream_id] { + c.on_trailers(mut existing, frame, mut handler)! + return + } // Stream ids from the client must be odd and strictly increasing. if frame.stream_id & 1 == 0 || frame.stream_id <= c.last_stream_id { return error('h2 server: invalid client stream id ${frame.stream_id}') @@ -288,6 +424,17 @@ fn (mut c H2ServerConn) on_continuation(frame H2ContinuationFrame, mut handler H mut s := c.streams[frame.stream_id] or { return error('h2 server: CONTINUATION for unknown stream ${frame.stream_id}') } + if s.in_trailers { + if s.trailer_block.len + frame.fragment.len > h2_max_recv_header_block { + return error('h2 server: trailer block exceeds ${h2_max_recv_header_block} bytes') + } + s.trailer_block << frame.fragment + if frame.end_headers { + c.awaiting_cont = 0 + c.finalize_trailers(mut s, mut handler)! + } + return + } if s.hpack_block.len + frame.fragment.len > h2_max_recv_header_block { return error('h2 server: request header block exceeds ${h2_max_recv_header_block} bytes') } @@ -300,17 +447,81 @@ fn (mut c H2ServerConn) on_continuation(frame H2ContinuationFrame, mut handler H } fn (mut c H2ServerConn) finalize_headers(mut s H2ServerStream, mut handler Handler) ! { + // An HPACK decoding failure is a CONNECTION error (RFC 9113 §4.3): the decoder + // mutates the dynamic table as it runs, so a failure leaves our table out of + // sync with the peer and no later header block on this connection can be decoded + // reliably. Close the connection with COMPRESSION_ERROR rather than RST-ing one + // stream. (send_goaway sets c.closing, so serve()'s catch won't re-GOAWAY.) s.headers = c.decoder.decode(s.hpack_block) or { - c.send_rst_stream(s.id, .compression_error)! + c.send_goaway(.compression_error, 'HPACK decode error') or {} + return error('h2 server: HPACK decode error (COMPRESSION_ERROR)') + } + s.headers_done = true + // A well-decoded but malformed request is a STREAM error (RFC 9113 §8.1.1) — + // reset just this stream and keep the connection alive. + h2_validate_request_pseudo(s.headers) or { + c.send_rst_stream(s.id, .protocol_error)! c.streams.delete(s.id) return } - s.headers_done = true if s.end_stream { c.run_request(mut s, mut handler)! } } +// on_trailers handles a second HEADERS block on an already-open stream: an +// HTTP/2 trailer section (RFC 9113 §8.1). The block is always assembled and +// decoded; whether it is well-formed — it must carry END_STREAM and no +// pseudo-headers — is decided in finalize_trailers. A well-decoded but malformed +// trailer section is reset as a stream error; an HPACK decode failure is a +// connection error (§4.3) since it desyncs the decoder. +fn (mut c H2ServerConn) on_trailers(mut s H2ServerStream, frame H2HeadersFrame, mut handler Handler) ! { + s.in_trailers = true + s.end_stream = frame.end_stream + s.trailer_block << frame.fragment + if !frame.end_headers { + c.awaiting_cont = frame.stream_id + return + } + c.finalize_trailers(mut s, mut handler)! +} + +fn (mut c H2ServerConn) finalize_trailers(mut s H2ServerStream, mut handler Handler) ! { + // An HPACK decoding failure is a CONNECTION error (RFC 9113 §4.3): the decoder's + // dynamic table is now out of sync with the peer. Close the connection rather + // than RST one stream. + trailers := c.decoder.decode(s.trailer_block) or { + c.send_goaway(.compression_error, 'HPACK decode error in trailers') or {} + return error('h2 server: HPACK decode error in trailers (COMPRESSION_ERROR)') + } + // A well-decoded but malformed trailer section is a STREAM error (§8.1.1). + mut reason := if !s.end_stream { + // A trailer section MUST terminate the stream (RFC 9113 §8.1). + 'trailing HEADERS without END_STREAM' + } else { + '' + } + if reason == '' { + for f in trailers { + // Trailers carry no pseudo-headers and follow the §8.2 field rules. + reason = if f.name.starts_with(':') { + 'pseudo-header "${f.name}" in trailers' + } else { + h2_request_field_error(f.name, f.value) + } + if reason != '' { + break + } + } + } + if reason != '' { + c.send_rst_stream(s.id, .protocol_error)! + c.streams.delete(s.id) + return + } + c.run_request(mut s, mut handler)! +} + fn (mut c H2ServerConn) on_data(frame H2DataFrame, mut handler Handler) ! { mut s := c.streams[frame.stream_id] or { // DATA for an unknown stream (likely already RST'd); just drop and @@ -366,10 +577,12 @@ fn (mut c H2ServerConn) build_request(s &H2ServerStream) !Request { version: .v2_0 header: new_header() } + // Pseudo-headers were already validated in h2_validate_request_pseudo + // (presence, no duplicates, ordering, allowed set); here we only extract. mut method := '' mut path := '' mut authority := '' - mut scheme := 'https' + mut content_length := -1 for f in s.headers { match f.name { ':method' { @@ -382,18 +595,34 @@ fn (mut c H2ServerConn) build_request(s &H2ServerStream) !Request { authority = f.value } ':scheme' { - scheme = f.value + // Validated already; handlers infer the scheme from Host, matching + // the HTTP/1.1 path. } else { if f.name.starts_with(':') { - return error('h2 server: unknown pseudo-header ${f.name}') + continue + } + if f.name == 'content-length' { + if !h2_all_digits(f.value) { + return error('h2 server: malformed content-length "${f.value}"') + } + cl := f.value.int() + // Multiple content-length fields with differing values are a + // malformed request (RFC 9110 §8.6); validate every occurrence, + // not just the last. + if content_length >= 0 && content_length != cl { + return error('h2 server: conflicting content-length values ${content_length} and ${cl}') + } + content_length = cl } req.header.add_custom(f.name, f.value) or {} } } } - if method == '' || path == '' { - return error('h2 server: missing :method or :path') + // RFC 9113 §8.1.2.6: a declared content-length MUST equal the sum of the DATA + // payload lengths; a mismatch is a malformed request (stream error). + if content_length >= 0 && content_length != s.body.len { + return error('h2 server: content-length ${content_length} != DATA length ${s.body.len}') } req.method = method_from_str(method) if authority != '' && !req.header.contains(.host) { @@ -401,7 +630,6 @@ fn (mut c H2ServerConn) build_request(s &H2ServerStream) !Request { } // Match the HTTP/1.1 path: req.url is the request-target (the :path // pseudo-header), so handlers see the same shape on both transports. - _ = scheme // :scheme is parsed and discarded; handlers infer it from Host req.url = path req.data = s.body.bytestr() req.host = authority @@ -413,8 +641,8 @@ fn (mut c H2ServerConn) send_response(stream_id u32, resp Response) ! { mut fields := [H2HeaderField{':status', status.str()}] for key in resp.header.keys() { lkey := key.to_lower() - // Drop hop-by-hop headers; HTTP/2 forbids them (RFC 7540 §8.1.2.2). - if lkey in ['connection', 'keep-alive', 'transfer-encoding', 'upgrade', 'proxy-connection'] { + // Drop hop-by-hop headers; HTTP/2 forbids them (RFC 9113 §8.2.2). + if lkey in h2_conn_specific_headers { continue } for val in resp.header.custom_values(key) { diff --git a/vlib/net/http/h2_server_test.v b/vlib/net/http/h2_server_test.v index b4442dce0..e5fad077f 100644 --- a/vlib/net/http/h2_server_test.v +++ b/vlib/net/http/h2_server_test.v @@ -303,3 +303,353 @@ fn test_h2_server_respects_send_window() { // The first DATA frame must not exceed the initial 10-byte window. assert first_data_len <= 10 } + +// spawn_h2_echo_server starts the h2 server driver on the server end of a fresh +// pipe and returns the client end for raw-frame scripting. +fn spawn_h2_echo_server() &PipeEnd { + mut client_end, mut server_end := new_pipe() + mut handler_iface := Handler(ServerEchoHandler{}) + spawn fn [mut server_end, mut handler_iface] () { + mut transport := H2Transport(server_end) + serve_h2_conn(mut transport, mut handler_iface) or {} + }() + return client_end +} + +// drive_until_rst_or_response writes `out` to the server, then reads frames back +// until it sees a RST_STREAM on stream 1 (returns the error code) or a HEADERS +// frame (a response — returns -1, meaning the request was accepted). It never +// blocks indefinitely: the server always answers a malformed request with +// RST_STREAM and a valid one with HEADERS. +fn drive_until_rst_or_response(mut client_end PipeEnd, out []u8, label string) i64 { + client_end.write(out) or { + assert false, '${label}: client write failed: ${err}' + return -2 + } + mut fr := FrameReader{ + end: client_end + } + for _ in 0 .. 32 { + f := fr.next() or { break } + match f { + H2RstStreamFrame { + if f.stream_id == 1 { + return i64(f.error_code) + } + } + H2HeadersFrame { + return -1 + } + else {} + } + } + assert false, '${label}: server sent neither RST_STREAM nor a response' + return -2 +} + +// malformed_headers_out builds preface + SETTINGS + a single complete HEADERS +// frame (END_HEADERS, END_STREAM) carrying `fields`. +fn malformed_headers_out(fields []H2HeaderField) []u8 { + mut enc := H2HpackEncoder{} + block := enc.encode(fields) + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: block + end_headers: true + end_stream: true + }).encode() + return out +} + +fn assert_request_malformed(fields []H2HeaderField, label string) { + mut client_end := spawn_h2_echo_server() + code := drive_until_rst_or_response(mut client_end, malformed_headers_out(fields), label) + assert code == i64(u32(H2ErrorCode.protocol_error)), '${label}: expected RST_STREAM(PROTOCOL_ERROR), got ${code}' +} + +const valid_get_pseudo = [ + H2HeaderField{':method', 'GET'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/'}, +] + +// RFC 9113 §8.1.2: a field name with uppercase letters is malformed. +fn test_h2_server_rejects_uppercase_header() { + mut fields := valid_get_pseudo.clone() + fields << H2HeaderField{'X-Test', 'v'} + assert_request_malformed(fields, 'uppercase field name') +} + +// RFC 9113 §8.1.2.1: a pseudo-header after a regular field is malformed. +fn test_h2_server_rejects_pseudo_after_regular() { + fields := [ + H2HeaderField{'x-a', '1'}, + H2HeaderField{':method', 'GET'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/'}, + ] + assert_request_malformed(fields, 'pseudo after regular') +} + +// RFC 9113 §8.1.2.2: TE may only carry the value "trailers". +fn test_h2_server_rejects_te_not_trailers() { + mut fields := valid_get_pseudo.clone() + fields << H2HeaderField{'te', 'gzip'} + assert_request_malformed(fields, 'TE other than trailers') +} + +// RFC 9113 §8.1.2.2: connection-specific fields are forbidden. +fn test_h2_server_rejects_connection_specific_header() { + mut fields := valid_get_pseudo.clone() + fields << H2HeaderField{'connection', 'keep-alive'} + assert_request_malformed(fields, 'connection-specific field') +} + +// RFC 9113 §8.1.2.3: omitting :scheme is malformed. +fn test_h2_server_rejects_missing_scheme() { + fields := [ + H2HeaderField{':method', 'GET'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/'}, + ] + assert_request_malformed(fields, 'missing :scheme') +} + +// RFC 9113 §8.1.2.3: a duplicated request pseudo-header is malformed. +fn test_h2_server_rejects_duplicate_pseudo() { + for dup in [ + H2HeaderField{':method', 'GET'}, + H2HeaderField{':path', '/'}, + H2HeaderField{':scheme', 'https'}, + ] { + mut fields := valid_get_pseudo.clone() + fields << dup + assert_request_malformed(fields, 'duplicate ${dup.name}') + } +} + +// RFC 9113 §8.2.1: a field value containing NUL/CR/LF is malformed (and would be +// a header-injection vector if forwarded to an HTTP/1.x peer). +fn test_h2_server_rejects_control_char_in_value() { + mut fields := valid_get_pseudo.clone() + fields << H2HeaderField{'x-evil', 'a\r\nInjected: 1'} + assert_request_malformed(fields, 'CR/LF in field value') +} + +// RFC 9113 §8.3.1: an empty :path pseudo-header is malformed for http/https. +fn test_h2_server_rejects_empty_path() { + fields := [ + H2HeaderField{':method', 'GET'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', ''}, + ] + assert_request_malformed(fields, 'empty :path') +} + +// RFC 9113 §8.1.2.6: content-length must equal the DATA payload length. +fn test_h2_server_rejects_content_length_mismatch() { + mut enc := H2HpackEncoder{} + mut fields := [ + H2HeaderField{':method', 'POST'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/upload'}, + H2HeaderField{'content-length', '5'}, + ] + block := enc.encode(fields) + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: block + end_headers: true + end_stream: false + }).encode() + // Two DATA frames totalling 2 bytes, not the declared 5 (covers the + // "sum of multiple DATA frames" variant too). + out << H2Frame(H2DataFrame{ + stream_id: 1 + data: 'h'.bytes() + end_stream: false + }).encode() + out << H2Frame(H2DataFrame{ + stream_id: 1 + data: 'i'.bytes() + end_stream: true + }).encode() + mut client_end := spawn_h2_echo_server() + code := drive_until_rst_or_response(mut client_end, out, 'content-length mismatch') + assert code == i64(u32(H2ErrorCode.protocol_error)), 'content-length mismatch: expected RST_STREAM(PROTOCOL_ERROR), got ${code}' +} + +// RFC 9110 §8.6: conflicting duplicate content-length fields are malformed — +// every occurrence is validated, not just the last. +fn test_h2_server_rejects_conflicting_content_length() { + mut enc := H2HpackEncoder{} + block := enc.encode([ + H2HeaderField{':method', 'POST'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/upload'}, + H2HeaderField{'content-length', '5'}, + H2HeaderField{'content-length', '0'}, + ]) + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: block + end_headers: true + end_stream: true + }).encode() + mut client_end := spawn_h2_echo_server() + code := drive_until_rst_or_response(mut client_end, out, 'conflicting content-length') + assert code == i64(u32(H2ErrorCode.protocol_error)), 'conflicting content-length: expected RST_STREAM(PROTOCOL_ERROR), got ${code}' +} + +// RFC 9113 §4.3: an HPACK decoding error is a CONNECTION error +// (GOAWAY COMPRESSION_ERROR), not a stream reset — the decoder's dynamic table is +// desynced, so the whole connection must close. +fn test_h2_server_hpack_error_closes_connection() { + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + // HEADERS with an invalid HPACK block: indexed header field with index 0 + // (0x80), which RFC 7541 §6.1 forbids -> decode error. + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: [u8(0x80)] + end_headers: true + end_stream: true + }).encode() + mut client_end := spawn_h2_echo_server() + client_end.write(out) or { + assert false, 'hpack error: client write failed: ${err}' + return + } + mut fr := FrameReader{ + end: client_end + } + mut code := i64(-1) + for _ in 0 .. 32 { + f := fr.next() or { break } + match f { + H2GoawayFrame { + code = i64(f.error_code) + break + } + H2RstStreamFrame { + // Wrong scope: a per-stream reset for an HPACK error. + code = -2 + break + } + else {} + } + } + assert code == i64(u32(H2ErrorCode.compression_error)), 'HPACK decode error must be GOAWAY(COMPRESSION_ERROR), got ${code}' +} + +// RFC 9113 §8.1: a trailer section that does not carry END_STREAM is malformed. +// It must be a STREAM error (RST_STREAM) — the connection stays up — not a GOAWAY. +fn test_h2_server_rejects_trailers_without_end_stream() { + mut enc := H2HpackEncoder{} + req_block := enc.encode([ + H2HeaderField{':method', 'POST'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/upload'}, + ]) + trailer_block := enc.encode([ + H2HeaderField{'x-checksum', 'abc123'}, + ]) + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: req_block + end_headers: true + end_stream: false + }).encode() + out << H2Frame(H2DataFrame{ + stream_id: 1 + data: 'hi'.bytes() + end_stream: false + }).encode() + // Trailer HEADERS missing END_STREAM. + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: trailer_block + end_headers: true + end_stream: false + }).encode() + mut client_end := spawn_h2_echo_server() + code := drive_until_rst_or_response(mut client_end, out, 'trailers without END_STREAM') + assert code == i64(u32(H2ErrorCode.protocol_error)), 'trailers without END_STREAM: expected RST_STREAM(PROTOCOL_ERROR), got ${code}' +} + +// RFC 9113 §8.1: a POST with a trailer section (a 2nd HEADERS block ending the +// stream) is accepted and dispatched. +fn test_h2_server_accepts_post_with_trailers() { + mut enc := H2HpackEncoder{} + req_block := enc.encode([ + H2HeaderField{':method', 'POST'}, + H2HeaderField{':scheme', 'https'}, + H2HeaderField{':authority', 'h.example'}, + H2HeaderField{':path', '/upload'}, + ]) + trailer_block := enc.encode([ + H2HeaderField{'x-checksum', 'abc123'}, + ]) + mut out := []u8{} + out << h2_client_preface.bytes() + out << H2Frame(H2SettingsFrame{}).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: req_block + end_headers: true + end_stream: false + }).encode() + out << H2Frame(H2DataFrame{ + stream_id: 1 + data: 'hi'.bytes() + end_stream: false + }).encode() + out << H2Frame(H2HeadersFrame{ + stream_id: 1 + fragment: trailer_block + end_headers: true + end_stream: true + }).encode() + + mut client_end := spawn_h2_echo_server() + client_end.write(out) or { + assert false, 'trailers POST: client write failed: ${err}' + return + } + mut fr := FrameReader{ + end: client_end + } + mut dec := H2HpackDecoder{} + mut status := 0 + for _ in 0 .. 32 { + f := fr.next() or { break } + if f is H2HeadersFrame { + for hf in dec.decode(f.fragment) or { []H2HeaderField{} } { + if hf.name == ':status' { + status = hf.value.int() + } + } + break + } + } + assert status == 200, 'trailers POST should succeed, got status ${status}' +} diff --git a/vlib/net/http/h2spec/h2spec_expected_failures.txt b/vlib/net/http/h2spec/h2spec_expected_failures.txt index 73a8be3e7..9f8dea2b7 100644 --- a/vlib/net/http/h2spec/h2spec_expected_failures.txt +++ b/vlib/net/http/h2spec/h2spec_expected_failures.txt @@ -7,29 +7,18 @@ # a REGRESSION (a case not listed here that newly fails). When a listed case starts # passing, run_h2spec.sh warns so the line can be removed and the gate tightened. # -# Generated with h2spec v2.6.0 against the net.http server at this branch's base -# (146 tests: 109 pass, 37 fail). The set was identical across four runs (h2spec -# --timeout 2 and --timeout 5, two each) — stable. NOTE: a handful of OTHER h2spec -# cases (CONTINUATION sequencing, unknown error codes) are timing-flaky at h2spec's -# 2s default — they wait for the server's GOAWAY/close — so run_h2spec.sh uses -# --timeout 5 (H2SPEC_TIMEOUT) to keep the gate stable. Do not add a flaky case to -# this list; raise the timeout instead. These 37 are genuine server-side conformance -# gaps: stricter request -# validation (uppercase/duplicate/missing pseudo-headers, connection-specific & TE -# headers, content-length vs DATA), HPACK decode error SCOPE (must be a -# connection-level COMPRESSION_ERROR, not RST_STREAM), the stream state machine -# (frames on idle/closed streams), CONTINUATION sequencing, and flow-control -# overflow/zero-increment handling. Remove a line when the server is fixed to pass. -generic/4 :: Sends a POST request with trailers -hpack/2.3.3 :: Sends a indexed header field representation with invalid index -hpack/2.3.3 :: Sends a literal header field representation with invalid index -hpack/4.2 :: Sends a dynamic table size update at the end of header block -hpack/5.2 :: Sends a Huffman-encoded string literal representation containing the EOS symbol -hpack/5.2 :: Sends a Huffman-encoded string literal representation padded by zero -hpack/5.2 :: Sends a Huffman-encoded string literal representation with padding longer than 7 bits -hpack/6.1 :: Sends a indexed header field representation with index 0 -hpack/6.3 :: Sends a dynamic table size update larger than the value of SETTINGS_HEADER_TABLE_SIZE -http2/4.3 :: Sends invalid header block fragment +# Generated with h2spec v2.6.0 (146 tests). The original baseline was 37 failures +# (109 pass). Request header/pseudo-header validation (RFC 9113 §8.1.2/§8.2.2/§8.3) +# plus HPACK decode-error scope (§4.3: a decode failure is a connection-level +# COMPRESSION_ERROR, not RST_STREAM) closed 21 of them — 129 pass, 16 fail — +# verified stable at --timeout 5. NOTE: a handful of h2spec cases (CONTINUATION +# sequencing, unknown error codes) are timing-flaky at h2spec's 2s default — they +# wait for the server's GOAWAY/close — so run_h2spec.sh uses --timeout 5 +# (H2SPEC_TIMEOUT) to keep the gate stable. Do not add a flaky case to this list; +# raise the timeout instead. The remaining 16 are genuine server-side conformance +# gaps: the stream state machine (frames on idle/closed streams), concurrent-stream +# limit, PRIORITY self-dependency, SETTINGS_ENABLE_PUSH validation, and flow-control +# zero-increment/overflow handling. Remove a line when the server is fixed to pass. http2/5.1 :: closed: Sends a DATA frame http2/5.1 :: closed: Sends a DATA frame after sending RST_STREAM frame http2/5.1 :: half closed (remote): Sends a DATA frame @@ -46,14 +35,3 @@ http2/6.9 :: Sends a WINDOW_UPDATE frame with a flow control window increment of http2/6.9 :: Sends a WINDOW_UPDATE frame with a flow control window increment of 0 on a stream http2/6.9.1 :: Sends multiple WINDOW_UPDATE frames increasing the flow control window to above 2^31-1 http2/6.9.1 :: Sends multiple WINDOW_UPDATE frames increasing the flow control window to above 2^31-1 on a stream -http2/6.9.2 :: Sends a SETTINGS frame for window size to be negative -http2/8.1.2 :: Sends a HEADERS frame that contains the header field name in uppercase letters -http2/8.1.2.1 :: Sends a HEADERS frame that contains a pseudo-header field that appears in a header block after a regular header field -http2/8.1.2.2 :: Sends a HEADERS frame that contains the TE header field with any value other than "trailers" -http2/8.1.2.2 :: Sends a HEADERS frame that contains the connection-specific header field -http2/8.1.2.3 :: Sends a HEADERS frame that omits ":scheme" pseudo-header field -http2/8.1.2.3 :: Sends a HEADERS frame with duplicated ":method" pseudo-header field -http2/8.1.2.3 :: Sends a HEADERS frame with duplicated ":path" pseudo-header field -http2/8.1.2.3 :: Sends a HEADERS frame with duplicated ":scheme" pseudo-header field -http2/8.1.2.6 :: Sends a HEADERS frame with the "content-length" header field which does not equal the DATA frame payload length -http2/8.1.2.6 :: Sends a HEADERS frame with the "content-length" header field which does not equal the sum of the multiple DATA frames payload length -- 2.39.5