From 437686b5fd532c827a3b38f1e22848b53eb488f0 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 12:35:55 +1300 Subject: [PATCH 1/5] Preserve the trailer boundary when deleting or extracting header fields. --- lib/protocol/http/headers.rb | 33 ++++++++++- releases.md | 4 ++ test/protocol/http/headers.rb | 107 ++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index ddccf02..ccae9f0 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -233,7 +233,7 @@ def keys # # @parameter keys [Array] The keys to extract. def extract(keys) - deleted, @fields = @fields.partition do |field| + deleted = remove_fields do |field| keys.include?(field.first.downcase) end @@ -425,7 +425,7 @@ def delete(key) return nil end - deleted, @fields = @fields.partition do |field| + deleted = remove_fields do |field| field.first.downcase == key end @@ -448,6 +448,35 @@ def delete(key) end end + # Remove all fields for which the block returns true, preserving the trailer boundary. + # + # The trailer boundary `@tail` is an index into `@fields`, so removing a field from the header section (before `@tail`) must shift the boundary down by one, otherwise the first trailer field would be treated as a header. Removing a field from the trailer section leaves the boundary unchanged. + # + # @yields {|field| ...} Each `[key, value]` pair; return true to remove it. + # @returns [Array] The removed fields, in their original order. + private def remove_fields + deleted = [] + kept = [] + tail = @tail + + @fields.each_with_index do |field, index| + if yield(field) + deleted << field + + if @tail and index < @tail + tail -= 1 + end + else + kept << field + end + end + + @fields = kept + @tail = tail + + return deleted + 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..42d8c81 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Preserve the trailer boundary when `Protocol::HTTP::Headers#delete` or `#extract` removes fields from the header section, so existing trailers are no longer sent as headers. + ## 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..b545bf5 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -512,6 +512,113 @@ end end + with "trailer boundary" do + let(:header_fields) {fields[1..-1]} + + before do + headers.trailer! + headers.add("etag", "abcd") + headers.add("digest", "sha-256=xyz") + end + + it "preserves trailers when deleting a header" do + expect(headers.delete("content-type")).to be == "text/html" + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 4 + expect(headers.header.to_a).to be == header_fields + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + end + + it "preserves trailers when deleting multiple fields of a header" do + headers.delete("set-cookie") + + expect(headers.tail).to be == 3 + expect(headers.header.to_a).to be == [ + ["Content-Type", "text/html"], + ["connection", "Keep-Alive"], + ["Accept", "*/*"], + ] + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + end + + it "preserves trailers when deleting a header with an index" do + # Force the headers to be indexed: + headers.to_h + + headers.delete("content-type") + + expect(headers.tail).to be == 4 + expect(headers.header.to_a).to be == header_fields + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + expect(headers["etag"]).to be == "abcd" + end + + it "preserves trailers when extracting headers" do + expect(headers.extract(["content-type", "set-cookie"])).to be == [ + ["Content-Type", "text/html"], + ["Set-Cookie", "hello=world"], + ["set-cookie", "foo=bar"], + ] + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 2 + expect(headers.header.to_a).to be == [ + ["connection", "Keep-Alive"], + ["Accept", "*/*"], + ] + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + end + + it "preserves trailers when extracting all headers" do + headers.extract(["content-type", "connection", "set-cookie", "accept"]) + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 0 + expect(headers.header.to_a).to be == [] + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + end + + it "does not move the boundary when deleting a trailer" do + expect(headers.delete("etag")).to be == "abcd" + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 5 + expect(headers.header.to_a).to be == fields + expect(headers.trailer.to_a).to be == [["digest", "sha-256=xyz"]] + end + + it "does not move the boundary when extracting trailers" do + expect(headers.extract(["etag", "digest"])).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 5 + expect(headers.header.to_a).to be == fields + expect(headers.trailer.to_a).to be == [] + end + + it "preserves trailers when replacing a header" do + headers["content-type"] = "text/plain" + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 4 + expect(headers.header.to_a).to be == header_fields + expect(headers.trailer.to_a).to be == [ + ["etag", "abcd"], + ["digest", "sha-256=xyz"], + ["content-type", "text/plain"], + ] + end + + it "preserves the boundary when deleting a missing header" do + expect(headers.delete("transfer-encoding")).to be_nil + + expect(headers.tail).to be == 5 + expect(headers.header.to_a).to be == fields + expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] + end + end + with "#add with trailer: keyword" do let(:headers) {subject.new} From 20e095944aff0f88ae9ea16dc02d6d5f93f4c708 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:13:09 +1300 Subject: [PATCH 2/5] Keep the partition fast path in remove_fields when there is no trailer boundary. --- lib/protocol/http/headers.rb | 37 ++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index ccae9f0..31f2425 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -452,28 +452,33 @@ def delete(key) # # The trailer boundary `@tail` is an index into `@fields`, so removing a field from the header section (before `@tail`) must shift the boundary down by one, otherwise the first trailer field would be treated as a header. Removing a field from the trailer section leaves the boundary unchanged. # + # Most headers never have a trailer boundary, so that case is kept on the fast `partition` path and the indexed loop only runs when a boundary exists. + # # @yields {|field| ...} Each `[key, value]` pair; return true to remove it. # @returns [Array] The removed fields, in their original order. - private def remove_fields - deleted = [] - kept = [] - tail = @tail - - @fields.each_with_index do |field, index| - if yield(field) - deleted << field - - if @tail and index < @tail - tail -= 1 + private def remove_fields(&block) + if tail = @tail + deleted = [] + kept = [] + + @fields.each_with_index do |field, index| + if yield(field) + deleted << field + + if index < @tail + tail -= 1 + end + else + kept << field end - else - kept << field end + + @fields = kept + @tail = tail + else + deleted, @fields = @fields.partition(&block) end - @fields = kept - @tail = tail - return deleted end From 512a1c06f13ff15656599892b08b0026ece5aade Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:16:14 +1300 Subject: [PATCH 3/5] Normalize the key to lowercase in `Headers#delete` so mixed case keys remove the matching fields. --- lib/protocol/http/headers.rb | 6 ++++-- releases.md | 1 + test/protocol/http/headers.rb | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/protocol/http/headers.rb b/lib/protocol/http/headers.rb index 31f2425..3702c4b 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -231,7 +231,7 @@ def keys # Extract the specified keys from the headers. # - # @parameter keys [Array] The keys to extract. + # @parameter keys [Array] The keys to extract, which must be lowercase. def extract(keys) deleted = remove_fields do |field| keys.include?(field.first.downcase) @@ -420,8 +420,10 @@ def merge(headers) # @parameter key [String] The header key. # @returns [String | Array | Object] The merged header value. def delete(key) + key = key.downcase + # If we've indexed the headers, we can bail out early if the key is not present: - if @indexed && !@indexed.key?(key.downcase) + if @indexed && !@indexed.key?(key) return nil end diff --git a/releases.md b/releases.md index 42d8c81..26fa117 100644 --- a/releases.md +++ b/releases.md @@ -3,6 +3,7 @@ ## Unreleased - Preserve the trailer boundary when `Protocol::HTTP::Headers#delete` or `#extract` removes fields from the header section, so existing trailers are no longer sent as headers. + - `Protocol::HTTP::Headers#delete` (and therefore `#set`) now normalizes the key to lowercase, so mixed case keys remove the matching fields instead of leaving them in place. ## v0.72.0 diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index b545bf5..63afac6 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -293,6 +293,13 @@ expect(headers["accept-encoding"]).to be == ["gzip"] end + + it "can replace an existing field using a mixed case key" do + headers.set("Content-Type", "text/plain") + + expect(headers.fields.count{|key, value| key.downcase == "content-type"}).to be == 1 + expect(headers["content-type"]).to be == "text/plain" + end end with "#extract" do @@ -329,6 +336,22 @@ it "can delete non-existant fields" do expect(headers.delete("transfer-encoding")).to be_nil end + + it "can delete fields using a mixed case key" do + expect(headers.delete("Content-Type")).to be == "text/html" + + expect(headers.fields).to be == fields[1..-1] + end + + it "can delete indexed fields using a mixed case key" do + # Force the headers to be indexed: + headers.to_h + + expect(headers.delete("Content-Type")).to be == "text/html" + + expect(headers.fields).to be == fields[1..-1] + expect(headers["content-type"]).to be_nil + end end with "#merge" do From b8e143828cf37cb37d7868b318e22fa7a1654418 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:17:02 +1300 Subject: [PATCH 4/5] Test trailer boundary preservation with a trailer-permitted replacement, a key straddling the boundary, and no trailers. --- test/protocol/http/headers.rb | 79 ++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/test/protocol/http/headers.rb b/test/protocol/http/headers.rb index 63afac6..98e24b3 100644 --- a/test/protocol/http/headers.rb +++ b/test/protocol/http/headers.rb @@ -553,7 +553,7 @@ expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] end - it "preserves trailers when deleting multiple fields of a header" do + it "preserves trailers when deleting multiple fields with the same key" do headers.delete("set-cookie") expect(headers.tail).to be == 3 @@ -620,19 +620,6 @@ expect(headers.trailer.to_a).to be == [] end - it "preserves trailers when replacing a header" do - headers["content-type"] = "text/plain" - - expect(headers).to be(:trailer?) - expect(headers.tail).to be == 4 - expect(headers.header.to_a).to be == header_fields - expect(headers.trailer.to_a).to be == [ - ["etag", "abcd"], - ["digest", "sha-256=xyz"], - ["content-type", "text/plain"], - ] - end - it "preserves the boundary when deleting a missing header" do expect(headers.delete("transfer-encoding")).to be_nil @@ -640,6 +627,70 @@ expect(headers.header.to_a).to be == fields expect(headers.trailer.to_a).to be == [["etag", "abcd"], ["digest", "sha-256=xyz"]] end + + with "a header that is permitted as a trailer" do + let(:fields) do + [ + ["Content-Type", "text/html"], + ["Server-Timing", "db;dur=1"], + ["Accept", "*/*"], + ] + end + + it "preserves trailers when replacing the header" do + headers["server-timing"] = "db;dur=2" + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == 2 + expect(headers.header.to_a).to be == [ + ["Content-Type", "text/html"], + ["Accept", "*/*"], + ] + expect(headers.trailer.to_a).to be == [ + ["etag", "abcd"], + ["digest", "sha-256=xyz"], + ["server-timing", "db;dur=2"], + ] + end + end + + with "a key that appears in both the header and trailer sections" do + let(:fields) do + [ + ["Content-Type", "text/html"], + ["ETag", "wxyz"], + ["Accept", "*/*"], + ] + end + + it "only moves the boundary for the field in the header section" do + expect(headers.delete("etag")).to be == "abcd" + + expect(headers.tail).to be == 2 + expect(headers.header.to_a).to be == [ + ["Content-Type", "text/html"], + ["Accept", "*/*"], + ] + expect(headers.trailer.to_a).to be == [["digest", "sha-256=xyz"]] + end + end + end + + with "trailer boundary and no trailers" do + before do + headers.trailer! + end + + it "keeps the boundary at the end of the fields when deleting a header" do + expect(headers.tail).to be == fields.size + + headers.delete("content-type") + + expect(headers).to be(:trailer?) + expect(headers.tail).to be == headers.fields.size + expect(headers.header.to_a).to be == fields[1..-1] + expect(headers.trailer.to_a).to be == [] + end end with "#add with trailer: keyword" do From eaa3cc6213f522535babb2f9d362087b9c992dbe Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Thu, 5 Nov 2026 23:02:11 +1300 Subject: [PATCH 5/5] Shorten the documentation. --- 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 3702c4b..4175fbe 100644 --- a/lib/protocol/http/headers.rb +++ b/lib/protocol/http/headers.rb @@ -452,7 +452,7 @@ def delete(key) # Remove all fields for which the block returns true, preserving the trailer boundary. # - # The trailer boundary `@tail` is an index into `@fields`, so removing a field from the header section (before `@tail`) must shift the boundary down by one, otherwise the first trailer field would be treated as a header. Removing a field from the trailer section leaves the boundary unchanged. + # Removing a field before the trailer boundary `@tail` must shift the boundary down, otherwise the first trailer would be treated as a header. # # Most headers never have a trailer boundary, so that case is kept on the fast `partition` path and the indexed loop only runs when a boundary exists. #