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