From 28852a61df3350e022c87ad2c9c04f1f095e2120 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 12:36:00 +1300 Subject: [PATCH 1/7] Validate trailers when assigning headers with `Headers#[]=`. --- context/headers.md | 9 ++--- guides/headers/readme.md | 9 ++--- lib/protocol/http/headers.rb | 29 ++++++++++++---- releases.md | 4 +++ test/protocol/http/headers.rb | 65 +++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 14 deletions(-) diff --git a/context/headers.md b/context/headers.md index bd947c1..b562a59 100644 --- a/context/headers.md +++ b/context/headers.md @@ -74,12 +74,13 @@ headers.trailer! headers.add("etag", '"12345"') headers.add("date", Time.now.httpdate) -# These will be silently ignored for security -headers.add("authorization", "Bearer token") # Ignored - credential leakage risk -headers.add("connection", "close") # Ignored - hop-by-hop header +# These will raise InvalidTrailerError for security +headers.add("authorization", "Bearer token") # Credential leakage risk +headers.add("connection", "close") # Hop-by-hop header +headers["content-length"] = "0" # Framing header ``` -The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers: +The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: **Allowed headers** (return `true` for `trailer?`): - `date` - Response generation timestamps. diff --git a/guides/headers/readme.md b/guides/headers/readme.md index bd947c1..b562a59 100644 --- a/guides/headers/readme.md +++ b/guides/headers/readme.md @@ -74,12 +74,13 @@ headers.trailer! headers.add("etag", '"12345"') headers.add("date", Time.now.httpdate) -# These will be silently ignored for security -headers.add("authorization", "Bearer token") # Ignored - credential leakage risk -headers.add("connection", "close") # Ignored - hop-by-hop header +# These will raise InvalidTrailerError for security +headers.add("authorization", "Bearer token") # Credential leakage risk +headers.add("connection", "close") # Hop-by-hop header +headers["content-length"] = "0" # Framing header ``` -The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers: +The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: **Allowed headers** (return `true` for `trailer?`): - `date` - Response generation timestamps. diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index ddccf02..184732c 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -251,15 +251,12 @@ def extract(keys) # @parameter key [String] the header key. # @parameter value [String] the header value to assign. # @parameter trailer [Boolean] whether this header is being added as a trailer. + # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. def add(key, value, trailer: self.trailer?) value = value.to_s if trailer - policy = @policy[key.downcase] - - if !policy or !policy.trailer? - raise InvalidTrailerError, key - end + validate_trailer!(key, @policy[key.downcase]) end if @indexed @@ -284,13 +281,20 @@ def set(key, value) # # @parameter key [String] the header key. # @parameter value [String | Array] the header value to assign. + # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. def []=(key, value) key = key.downcase + policy = @policy[key] + + # Validate before deleting, so that a rejected assignment leaves the headers unchanged: + if self.trailer? + validate_trailer!(key, policy) + end # Delete existing value if any: self.delete(key) - if policy = @policy[key] + if policy unless value.is_a?(policy) value = policy.coerce(value) end @@ -448,6 +452,19 @@ def delete(key) end end + # Ensure the given header is permitted in the trailer section. + # + # Headers which affect framing, routing, connection handling or authorization must not appear in trailers, as they could otherwise be used for request smuggling or to bypass processing that occurred while the header section was being handled. + # + # @parameter key [String] The header key. + # @parameter policy [Class | Boolean] The policy for the header key, or `false` if the header is a simple singleton string. + # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. + protected def validate_trailer!(key, policy) + if !policy or !policy.trailer? + raise InvalidTrailerError, key + end + end + # Merge the value into the hash according to the policy for the given key. # # @parameter hash [Hash] The hash to merge into. diff --git a/releases.md b/releases.md index 9480d0a..26069d0 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - `Protocol::HTTP::Headers#[]=` now validates trailers in the same way as `#add`, raising `InvalidTrailerError` when a forbidden header (e.g. `content-length`, `transfer-encoding`, `authorization`) is assigned after `#trailer!` has been called. + ## v0.72.0 - Clarified body stream lifecycle and ownership, including the directional semantics of `Protocol::HTTP::Body::Stream#close_read`, `#close_write`, and `#close`, how premature input closure affects the associated HTTP exchange, and ownership of streams passed to `Streamable#call`. diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index 9b0c504..8d79d91 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -274,6 +274,71 @@ headers["Content-Length"] = "1" expect(headers["content-length"]).to be == "1" end + + with "trailer" do + it "can assign a permitted trailer" do + trailer = headers.trailer! + + headers["ETag"] = "abcd" + + expect(trailer.to_a).to be == [["etag", "abcd"]] + expect(headers["etag"]).to be == "abcd" + end + + %w[content-length transfer-encoding authorization].each do |key| + with "forbidden trailer #{key.inspect}", unique: key do + it "can't assign a #{key.inspect} header in the trailer" do + headers.trailer! + + expect do + headers[key] = "example" + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + + expect(headers.trailer.to_a).to be(:empty?) + expect(headers).not.to be(:include?, key) + end + end + end + + it "does not delete an existing header when the trailer is rejected" do + headers["content-length"] = "1" + headers.trailer! + + expect do + headers["content-length"] = "999" + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + + expect(headers["content-length"]).to be == "1" + expect(headers.trailer.to_a).to be(:empty?) + end + + it "can assign a forbidden trailer before the trailer section starts" do + headers["content-length"] = "1" + + expect(headers).not.to be(:trailer?) + expect(headers["content-length"]).to be == "1" + end + end + end + + with "#set" do + it "can't set a forbidden header in the trailer" do + headers.trailer! + + expect do + headers.set("content-length", "999") + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + end + end + + with "#merge!" do + it "can't merge a forbidden header into the trailer" do + headers.trailer! + + expect do + headers.merge!("content-length" => "999") + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + end end with "#add" do From 97cc0bbb61ca063e5fd1d6d0d1a7e54ddf387fc4 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:11:51 +1300 Subject: [PATCH 2/7] Validate trailers in `Headers#set` before deleting the existing header. --- lib/protocol/http/headers.rb | 6 ++++++ releases.md | 1 + test/protocol/http/headers.rb | 12 ++++++++++++ 3 files changed, 19 insertions(+) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index 184732c..506287b 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -270,7 +270,13 @@ def add(key, value, trailer: self.trailer?) # # @parameter key [String] the header key to replace. # @parameter value [String] the header value to assign. + # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. def set(key, value) + # Validate before deleting, so that a rejected assignment leaves the headers unchanged: + if self.trailer? + validate_trailer!(key, @policy[key.downcase]) + end + self.delete(key) self.add(key, value) end diff --git a/releases.md b/releases.md index 26069d0..6689406 100644 --- a/releases.md +++ b/releases.md @@ -3,6 +3,7 @@ ## Unreleased - `Protocol::HTTP::Headers#[]=` now validates trailers in the same way as `#add`, raising `InvalidTrailerError` when a forbidden header (e.g. `content-length`, `transfer-encoding`, `authorization`) is assigned after `#trailer!` has been called. + - `Protocol::HTTP::Headers#set` now validates the trailer before deleting the existing header, so a rejected assignment no longer removes the existing value. ## v0.72.0 diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index 8d79d91..8fafeb6 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -329,6 +329,18 @@ headers.set("content-length", "999") end.to raise_exception(Protocol::HTTP::InvalidTrailerError) end + + it "does not delete an existing header when the trailer is rejected" do + headers.set("content-length", "1") + headers.trailer! + + expect do + headers.set("content-length", "999") + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + + expect(headers["content-length"]).to be == "1" + expect(headers.trailer.to_a).to be(:empty?) + end end with "#merge!" do From b8bcc7d914a889dfb19e499af361fd6e4f30729d Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:12:29 +1300 Subject: [PATCH 3/7] Assign `InvalidTrailerError#key` so it is no longer always `nil`. --- lib/protocol/http/error.rb | 2 ++ releases.md | 1 + test/protocol/http/headers.rb | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/protocol/http/error.rb b/lib/protocol/http/error.rb index 5e5911f..9307198 100644 --- a/lib/protocol/http/error.rb +++ b/lib/protocol/http/error.rb @@ -68,6 +68,8 @@ class InvalidTrailerError < InvalidHeaderError # @parameter key [String] The trailer key that is invalid. def initialize(key) super("Invalid trailer key: #{key.inspect}") + + @key = key end # @attribute [String] key The trailer key that is invalid. diff --git a/releases.md b/releases.md index 6689406..6869d30 100644 --- a/releases.md +++ b/releases.md @@ -4,6 +4,7 @@ - `Protocol::HTTP::Headers#[]=` now validates trailers in the same way as `#add`, raising `InvalidTrailerError` when a forbidden header (e.g. `content-length`, `transfer-encoding`, `authorization`) is assigned after `#trailer!` has been called. - `Protocol::HTTP::Headers#set` now validates the trailer before deleting the existing header, so a rejected assignment no longer removes the existing value. + - `Protocol::HTTP::InvalidTrailerError#key` now returns the rejected header key instead of `nil`. ## v0.72.0 diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index 8fafeb6..c68c48f 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -292,7 +292,7 @@ expect do headers[key] = "example" - end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + end.to raise_exception(Protocol::HTTP::InvalidTrailerError).and(have_attributes(key: be == key)) expect(headers.trailer.to_a).to be(:empty?) expect(headers).not.to be(:include?, key) From 315d4d4025df564de74046e3ab9bc45b0ef7ea15 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:12:57 +1300 Subject: [PATCH 4/7] Move trailer validation tests for `#set` and `#merge!` next to the existing blocks. --- test/protocol/http/headers.rb | 56 +++++++++++++++++------------------ 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index c68c48f..2bf2cb4 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -321,7 +321,24 @@ end end + with "#add" do + it "can add field" do + headers.add("Content-Length", 1) + + expect(headers.fields.last).to be == ["Content-Length", "1"] + expect(headers["content-length"]).to be == "1" + end + end + with "#set" do + it "can replace an existing field" do + headers.add("accept-encoding", "gzip,deflate") + + headers.set("accept-encoding", "gzip") + + expect(headers["accept-encoding"]).to be == ["gzip"] + end + it "can't set a forbidden header in the trailer" do headers.trailer! @@ -343,35 +360,6 @@ end end - with "#merge!" do - it "can't merge a forbidden header into the trailer" do - headers.trailer! - - expect do - headers.merge!("content-length" => "999") - end.to raise_exception(Protocol::HTTP::InvalidTrailerError) - end - end - - with "#add" do - it "can add field" do - headers.add("Content-Length", 1) - - expect(headers.fields.last).to be == ["Content-Length", "1"] - expect(headers["content-length"]).to be == "1" - end - end - - with "#set" do - it "can replace an existing field" do - headers.add("accept-encoding", "gzip,deflate") - - headers.set("accept-encoding", "gzip") - - expect(headers["accept-encoding"]).to be == ["gzip"] - end - end - with "#extract" do it "can extract key's that don't exist" do expect(headers.extract("foo")).to be(:empty?) @@ -416,6 +404,16 @@ end end + with "#merge!" do + it "can't merge a forbidden header into the trailer" do + headers.trailer! + + expect do + headers.merge!("content-length" => "999") + end.to raise_exception(Protocol::HTTP::InvalidTrailerError) + end + end + with "#trailer!" do it "can add trailer" do headers.add("trailer", "etag") From 6ac43327ee3756024bac044515c122de8eeefb41 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:13:18 +1300 Subject: [PATCH 5/7] Report the key as given when `Headers#[]=` rejects a trailer. --- lib/protocol/http/headers.rb | 5 +++-- test/protocol/http/headers.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index 506287b..c6383ea 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -289,14 +289,15 @@ def set(key, value) # @parameter value [String | Array] the header value to assign. # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. def []=(key, value) - key = key.downcase - policy = @policy[key] + policy = @policy[key.downcase] # Validate before deleting, so that a rejected assignment leaves the headers unchanged: if self.trailer? validate_trailer!(key, policy) end + key = key.downcase + # Delete existing value if any: self.delete(key) diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index 2bf2cb4..1c537a1 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -300,6 +300,14 @@ end end + it "reports the key as given in the error" do + headers.trailer! + + expect do + headers["Content-Length"] = "1" + end.to raise_exception(Protocol::HTTP::InvalidTrailerError, message: be == 'Invalid trailer key: "Content-Length"').and(have_attributes(key: be == "Content-Length")) + end + it "does not delete an existing header when the trailer is rejected" do headers["content-length"] = "1" headers.trailer! From 612f40947969b4af0094a9879c05b8dccbcc3811 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:13:28 +1300 Subject: [PATCH 6/7] Document that `validate_trailer!` may receive a `nil` policy. --- lib/protocol/http/headers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index c6383ea..ea6ae47 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -464,7 +464,7 @@ def delete(key) # Headers which affect framing, routing, connection handling or authorization must not appear in trailers, as they could otherwise be used for request smuggling or to bypass processing that occurred while the header section was being handled. # # @parameter key [String] The header key. - # @parameter policy [Class | Boolean] The policy for the header key, or `false` if the header is a simple singleton string. + # @parameter policy [Class | Boolean | Nil] The policy for the header key, `false` if the header is a simple singleton string, or `nil` if the policy has no entry for the key. # @raises [InvalidTrailerError] If the header is not permitted in the trailer section. protected def validate_trailer!(key, policy) if !policy or !policy.trailer? From 7f475487a6f99e9f94fce35fa3fc401eb51070bb Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:13:50 +1300 Subject: [PATCH 7/7] Annotate each rejected trailer in the headers guide example. --- context/headers.md | 10 +++++----- guides/headers/readme.md | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/context/headers.md b/context/headers.md index b562a59..191a0ef 100644 --- a/context/headers.md +++ b/context/headers.md @@ -74,13 +74,13 @@ headers.trailer! headers.add("etag", '"12345"') headers.add("date", Time.now.httpdate) -# These will raise InvalidTrailerError for security -headers.add("authorization", "Bearer token") # Credential leakage risk -headers.add("connection", "close") # Hop-by-hop header -headers["content-length"] = "0" # Framing header +# Each of these will raise InvalidTrailerError for security: +headers.add("authorization", "Bearer token") # => raises InvalidTrailerError (credential leakage risk) +headers.add("connection", "close") # => raises InvalidTrailerError (hop-by-hop header) +headers["content-length"] = "0" # => raises InvalidTrailerError (framing header) ``` -The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: +The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy before modifying the headers, and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: **Allowed headers** (return `true` for `trailer?`): - `date` - Response generation timestamps. diff --git a/guides/headers/readme.md b/guides/headers/readme.md index b562a59..191a0ef 100644 --- a/guides/headers/readme.md +++ b/guides/headers/readme.md @@ -74,13 +74,13 @@ headers.trailer! headers.add("etag", '"12345"') headers.add("date", Time.now.httpdate) -# These will raise InvalidTrailerError for security -headers.add("authorization", "Bearer token") # Credential leakage risk -headers.add("connection", "close") # Hop-by-hop header -headers["content-length"] = "0" # Framing header +# Each of these will raise InvalidTrailerError for security: +headers.add("authorization", "Bearer token") # => raises InvalidTrailerError (credential leakage risk) +headers.add("connection", "close") # => raises InvalidTrailerError (hop-by-hop header) +headers["content-length"] = "0" # => raises InvalidTrailerError (framing header) ``` -The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: +The trailer security system prevents HTTP request smuggling by restricting which headers can appear in trailers. Once `trailer!` has been called, every method which appends fields (`add`, `set`, `[]=` and `merge!`) validates the header against the policy before modifying the headers, and raises {ruby Protocol::HTTP::InvalidTrailerError} if it is not permitted: **Allowed headers** (return `true` for `trailer?`): - `date` - Response generation timestamps.