Skip to content

Commit 6d333e1

Browse files
committed
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.
1 parent 13a5119 commit 6d333e1

File tree

1 file changed

+23
-7
lines changed

1 file changed

+23
-7
lines changed

http/h2_stream.lua

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ local function new_stream(connection)
9999
recv_headers_fifo = new_fifo();
100100
recv_headers_cond = cc.new();
101101

102+
recv_final_header = false;
103+
sent_final_header = false;
104+
102105
chunk_fifo = new_fifo();
103106
chunk_cond = cc.new();
104107

@@ -371,7 +374,7 @@ local valid_pseudo_headers = {
371374
[":authority"] = true;
372375
[":status"] = false;
373376
}
374-
local function validate_headers(headers, is_request, nth_header, ended_stream)
377+
local function validate_headers(headers, is_request, nth_header, ended_stream, expect_trailer)
375378
-- Section 8.1.2: A request or response containing uppercase header field names MUST be treated as malformed
376379
for name in headers:each() do
377380
if name:lower() ~= name then
@@ -389,7 +392,7 @@ local function validate_headers(headers, is_request, nth_header, ended_stream)
389392
Pseudo-header fields MUST NOT appear in trailers.
390393
Endpoints MUST treat a request or response that contains
391394
undefined or invalid pseudo-header fields as malformed]]
392-
if (is_request and nth_header ~= 1) or valid_pseudo_headers[name] ~= is_request then
395+
if (is_request and nth_header ~= 1) or valid_pseudo_headers[name] ~= is_request or expect_trailer then
393396
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
394397
end
395398
if seen_non_colon then
@@ -440,14 +443,18 @@ local function validate_headers(headers, is_request, nth_header, ended_stream)
440443
elseif nth_header > 2 then
441444
return nil, h2_errors.PROTOCOL_ERROR:new_traceback("An HTTP request consists of maximum 2 HEADER blocks", true), ce.EILSEQ
442445
end
443-
else
446+
elseif not expect_trailer then
444447
--[[ For HTTP/2 responses, a single :status pseudo-header field is
445448
defined that carries the HTTP status code field (RFC7231, Section 6).
446449
This pseudo-header field MUST be included in all responses; otherwise,
447450
the response is malformed (Section 8.1.2.6)]]
448451
if not headers:has(":status") then
449452
return nil, h2_errors.PROTOCOL_ERROR:new_traceback(":status pseudo-header field MUST be included in all responses", true), ce.EILSEQ
450453
end
454+
else
455+
if not ended_stream then
456+
return nil, h2_errors.PROTOCOL_ERROR:new_traceback("Trailers MUST be at end of stream", true), ce.EILSEQ
457+
end
451458
end
452459
return true
453460
end
@@ -474,13 +481,17 @@ local function process_end_headers(stream, end_stream, pad_len, pos, promised_st
474481

475482
if not promised_stream then
476483
stream.stats_recv_headers = stream.stats_recv_headers + 1
477-
local validate_ok, validate_err, errno2 = validate_headers(headers, stream.type ~= "client", stream.stats_recv_headers, end_stream)
484+
local validate_ok, validate_err, errno2 = validate_headers(headers, stream.type ~= "client", stream.stats_recv_headers, end_stream, stream.recv_final_header)
478485
if not validate_ok then
479486
return nil, validate_err, errno2
480487
end
481488
if headers:has("content-length") then
482489
stream.content_length = tonumber(headers:get("content-length"), 10)
483490
end
491+
if stream.type == "client" and not stream.recv_final_header then
492+
local status = headers:get(":status")
493+
stream.recv_final_header = string.match(status, "1%d%d") == nil
494+
end
484495
stream.recv_headers_fifo:push(headers)
485496

486497
if end_stream then
@@ -498,7 +509,7 @@ local function process_end_headers(stream, end_stream, pad_len, pos, promised_st
498509
end
499510
end
500511
else
501-
local validate_ok, validate_err, errno2 = validate_headers(headers, true, 1, false)
512+
local validate_ok, validate_err, errno2 = validate_headers(headers, true, 1, false, false)
502513
if not validate_ok then
503514
return nil, validate_err, errno2
504515
end
@@ -1321,9 +1332,14 @@ end
13211332

13221333
function stream_methods:write_headers(headers, end_stream, timeout)
13231334
assert(headers, "missing argument: headers")
1324-
assert(validate_headers(headers, self.type == "client", self.stats_sent_headers+1, end_stream))
1335+
assert(validate_headers(headers, self.type == "client", self.stats_sent_headers+1, end_stream, self.sent_final_header))
13251336
assert(type(end_stream) == "boolean", "'end_stream' MUST be a boolean")
13261337

1338+
if self.type ~= "client" and not self.sent_final_header then
1339+
local status = headers:get(":status")
1340+
self.sent_final_header = string.match(status, "1%d%d") == nil
1341+
end
1342+
13271343
local padded, exclusive, stream_dep, weight = nil, nil, nil, nil
13281344
return write_headers(self, function(payload, end_headers, deadline)
13291345
return self:write_headers_frame(payload, end_stream, end_headers, padded, exclusive, stream_dep, weight, deadline and deadline-monotime())
@@ -1333,7 +1349,7 @@ end
13331349
function stream_methods:push_promise(headers, timeout)
13341350
assert(self.type == "server")
13351351
assert(headers, "missing argument: headers")
1336-
assert(validate_headers(headers, true, 1, false))
1352+
assert(validate_headers(headers, true, 1, false, false))
13371353
assert(headers:has(":authority"), "PUSH_PROMISE must have an :authority")
13381354

13391355
local promised_stream = self.connection:new_stream()

0 commit comments

Comments
 (0)