From fa18ed9e3e620f80b86d06c32dd694a82edc60d0 Mon Sep 17 00:00:00 2001 From: Alexander Mankuta Date: Tue, 29 Sep 2026 15:48:05 +0300 Subject: [PATCH] Fix HPACK dynamic table size handling SETTINGS_HEADER_TABLE_SIZE sets the maximum size of the table used by the encoder on the other side. Our acknowledged setting limits the peer's encoder, which is our decompressor. The peer's setting limits our encoder, which is our compressor. 3b51a2e swapped the two. That commit fixed igrigorik/http-2#97, where Chrome sent SETTINGS_HEADER_TABLE_SIZE=65536 and then failed to decode our headers with COMPRESSION_ERROR. The swap removed that error, but the assignment had been correct before it. The actual cause was that the compressor grew its table to 65536 without sending a dynamic table size update. Chrome's decoder kept its table at 4096, so our encoder referred to entries the decoder had already evicted. With the swap, the compressor stayed at 4096 and the error went away. But a peer that advertised a smaller table (for example 0) could no longer decode our headers, and our decoder evicted entries the peer still referred to. Restore the correct assignment. Change table sizes the way RFC 7541 section 4.2 requires: - SETTINGS_HEADER_TABLE_SIZE only sets the maximum table size. The table changes with a dynamic table size update at the beginning of a header block. - The compressor uses its own table size, capped at the peer's maximum. When the size has to change, the next header block starts with a size update. If the maximum was reduced and then increased between header blocks, it sends the smallest maximum first and then the final size. - The decompressor rejects size updates above the maximum from our acknowledged setting. Before, it compared against the initial settings. When the maximum is reduced below the current table size, the next header block has to start with a size update to the smallest maximum or lower. --- lib/http/2/connection.rb | 9 ++- lib/http/2/header/compressor.rb | 6 +- lib/http/2/header/decompressor.rb | 6 +- lib/http/2/header/encoding_context.rb | 61 +++++++++++++++-- sig/header/encoding_context.rbs | 6 ++ spec/client_spec.rb | 82 +++++++++++++++++++++++ spec/compressor_spec.rb | 95 +++++++++++++++++++++++++++ 7 files changed, 253 insertions(+), 12 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 420b98f6..23870861 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -667,12 +667,15 @@ def connection_settings(frame) end when :settings_header_table_size - # Setting header table size might cause some headers evicted + # The value is the maximum table size for the encoder of the other + # side: our setting limits the peer's encoder (our decompressor), + # and the peer's setting limits our encoder (our compressor). + # The table itself changes with the next dynamic table size update. case side when :local - @compressor.table_size = v - when :remote @decompressor.table_size = v + when :remote + @compressor.table_size = v end when :settings_enable_push diff --git a/lib/http/2/header/compressor.rb b/lib/http/2/header/compressor.rb index 566a219b..47a61889 100644 --- a/lib/http/2/header/compressor.rb +++ b/lib/http/2/header/compressor.rb @@ -11,9 +11,11 @@ def initialize(settings = Settings.new) @cc = EncodingContext.new(settings) end - # Set dynamic table +size+ in EncodingContext + # Set the maximum dynamic table +size+ allowed by the peer's + # SETTINGS_HEADER_TABLE_SIZE. The next header block starts with a + # dynamic table size update when the table size has to change. def table_size=(size) - @cc.table_size = size + @cc.max_table_size = size end # Encodes +i+ via integer representation into +buffer+ at the offset set by +offset+. diff --git a/lib/http/2/header/decompressor.rb b/lib/http/2/header/decompressor.rb index b58ce41f..71abeefb 100644 --- a/lib/http/2/header/decompressor.rb +++ b/lib/http/2/header/decompressor.rb @@ -15,9 +15,11 @@ def initialize(settings = Settings.new) @cc = EncodingContext.new(settings) end - # Set dynamic table +size+ in EncodingContext + # Set the maximum dynamic table +size+ allowed by our acknowledged + # SETTINGS_HEADER_TABLE_SIZE. When the current table is larger, the + # next header block has to start with a dynamic table size update. def table_size=(size) - @cc.table_size = size + @cc.max_table_size = size end # Decodes integer value from provided +buffer+. diff --git a/lib/http/2/header/encoding_context.rb b/lib/http/2/header/encoding_context.rb index 9e13cd08..4bd27717 100644 --- a/lib/http/2/header/encoding_context.rb +++ b/lib/http/2/header/encoding_context.rb @@ -101,12 +101,20 @@ class EncodingContext # Initializes compression context with appropriate client/server # +settings+ and maximum size of the dynamic table. + # + # The dynamic table starts at +settings.table_size+ octets. The same + # value is the initial maximum size allowed for the table. def initialize(settings = Settings.new) @table = [] @table_by_field = Hash.new { |hs, k| hs[k] = [] } @unshifts = 0 @settings = settings + # Current dynamic table size, as known to both encoder and decoder. @limit = settings.table_size + # Maximum dynamic table size allowed by SETTINGS_HEADER_TABLE_SIZE. + @max_limit = @limit + # Smallest maximum size set since the last dynamic table size update. + @lowest_max_limit = @limit @_table_updated = false @current_table_size = 0 end @@ -118,11 +126,15 @@ def dup tbf = @table_by_field.transform_values(&:dup) unshifts = @unshifts l = @limit + ml = @max_limit + lml = @lowest_max_limit other.instance_eval do @table = t.dup # shallow copy @table_by_field = tbf @unshifts = unshifts @limit = l + @max_limit = ml + @lowest_max_limit = lml end other end @@ -152,16 +164,21 @@ def process(cmd) name = cmd[:name] value = cmd[:value] + # The maximum size was reduced below the current table size and the + # encoder has not signalled a size that fits into it. + raise CompressionError, "dynamic table size update required" if type != :changetablesize && @lowest_max_limit < @limit + case type when :changetablesize raise CompressionError, "tried to change table size after adding elements to table" if @_table_updated - # we can receive multiple table size change commands inside a header frame. However, - # we should blow up if we receive another frame where the new table size is bigger. - table_size_updated = @limit != @settings.table_size - - raise CompressionError, "dynamic table size update exceed limit" if !table_size_updated && value > @limit + # The new maximum size MUST be lower than or equal to the limit set + # by SETTINGS_HEADER_TABLE_SIZE. + # - https://www.rfc-editor.org/rfc/rfc7541#section-6.3 + raise CompressionError, "dynamic table size update exceed limit" if value > @max_limit + # The smallest maximum size set since the last update is signalled. + @lowest_max_limit = @max_limit if value <= @lowest_max_limit self.table_size = value nil @@ -210,10 +227,26 @@ def process(cmd) end # Plan +headers+ compression. + # + # Emits dynamic table size updates first when the table size has to + # change. See #max_table_size=. def encode(headers) # Literals commands are marked with :noindex when index is not used noindex = STATIC_NEVER.include?(@settings.index) + if @lowest_max_limit < @limit + self.table_size = @lowest_max_limit + yield({ type: :changetablesize, value: @limit }) + end + @lowest_max_limit = @max_limit + + size = @settings.table_size + size = @max_limit if @max_limit < size + if size != @limit + self.table_size = size + yield({ type: :changetablesize, value: size }) + end + headers.each do |field, value| # Literal header names MUST be translated to lowercase before # encoding and transmission. @@ -276,6 +309,24 @@ def table_size=(size) resize_table(0) end + # Set the maximum dynamic table +size+ allowed by + # SETTINGS_HEADER_TABLE_SIZE. + # + # The current table size does not change here. It changes only with a + # dynamic table size update at the beginning of a header block: + # - an encoder emits the update in the next #encode call. It uses + # +settings.table_size+, capped by this maximum. + # - a decoder requires the update in the next header block when the + # current table size exceeds this maximum. + # + # When the maximum is reduced and then increased again before the next + # header block, the smallest maximum has to be signalled first. + # - https://www.rfc-editor.org/rfc/rfc7541#section-4.2 + def max_table_size=(size) + @max_limit = size + @lowest_max_limit = size if size < @lowest_max_limit + end + def listen_on_table yield ensure diff --git a/sig/header/encoding_context.rbs b/sig/header/encoding_context.rbs index 05359dc5..5fc8c959 100644 --- a/sig/header/encoding_context.rbs +++ b/sig/header/encoding_context.rbs @@ -26,6 +26,10 @@ module HTTP2 @limit: Integer + @max_limit: Integer + + @lowest_max_limit: Integer + @_table_updated: bool def dup: () -> EncodingContext @@ -40,6 +44,8 @@ module HTTP2 def table_size=: (Integer) -> void + def max_table_size=: (Integer) -> void + def listen_on_table: { () -> void } -> void private diff --git a/spec/client_spec.rb b/spec/client_spec.rb index ac24c5e9..e5c1d988 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -90,6 +90,88 @@ end end + context "header table size" do + def sent_headers(conn) + frames = [] + conn.on(:frame) { |bytes| frames << f.parse(bytes) } + yield + frames.select { |frame| frame[:type] == :headers } + end + + def peer_settings(payload) + settings = settings_frame + settings[:payload] = payload + f.generate(settings) + end + + def response_headers(stream, payload) + f.generate(type: :headers, stream: stream.id, flags: END_HEADERS | END_STREAM, payload: payload) + end + + it "should signal remote SETTINGS_HEADER_TABLE_SIZE reduction in the next header block" do + client << peer_settings([[:settings_header_table_size, 0]]) + + headers = sent_headers(client) do + client.new_stream.headers(REQUEST_HEADERS, end_stream: true) + client.new_stream.headers(REQUEST_HEADERS, end_stream: true) + end + + # Dynamic table size update to 0. + expect(headers[0][:payload].getbyte(0)).to eq 0x20 + expect(headers[1][:payload].getbyte(0)).not_to eq 0x20 + + peer = Decompressor.new + peer.table_size = 0 + headers.each { |frame| expect(peer.decode(frame[:payload])).to eq REQUEST_HEADERS } + end + + it "should keep own table size when remote SETTINGS_HEADER_TABLE_SIZE is larger" do + client << peer_settings([[:settings_header_table_size, 65_536], [:settings_max_concurrent_streams, 1000]]) + + # The peer's decoder keeps the default table size until it receives + # a dynamic table size update. + peer = Decompressor.new + + # Unique headers fill the table, a repeated one refers to an old entry. + extra = Array.new(100) { |i| [["x-request-id", format("%03d-%s", i: i, pad: "a" * 40)], %w[x-app shared]] } + headers = sent_headers(client) do + extra.each { |h| client.new_stream.headers(REQUEST_HEADERS + h, end_stream: true) } + end + + headers.zip(extra).each do |frame, h| + expect(peer.decode(frame[:payload]).last(2)).to eq h + end + end + + it "should accept header blocks with a table size update after local SETTINGS_HEADER_TABLE_SIZE reduction" do + client = Client.new(settings_header_table_size: 256) + client << peer_settings([]) + stream = client.new_stream + stream.headers(REQUEST_HEADERS, end_stream: true) + client << f.generate(type: :settings, stream: 0, payload: [], flags: ACK) + expect(client.local_settings[:settings_header_table_size]).to eq 256 + + peer = Compressor.new + peer.table_size = 256 + received = nil + stream.on(:headers) { |h| received = h } + client << response_headers(stream, peer.encode(RESPONSE_HEADERS + [%w[x-app shared]])) + + expect(received).to eq RESPONSE_HEADERS + [%w[x-app shared]] + end + + it "should reject header blocks without a table size update after local SETTINGS_HEADER_TABLE_SIZE reduction" do + client = Client.new(settings_header_table_size: 256) + client << peer_settings([]) + stream = client.new_stream + stream.headers(REQUEST_HEADERS, end_stream: true) + client << f.generate(type: :settings, stream: 0, payload: [], flags: ACK) + + payload = Compressor.new.encode(RESPONSE_HEADERS) + expect { client << response_headers(stream, payload) }.to raise_error(CompressionError) + end + end + context "upgrade" do it "fails when client has already created streams" do client.new_stream diff --git a/spec/compressor_spec.rb b/spec/compressor_spec.rb index 72417b08..dbfe7d1a 100644 --- a/spec/compressor_spec.rb +++ b/spec/compressor_spec.rb @@ -237,6 +237,101 @@ expect { cc.process(type: :changetablesize, value: 150_000_000) }.to raise_error(CompressionError) end + + it "should accept table size update up to increased maximum" do + cc = EncodingContext.new(Settings.new(table_size: 4096)) + cc.max_table_size = 8192 + + expect { cc.process(type: :changetablesize, value: 8192) }.not_to raise_error + expect { cc.process(type: :changetablesize, value: 8193) }.to raise_error(CompressionError) + end + + it "should not change table size when maximum is reduced" do + cc = EncodingContext.new(Settings.new(table_size: 2048)) + cc.process(name: "test1", value: "1" * 1024, type: :incremental) + cc.max_table_size = 0 + + expect(cc.table.size).to eq 1 + end + + it "should require table size update when maximum is reduced below table size" do + cc = EncodingContext.new + cc.max_table_size = 256 + + expect { cc.process(name: 1, type: :indexed) }.to raise_error(CompressionError) + end + + it "should require smallest maximum when maximum is reduced and increased" do + cc = EncodingContext.new + cc.max_table_size = 256 + cc.max_table_size = 4096 + + cc.process(type: :changetablesize, value: 1024) + expect { cc.process(name: 1, type: :indexed) }.to raise_error(CompressionError) + + cc = EncodingContext.new + cc.max_table_size = 256 + cc.max_table_size = 4096 + + cc.process(type: :changetablesize, value: 256) + cc.process(type: :changetablesize, value: 4096) + expect { cc.process(name: 1, type: :indexed) }.not_to raise_error + end + end + + context "table size updates" do + def updates(buffer) + cmds = [] + dc = Decompressor.new + until buffer.empty? + cmd = dc.header(buffer) + break unless cmd[:type] == :changetablesize + + cmds << cmd[:value] + end + cmds + end + + it "should not emit table size update without changes" do + expect(updates(c.encode(REQUEST_HEADERS))).to be_empty + end + + it "should emit table size update in the next header block after maximum is reduced" do + c.table_size = 256 + + expect(updates(c.encode(REQUEST_HEADERS))).to eq [256] + expect(updates(c.encode(REQUEST_HEADERS))).to be_empty + end + + it "should emit smallest and final table size when maximum is reduced and increased" do + c.table_size = 256 + c.table_size = 1024 + + expect(updates(c.encode(REQUEST_HEADERS))).to eq [256, 1024] + end + + it "should emit smallest table size when maximum is reduced and restored" do + c.table_size = 256 + c.table_size = 4096 + + expect(updates(c.encode(REQUEST_HEADERS))).to eq [256, 4096] + end + + it "should not grow table above own table size" do + c.table_size = 65_536 + + expect(updates(c.encode(REQUEST_HEADERS))).to be_empty + end + + it "should keep decoder in sync" do + d.table_size = 0 + c.table_size = 0 + + 3.times do + expect(d.decode(c.encode(REQUEST_HEADERS))).to eq REQUEST_HEADERS + end + expect(d.instance_eval { @cc.table }).to be_empty + end end context "encode" do