From 9c73b7b47ed13f2d8956477084e4cc441991cec0 Mon Sep 17 00:00:00 2001 From: phantomas Date: Sun, 9 Nov 2025 21:59:18 +0100 Subject: [PATCH 1/2] spec/h2_stream_spec: make sure trailers are properly handled on response Signed-off-by: phantomas --- spec/h2_stream_spec.lua | 60 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/spec/h2_stream_spec.lua b/spec/h2_stream_spec.lua index 1cfed79d..955ebb69 100644 --- a/spec/h2_stream_spec.lua +++ b/spec/h2_stream_spec.lua @@ -47,6 +47,66 @@ describe("http.h2_stream", function() assert_loop(cq, TEST_TIMEOUT) assert.truthy(cq:empty()) end) + it("accepts trailers without pseudo_header", function() + local s, c = new_pair() + local cq = cqueues.new() + cq:wrap(function() + local client_stream = c:new_stream() + local req_headers = new_headers() + req_headers:append(":method", "GET") + req_headers:append(":scheme", "http") + req_headers:append(":path", "/") + assert(client_stream:write_headers(req_headers, true)) + assert(client_stream:get_headers()) + assert(client_stream:get_headers()) + assert(c:close()) + end) + cq:wrap(function() + local stream = assert(s:get_next_incoming_stream()) + assert(stream:get_headers()) + local res_headers = new_headers() + res_headers:append(":status", "200") + local res_trailers = new_headers() + res_trailers:append("trailer-status", "0") + assert(stream:write_headers(res_headers, false)) + assert(stream:write_headers(res_trailers, true)) + assert(s:close()) + end) + assert_loop(cq, TEST_TIMEOUT) + assert.truthy(cq:empty()) + end) + it("accepts trailers after interim responses", function() + local s, c = new_pair() + local cq = cqueues.new() + cq:wrap(function() + local client_stream = c:new_stream() + local req_headers = new_headers() + req_headers:append(":method", "GET") + req_headers:append(":scheme", "http") + req_headers:append(":path", "/") + assert(client_stream:write_headers(req_headers, true)) + assert(client_stream:get_headers()) -- 100 continue + assert(client_stream:get_headers()) -- 200 OK + assert(client_stream:get_headers()) -- trailer + assert(c:close()) + end) + cq:wrap(function() + local stream = assert(s:get_next_incoming_stream()) + assert(stream:get_headers()) + local info_headers = new_headers() + info_headers:append(":status", "100") + local res_headers = new_headers() + res_headers:append(":status", "200") + local res_trailers = new_headers() + res_trailers:append("trailer-status", "0") + assert(stream:write_headers(info_headers, false)) + assert(stream:write_headers(res_headers, false)) + assert(stream:write_headers(res_trailers, true)) + assert(s:close()) + end) + assert_loop(cq, TEST_TIMEOUT) + assert.truthy(cq:empty()) + end) it("can send a body", function() local s, c = new_pair() local cq = cqueues.new() From e879651d525ed4a0c29d6f9eb6fcfa163c08e068 Mon Sep 17 00:00:00 2001 From: phantomas Date: Sun, 9 Nov 2025 23:27:50 +0100 Subject: [PATCH 2/2] http/h2_stream: fix trailers validation problem: h2_stream validate_header reject valid response trailers, while accepting invalid ones. cause: validate_header checks that all headers contains the ':status' pseudo-header, which is disallowed by the RFC. fix: add an 'expect_trailers' parameter to validate_header. This value is set to true after a final (i.e. non 1XX) header has validated. When this flag is set, pseudo-headers are disallowed, and ended_stream must be set on the next header validation. Signed-off-by: phantomas --- http/h2_stream.lua | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/http/h2_stream.lua b/http/h2_stream.lua index fc751294..ad74a5d0 100644 --- a/http/h2_stream.lua +++ b/http/h2_stream.lua @@ -99,6 +99,9 @@ local function new_stream(connection) recv_headers_fifo = new_fifo(); recv_headers_cond = cc.new(); + recv_final_header = false; + sent_final_header = false; + chunk_fifo = new_fifo(); chunk_cond = cc.new(); @@ -371,7 +374,7 @@ local valid_pseudo_headers = { [":authority"] = true; [":status"] = false; } -local function validate_headers(headers, is_request, nth_header, ended_stream) +local function validate_headers(headers, is_request, nth_header, ended_stream, expect_trailer) -- Section 8.1.2: A request or response containing uppercase header field names MUST be treated as malformed for name in headers:each() do if name:lower() ~= name then @@ -389,7 +392,7 @@ local function validate_headers(headers, is_request, nth_header, ended_stream) Pseudo-header fields MUST NOT appear in trailers. Endpoints MUST treat a request or response that contains undefined or invalid pseudo-header fields as malformed]] - if (is_request and nth_header ~= 1) or valid_pseudo_headers[name] ~= is_request then + if (is_request and nth_header ~= 1) or valid_pseudo_headers[name] ~= is_request or expect_trailer then return nil, h2_errors.PROTOCOL_ERROR:new_traceback("Pseudo-header fields are only valid in the context in which they are defined", true), ce.EILSEQ end if seen_non_colon then @@ -440,7 +443,7 @@ local function validate_headers(headers, is_request, nth_header, ended_stream) elseif nth_header > 2 then return nil, h2_errors.PROTOCOL_ERROR:new_traceback("An HTTP request consists of maximum 2 HEADER blocks", true), ce.EILSEQ end - else + elseif not expect_trailer then --[[ For HTTP/2 responses, a single :status pseudo-header field is defined that carries the HTTP status code field (RFC7231, Section 6). This pseudo-header field MUST be included in all responses; otherwise, @@ -448,6 +451,10 @@ local function validate_headers(headers, is_request, nth_header, ended_stream) if not headers:has(":status") then return nil, h2_errors.PROTOCOL_ERROR:new_traceback(":status pseudo-header field MUST be included in all responses", true), ce.EILSEQ end + else + if not ended_stream then + return nil, h2_errors.PROTOCOL_ERROR:new_traceback("Trailers MUST be at end of stream", true), ce.EILSEQ + end end return true end @@ -474,13 +481,17 @@ local function process_end_headers(stream, end_stream, pad_len, pos, promised_st if not promised_stream then stream.stats_recv_headers = stream.stats_recv_headers + 1 - local validate_ok, validate_err, errno2 = validate_headers(headers, stream.type ~= "client", stream.stats_recv_headers, end_stream) + local validate_ok, validate_err, errno2 = validate_headers(headers, stream.type ~= "client", stream.stats_recv_headers, end_stream, stream.recv_final_header) if not validate_ok then return nil, validate_err, errno2 end if headers:has("content-length") then stream.content_length = tonumber(headers:get("content-length"), 10) end + if stream.type == "client" and not stream.recv_final_header then + local status = headers:get(":status") + stream.recv_final_header = string.match(status, "1%d%d") == nil + end stream.recv_headers_fifo:push(headers) if end_stream then @@ -498,7 +509,7 @@ local function process_end_headers(stream, end_stream, pad_len, pos, promised_st end end else - local validate_ok, validate_err, errno2 = validate_headers(headers, true, 1, false) + local validate_ok, validate_err, errno2 = validate_headers(headers, true, 1, false, false) if not validate_ok then return nil, validate_err, errno2 end @@ -1321,9 +1332,14 @@ end function stream_methods:write_headers(headers, end_stream, timeout) assert(headers, "missing argument: headers") - assert(validate_headers(headers, self.type == "client", self.stats_sent_headers+1, end_stream)) + assert(validate_headers(headers, self.type == "client", self.stats_sent_headers+1, end_stream, self.sent_final_header)) assert(type(end_stream) == "boolean", "'end_stream' MUST be a boolean") + if self.type ~= "client" and not self.sent_final_header then + local status = headers:get(":status") + self.sent_final_header = string.match(status, "1%d%d") == nil + end + local padded, exclusive, stream_dep, weight = nil, nil, nil, nil return write_headers(self, function(payload, end_headers, deadline) return self:write_headers_frame(payload, end_stream, end_headers, padded, exclusive, stream_dep, weight, deadline and deadline-monotime()) @@ -1333,7 +1349,7 @@ end function stream_methods:push_promise(headers, timeout) assert(self.type == "server") assert(headers, "missing argument: headers") - assert(validate_headers(headers, true, 1, false)) + assert(validate_headers(headers, true, 1, false, false)) assert(headers:has(":authority"), "PUSH_PROMISE must have an :authority") local promised_stream = self.connection:new_stream()