From dc86ffa4cc5cd56a4372b1a031d2125f720edc00 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 15 May 2026 18:47:58 +0100 Subject: [PATCH 01/28] memoize padding bytes, speed up huffman encoding ~2x --- lib/http/2/header/huffman.rb | 6 ++++-- sig/header/huffman.rbs | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/http/2/header/huffman.rb b/lib/http/2/header/huffman.rb index 1036174..5033ed2 100644 --- a/lib/http/2/header/huffman.rb +++ b/lib/http/2/header/huffman.rb @@ -20,6 +20,8 @@ module Huffman EOS = 256 private_constant :EOS + EOS_PADDING = (0..7).map { |n| ("1" * n).b.freeze }.freeze + # Encodes provided value via huffman encoding. # Length is not encoded in this method. # @@ -29,7 +31,7 @@ module Huffman def encode(str, buffer = "".b) bitstring = String.new("", encoding: Encoding::BINARY, capacity: (str.bytesize * 30) + ((8 - str.size) % 8)) str.each_byte { |chr| append_str(bitstring, ENCODE_TABLE[chr]) } - append_str(bitstring, "1" * ((8 - bitstring.size) % 8)) + append_str(bitstring, EOS_PADDING[(8 - bitstring.size) % 8]) pack([bitstring], "B*", buffer: buffer) end @@ -325,7 +327,7 @@ def decode(buf) [0x3fffffff, 30] ].each(&:freeze).freeze - ENCODE_TABLE = CODES.map { |c, l| [c].pack("N").unpack1("B*")[-l..-1] }.each(&:freeze).freeze + ENCODE_TABLE = CODES.map { |c, l| [c].pack("N").unpack1("B*")[-l..-1].b.freeze }.freeze end end end diff --git a/sig/header/huffman.rbs b/sig/header/huffman.rbs index 3f5e345..0757d25 100644 --- a/sig/header/huffman.rbs +++ b/sig/header/huffman.rbs @@ -9,6 +9,8 @@ module HTTP2 EOS: Integer + EOS_PADDING: Array[String] + CODES: Array[[Integer, Integer]] ENCODE_TABLE: Array[String] From 092918ee31d86fdd0c7dc1477a2363617c35b5dd Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 21 May 2026 15:29:47 +0100 Subject: [PATCH 02/28] use bytesplice (when available) when reading chunks from a string buffer this removes one less extra string compared to the previous strategy --- lib/http/2/extensions.rb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/http/2/extensions.rb b/lib/http/2/extensions.rb index 8e840b6..5ff5824 100644 --- a/lib/http/2/extensions.rb +++ b/lib/http/2/extensions.rb @@ -22,13 +22,23 @@ def append_str(str, data) end end - def read_str(str, n) - return "".b if n == 0 + if String.method_defined?(:bytesplice) + def read_str(str, n) + return "".b if n == 0 - chunk = str.byteslice(0..(n - 1)) - remaining = str.byteslice(n..-1) - remaining ? str.replace(remaining) : str.clear - chunk + chunk = str.byteslice(0, n) + str.bytesplice(0, chunk.length, "") + chunk + end + else + def read_str(str, n) + return "".b if n == 0 + + chunk = str.byteslice(0, n) + remaining = str.byteslice(n, str.size - n) + remaining ? str.replace(remaining) : str.clear + chunk + end end def read_uint32(str) From 066112f6391fea9e867384d1891c987905e8a15b Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 21 May 2026 16:21:46 +0100 Subject: [PATCH 03/28] reducing the number of string allocations on Decompressor#integer this replaces multiple calls to #read_str (which allocates a string per byte popped) by an accounting loop using String#getbyte and a single #read_str call at the end algo should be roughly the same speed, with less GC pressure as a result --- lib/http/2/extensions.rb | 4 ---- lib/http/2/header/decompressor.rb | 21 +++++++++++---------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/http/2/extensions.rb b/lib/http/2/extensions.rb index 5ff5824..b0c4ed7 100644 --- a/lib/http/2/extensions.rb +++ b/lib/http/2/extensions.rb @@ -44,10 +44,6 @@ def read_str(str, n) def read_uint32(str) read_str(str, 4).unpack1("N") end - - def shift_byte(str) - read_str(str, 1).ord - end end # this mixin handles backwards-compatibility for the new packing options diff --git a/lib/http/2/header/decompressor.rb b/lib/http/2/header/decompressor.rb index 03cf01e..6f0385b 100644 --- a/lib/http/2/header/decompressor.rb +++ b/lib/http/2/header/decompressor.rb @@ -33,24 +33,25 @@ def table_size=(size) # @return [Integer] def integer(buf, n) limit = (1 << n) - 1 - i = n.zero? ? 0 : (shift_byte(buf) & limit) + if n.zero? + i = 0 + consumed = 0 + else + i = buf.getbyte(0) & limit + consumed = 1 + end - m = 0 if i == limit - offset = 0 - - buf.each_byte.with_index do |byte, idx| - offset = idx - # while (byte = shift_byte(buf)) + m = 0 + while (byte = buf.getbyte(consumed)) i += ((byte & 127) << m) m += 7 - + consumed += 1 break if byte.nobits?(128) end - - read_str(buf, offset + 1) end + read_str(buf, consumed) i end From 35e74077bfab6a3eb24872eebc6dbacadb9ddcc2 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 21 May 2026 16:26:01 +0100 Subject: [PATCH 04/28] correctly set buffer size for SETTINGS frame buffer --- lib/http/2/framer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index b09c118..3f9ef26 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -248,7 +248,7 @@ def generate(frame) end settings = frame[:payload] - bytes = String.new("", encoding: Encoding::BINARY, capacity: length) + bytes = String.new("", encoding: Encoding::BINARY, capacity: settings.size * 6) settings.each do |(k, v)| if k.is_a? Integer # rubocop:disable Style/GuardClause From 8e99b96a0c580b2d8d920da2e944710fb30c5a03 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Thu, 21 May 2026 16:30:07 +0100 Subject: [PATCH 05/28] eliminate DEFINED_SETTINGS O(n) walk by direct lookup this comes at the cost of having an extra global hash, albeit a small one. to compensate, DEFINE_SETTINGS was turned into an array. there's the space saving, but Array#include?, despite O(n), should be more performant than Hash#key. --- lib/http/2/framer.rb | 45 +++++++++++++++++++++++--------------------- sig/framer.rbs | 6 ++++-- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index 3f9ef26..cef92f7 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -83,23 +83,25 @@ class Framer settings_max_header_list_size: 6 }.freeze - # Default error types as defined by the spec - DEFINED_ERRORS = { - no_error: 0, - protocol_error: 1, - internal_error: 2, - flow_control_error: 3, - settings_timeout: 4, - stream_closed: 5, - frame_size_error: 6, - refused_stream: 7, - cancel: 8, - compression_error: 9, - connect_error: 10, - enhance_your_calm: 11, - inadequate_security: 12, - http_1_1_required: 13 - }.freeze + DEFINED_SETTINGS_BY_ID = DEFINED_SETTINGS.invert.freeze + + # Default error types as defined by the spec (the code is the array index) + DEFINED_ERRORS = %i[ + no_error + protocol_error + internal_error + flow_control_error + settings_timeout + stream_closed + frame_size_error + refused_stream + cancel + compression_error + connect_error + enhance_your_calm + inadequate_security + http_1_1_required + ].freeze RBIT = 0x7fffffff RBYTE = 0x0fffffff @@ -447,8 +449,9 @@ def parse(buf) # Unsupported or unrecognized settings MUST be ignored. # Here we send it along. - name, = DEFINED_SETTINGS.find { |_name, v| v == id } - frame[:payload] << [name, val] if name + if (name = DEFINED_SETTINGS_BY_ID[id]) + frame[:payload] << [name, val] + end end when :push_promise frame[:promise_stream] = read_uint32(payload) & RBIT @@ -495,7 +498,7 @@ def parse(buf) def pack_error(error, buffer:) unless error.is_a? Integer - error = DEFINED_ERRORS[error] + error = DEFINED_ERRORS.index(error) raise CompressionError, "Unknown error ID for #{error}" unless error end @@ -504,7 +507,7 @@ def pack_error(error, buffer:) end def unpack_error(error) - DEFINED_ERRORS.key(error) || error + DEFINED_ERRORS.fetch(error, error) end end end diff --git a/sig/framer.rbs b/sig/framer.rbs index 5205990..4ff70c6 100644 --- a/sig/framer.rbs +++ b/sig/framer.rbs @@ -20,7 +20,9 @@ module HTTP2 DEFINED_SETTINGS: Hash[Symbol, Integer] - DEFINED_ERRORS: Hash[Symbol, Integer] + DEFINED_SETTINGS_BY_ID: Hash[Integer, Symbol] + + DEFINED_ERRORS: Array[Symbol] RBIT: Integer RBYTE: Integer @@ -53,7 +55,7 @@ module HTTP2 def initialize: (?Integer local_max_frame_size, ?Integer remote_max_frame_size) -> untyped - def pack_error: (Integer | Symbol error, buffer: String) -> String + def pack_error: (Symbol | Integer error, buffer: String) -> String def unpack_error: (Integer) -> (Symbol | Integer) end From ad61754a70ceaa6f05d52e2453b4ea9e4308eeea Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 11:49:36 +0100 Subject: [PATCH 06/28] manage flags as integers instead of as arrays of symbols frame containers would store flag information as arrays of symbols, which would then be passed up and down the encoding/decoding chain in that format before being translated to/from the actual bits that are encoded into the buffer. this would cause some overhead, given that 1) arrays of symbols generate more GC pressure than raw integers (tagged objects); 2) array operations vs bitwise operations; 3) translation overhead (and given it used bit positions rather than the bit representation, bit shifting). this approach is replaced by carrying the bits around, which solves all problems, at the expense of making the flag less readable when exposed to the end user via the frame_sent/received callbacks. for that reason, one can consider it a "regression", but IMO this is an internal representation that end users shouldn't be relying on for anything. --- lib/http/2/connection.rb | 25 +++++---- lib/http/2/flow_buffer.rb | 6 +-- lib/http/2/framer.rb | 81 ++++++++++++------------------ lib/http/2/server.rb | 12 ++--- lib/http/2/stream.rb | 15 +++--- sig/2.rbs | 14 ++++-- sig/server.rbs | 2 +- spec/client_spec.rb | 8 +-- spec/framer_spec.rb | 24 ++++----- spec/helper.rb | 10 ++-- spec/shared_examples/connection.rb | 34 ++++++------- spec/stream_spec.rb | 66 ++++++++++++------------ 12 files changed, 143 insertions(+), 154 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index d3cf180..40f9965 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -240,7 +240,7 @@ def receive(data) connection_error unless frame_type == :continuation && stream_id == @continuation.first[:stream] @continuation << frame - unless frame[:flags].include? :end_headers + unless frame[:flags].anybits?(END_HEADERS) buffered_payload = @continuation.sum { |f| f[:payload].bytesize } # prevent HTTP/2 CONTINUATION FLOOD # same heuristic as the one from HAProxy: https://www.haproxy.com/blog/haproxy-is-resilient-to-the-http-2-continuation-flood @@ -262,7 +262,7 @@ def receive(data) frame.delete(:length) frame[:payload] = payload - frame[:flags] << :end_headers + frame[:flags] |= END_HEADERS end # SETTINGS frames always apply to a connection, never a single stream. @@ -282,7 +282,7 @@ def receive(data) # The last frame in a sequence of HEADERS/CONTINUATION # frames MUST have the END_HEADERS flag set. - unless frame[:flags].include? :end_headers + unless frame[:flags].anybits?(END_HEADERS) @continuation << frame next end @@ -314,7 +314,7 @@ def receive(data) when :push_promise # The last frame in a sequence of PUSH_PROMISE/CONTINUATION # frames MUST have the END_HEADERS flag set - unless frame[:flags].include? :end_headers + unless frame[:flags].anybits?(END_HEADERS) @continuation << frame return end @@ -512,7 +512,7 @@ def connection_management(frame) # information is invalid and MUST be ignored. emit(:altsvc, frame) if origin && !origin.empty? when :origin - return if @h2c_upgrade || !frame[:flags].empty? + return if @h2c_upgrade || !frame[:flags].zero? frame[:payload].each do |orig| emit(:origin, orig) @@ -538,11 +538,10 @@ def connection_management(frame) end def ping_management(frame) - if frame[:flags].include? :ack + if frame[:flags].anybits?(ACK) emit(:ack, frame[:payload]) else - send(type: :ping, stream: 0, - flags: [:ack], payload: frame[:payload]) + send(type: :ping, stream: 0, flags: ACK, payload: frame[:payload]) end end @@ -605,7 +604,7 @@ def connection_settings(frame) # side = # local: previously sent and pended our settings should be effective # remote: just received peer settings should immediately be effective - if frame[:flags].include?(:ack) + if frame[:flags].anybits?(ACK) # Process pending settings we have sent. settings = @pending_settings.shift side = :local @@ -681,7 +680,7 @@ def connection_settings(frame) when :remote unless @state == :closed || @h2c_upgrade == :start # Send ack to peer - send(type: :settings, stream: 0, payload: [], flags: [:ack]) + send(type: :settings, stream: 0, payload: EMPTY, flags: ACK) # when initial window size changes, we try to flush any buffered # data. @streams.each_value(&:flush) @@ -727,7 +726,7 @@ def encode_headers(headers_frame) end # split into multiple CONTINUATION frames - headers_frame[:flags].delete(:end_headers) + headers_frame[:flags] ^= END_HEADERS headers_frame[:payload] = payload.byteslice(0, max_frame_size) payload = payload.byteslice(max_frame_size..-1) @@ -737,14 +736,14 @@ def encode_headers(headers_frame) loop do continuation_frame = headers_frame.merge( type: :continuation, - flags: EMPTY, + flags: 0, payload: payload.byteslice(0, max_frame_size) ) payload = payload.byteslice(max_frame_size..-1) if payload.nil? || payload.empty? - continuation_frame[:flags] = [:end_headers] + continuation_frame[:flags] |= END_HEADERS emit(:frame, @framer.generate(continuation_frame)) break end diff --git a/lib/http/2/flow_buffer.rb b/lib/http/2/flow_buffer.rb index 9f72691..0989028 100644 --- a/lib/http/2/flow_buffer.rb +++ b/lib/http/2/flow_buffer.rb @@ -70,7 +70,7 @@ def send_data(frame = nil, encode = false) if frame if @send_buffer.empty? frame_size = frame[:payload].bytesize - end_stream = frame[:flags].include?(:end_stream) + end_stream = frame[:flags].anybits?(END_STREAM) # if buffer is empty, and frame is either end 0 length OR # is within available window size, skip buffering and send immediately. if @remote_window.positive? @@ -140,7 +140,7 @@ def retrieve(window_size) frame = @buffer.first or return frame_size = frame[:payload].bytesize - end_stream = frame[:flags].include?(:end_stream) + end_stream = frame[:flags].anybits?(END_STREAM) # Frames with zero length with the END_STREAM flag set (that # is, an empty DATA frame) MAY be sent if there is no available space @@ -160,7 +160,7 @@ def retrieve(window_size) frame[:length] = frame_size - window_size # if no longer last frame in sequence... - chunk[:flags] -= [:end_stream] if end_stream + chunk[:flags] ^= END_STREAM if end_stream @bytesize -= window_size chunk diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index cef92f7..7565cc7 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -1,6 +1,25 @@ # frozen_string_literal: true module HTTP2 + # Frame flags as defined by the spec (max 255 bits) + # DATA: ( X X COMPRESSED X PADDED X X END_STREAM ) + # HEADERS: ( X X PRIORITY X PADDED END_HEADERS X END_STREAM ) + # PRIORITY: ( X X X X X X X X ) + # RST_STREAM: ( X X X X X X X X ) + # SETTINGS: ( X X X X X X X ACK ) + # PUSH_PROMISE: ( X X X X PADDED END_HEADERS X X ) + # PING: ( X X X X X X X ACK ) + # GOAWAY: ( X X X X X X X X ) + # WINDOW_UPDATE: ( X X X X X X X X ) + # CONTINUATION: ( X X X X X END_HEADERS X X ) + # ALTSVC: ( X X X X X X X X ) + # ORIGIN: ( RESERVED4 X X RESERVED3 X RESERVED2 RESERVED X ) + END_STREAM = ACK = 0b0001 # 1 + RESERVED = 0b0010 # 2 + END_HEADERS = 0b0100 # 4 + PADDED = 0b1000 # 8 + PRIORITY = 0b0010_0000 # 32 + # Performs encoding, decoding, and validation of binary HTTP/2 frames. # class Framer @@ -40,39 +59,6 @@ class Framer FRAME_TYPES_WITH_PADDING = %i[data headers push_promise].freeze - # Per frame flags as defined by the spec - FRAME_FLAGS = { - data: { - end_stream: 0, - padded: 3, - compressed: 5 - }, - headers: { - end_stream: 0, - end_headers: 2, - padded: 3, - priority: 5 - }, - priority: {}, - rst_stream: {}, - settings: { ack: 0 }, - push_promise: { - end_headers: 2, - padded: 3 - }, - ping: { ack: 0 }, - goaway: {}, - window_update: {}, - continuation: { end_headers: 2 }, - altsvc: {}, - origin: { - reserved: 1, - reserved2: 2, - reserved3: 4, - reserved4: 8 - } - }.each_value(&:freeze).freeze - # Default settings as defined by the spec DEFINED_SETTINGS = { settings_header_table_size: 1, @@ -148,6 +134,10 @@ def common_header(frame, buffer:) raise CompressionError, "Window increment (#{frame[:increment]}) is too large" end + flags = frame[:flags] + + raise CompressionError, "Invalid frame flag (#{flags}) for #{type}" unless flags.between?(0, 255) + header = buffer # make sure the buffer is binary and unfrozen @@ -162,12 +152,7 @@ def common_header(frame, buffer:) (length >> FRAME_LENGTH_HISHIFT), (length & FRAME_LENGTH_LOMASK), FRAME_TYPES[type], - frame[:flags].reduce(0) do |acc, f| - position = FRAME_FLAGS[type][f] - raise CompressionError, "Invalid frame flag (#{f}) for #{type}" unless position - - acc | (1 << position) - end, + flags, stream_id ], HEADERPACK, buffer: header, offset: 0) # 8+16,8,8,32 end @@ -186,9 +171,7 @@ def read_common_header(buf) { type: type, - flags: FRAME_FLAGS[type].filter_map do |name, pos| - name if flags.anybits?(1 << pos) - end, + flags: flags, length: length, stream: stream & RBIT } @@ -200,7 +183,7 @@ def read_common_header(buf) # @param frame [Hash] def generate(frame) length = 0 - frame[:flags] ||= EMPTY + frame[:flags] ||= 0 case frame[:type] when :data, :continuation @@ -215,10 +198,10 @@ def generate(frame) raise CompressionError, "Must specify all of priority parameters for #{frame[:type]}" end - frame[:flags] += [:priority] unless frame[:flags].include?(:priority) + frame[:flags] |= PRIORITY end - if frame[:flags].include?(:priority) + if frame[:flags].anybits?(PRIORITY) length = 5 + headers.bytesize bytes = String.new("", encoding: Encoding::BINARY, capacity: length) pack([(frame[:exclusive] ? EBIT : 0) | (frame[:dependency] & RBIT)], UINT32, buffer: bytes) @@ -356,7 +339,7 @@ def generate(frame) length += padlen pack([padlen -= 1], UINT8, buffer: bytes, offset: 0) - frame[:flags] += [:padded] + frame[:flags] |= PADDED # Padding: Padding octets that contain no application semantic value. # Padding octets MUST be set to zero when sending and ignored when @@ -396,7 +379,7 @@ def parse(buf) # Process padding padlen = 0 if FRAME_TYPES_WITH_PADDING.include?(type) - padded = flags.include?(:padded) + padded = flags.anybits?(PADDED) if padded padlen = read_str(payload, 1).unpack1(UINT8) frame[:padding] = padlen + 1 @@ -404,7 +387,7 @@ def parse(buf) payload = payload.byteslice(0, payload.bytesize - padlen) if padlen > 0 frame[:length] -= frame[:padding] - flags.delete(:padded) + frame[:flags] ^= PADDED end end @@ -412,7 +395,7 @@ def parse(buf) when :data, :ping, :continuation frame[:payload] = read_str(payload, length) when :headers - if flags.include?(:priority) + if flags.anybits?(PRIORITY) e_sd = read_uint32(payload) frame[:dependency] = e_sd & RBIT frame[:exclusive] = e_sd.anybits?(EBIT) diff --git a/lib/http/2/server.rb b/lib/http/2/server.rb index a91f041..8f00ecb 100644 --- a/lib/http/2/server.rb +++ b/lib/http/2/server.rb @@ -85,7 +85,7 @@ def upgrade(settings, headers, body) length: buf.bytesize, type: :settings, stream: 0, - flags: [] + flags: 0 }, buffer: buf ) @@ -97,7 +97,7 @@ def upgrade(settings, headers, body) headers_frame = { type: :headers, - flags: [:end_headers], + flags: END_HEADERS, stream: 1, weight: DEFAULT_WEIGHT, dependency: 0, @@ -106,11 +106,11 @@ def upgrade(settings, headers, body) } if body.empty? - headers_frame[:flags] << [:end_stream] + headers_frame[:flags] |= END_HEADERS stream << headers_frame else stream << headers_frame - stream << { type: :data, stream: 1, payload: body, flags: [:end_stream] } + stream << { type: :data, stream: 1, payload: body, flags: END_STREAM } end # Mark h2c upgrade as finished @@ -135,7 +135,7 @@ def origin_set=(origins) def connection_settings(frame) super - return unless frame[:flags].include?(:ack) && !@origins_sent + return unless frame[:flags].anybits?(ACK) && !@origins_sent send(type: :origin, stream: 0, payload: @origin_set) end @@ -148,7 +148,7 @@ def verify_pseudo_headers(frame) # # @param parent [Stream] # @param headers [Enumerable[String, String]] - # @param flags [Array[Symbol]] + # @param flags Integer # @param callback [Proc] def promise(parent, headers, flags) promise = new_stream(parent: parent) diff --git a/lib/http/2/stream.rb b/lib/http/2/stream.rb index 796f3b5..c64c286 100644 --- a/lib/http/2/stream.rb +++ b/lib/http/2/stream.rb @@ -219,9 +219,8 @@ def send(frame) # @param end_headers [Boolean] indicates that no more headers will be sent # @param end_stream [Boolean] indicates that no payload will be sent def headers(headers, end_headers: true, end_stream: false) - flags = [] - flags << :end_headers if end_headers - flags << :end_stream if end_stream || @_method == "HEAD" + flags = end_headers ? END_HEADERS : 0 + flags |= END_STREAM if end_stream || @_method == "HEAD" send(type: :headers, flags: flags, payload: headers) end @@ -229,7 +228,7 @@ def headers(headers, end_headers: true, end_stream: false) def promise(headers, end_headers: true, &block) raise ArgumentError, "must provide callback" unless block - flags = end_headers ? [:end_headers] : [] + flags = end_headers ? END_HEADERS : 0 emit(:promise, self, headers, flags, &block) end @@ -254,12 +253,12 @@ def data(payload, end_stream: true) if payload.bytesize > max_size payload = chunk_data(payload, max_size) do |chunk| - send(type: :data, flags: [], payload: chunk) + send(type: :data, flags: 0, payload: chunk) end end - flags = [] - flags << :end_stream if end_stream + flags = 0 + flags |= END_STREAM if end_stream send(type: :data, flags: flags, payload: payload) end @@ -663,7 +662,7 @@ def process_priority(frame) def end_stream?(frame) case frame[:type] when :data, :headers, :continuation - frame[:flags] && frame[:flags].include?(:end_stream) + frame[:flags] && frame[:flags].anybits?(END_STREAM) else false end end diff --git a/sig/2.rbs b/sig/2.rbs index 7887f62..f8fcd6e 100644 --- a/sig/2.rbs +++ b/sig/2.rbs @@ -36,10 +36,18 @@ module HTTP2 RESPONSE_MANDATORY_HEADERS: Array[String] + # Frame flags + END_STREAM: Integer + ACK: Integer + RESERVED: Integer + END_HEADERS: Integer + PADDED: Integer + PRIORITY: Integer + type header_pair = [string, string] # # FRAMES - type frame_control_flags = Array[:end_headers | :end_stream] + type frame_control_flags = Integer type common_frame = { stream: Integer } @@ -56,7 +64,7 @@ module HTTP2 type push_promise_frame = { type: :push_promise, promise_stream: Integer, flags: frame_control_flags, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, payload: Enumerable[header_pair], ?padding: Integer } # # SETTINGS - type settings_frame = { type: :settings, payload: Array[[Symbol | Integer, Integer]] } + type settings_frame = { type: :settings, ?payload: Array[[Symbol | Integer, Integer]], ?flags: frame_control_flags } # # WINDOW_UPDATE type window_update_frame = { type: :window_update, increment: Integer } @@ -71,7 +79,7 @@ module HTTP2 type origin_frame = { type: :origin, origin: Array[String] } # # PING - type ping_frame = { type: :ping, payload: String, length: Integer } + type ping_frame = { type: :ping, payload: String, length: Integer, ?flags: frame_control_flags } # # GOAWAY type goaway_frame = { type: :goaway, last_stream: Integer, error: Symbol? } diff --git a/sig/server.rbs b/sig/server.rbs index 28d2290..dd46e5c 100644 --- a/sig/server.rbs +++ b/sig/server.rbs @@ -7,6 +7,6 @@ module HTTP2 private - def promise: (Stream parent, Enumerable[header_pair] headers, Array[Symbol] flags) { (Stream) -> void } -> void + def promise: (Stream parent, Enumerable[header_pair] headers, frame_control_flags flags) { (Stream) -> void } -> void end end \ No newline at end of file diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 2198c72..3021d9e 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -72,7 +72,7 @@ expect(f.parse(frames[1])[:type]).to eq :settings ack_frame = f.parse(frames[2]) expect(ack_frame[:type]).to eq :settings - expect(ack_frame[:flags]).to include(:ack) + expect(ack_frame[:flags]).to be_anybits(ACK) end end @@ -83,7 +83,7 @@ client.settings(settings_header_table_size: 256) expect(client.local_settings[:settings_header_table_size]).to eq 4096 - ack = { type: :settings, stream: 0, payload: [], flags: [:ack] } + ack = { type: :settings, stream: 0, payload: [], flags: ACK } client << f.generate(ack) expect(client.local_settings[:settings_header_table_size]).to eq 256 @@ -252,7 +252,7 @@ end end context "when receiving a reserved flag" do - let(:orig_frame) { origin_frame.merge(flags: [:reserved]) } + let(:orig_frame) { origin_frame.merge(flags: RESERVED) } it "should be ignored" do client << f.generate(settings_frame) origins = [] @@ -350,7 +350,7 @@ payload = cc.encode(req_headers) h1[:payload] = payload.slice!(0, payload.size / 2) # first half h1[:stream] = 2 - h1[:flags] = [] + h1[:flags] = 0 h2[:payload] = payload # the remaining h2[:stream] = 2 diff --git a/spec/framer_spec.rb b/spec/framer_spec.rb index 265d3de..b62aa6c 100644 --- a/spec/framer_spec.rb +++ b/spec/framer_spec.rb @@ -10,7 +10,7 @@ { length: 4, type: :headers, - flags: %i[end_stream end_headers], + flags: END_HEADERS | END_STREAM, stream: 15 } end @@ -31,7 +31,7 @@ frame = { length: (2**18) + (2**16) + 17, type: :headers, - flags: %i[end_stream end_headers], + flags: END_HEADERS | END_STREAM, stream: 15 } bytes = [5, 17, 0x01, 0x5, 0x0000000F].pack("CnCCN") @@ -55,7 +55,7 @@ it "should raise exception on invalid frame flag" do expect do - frame[:flags] = [:bogus] + frame[:flags] = 1024 f.common_header(frame, buffer: "".b) end.to raise_error(CompressionError, /frame flag/) end @@ -73,7 +73,7 @@ frame = { length: 4, type: :data, - flags: [:end_stream], + flags: END_STREAM, stream: 1, payload: "text" } @@ -90,7 +90,7 @@ frame = { length: 12, type: :headers, - flags: %i[end_stream end_headers], + flags: END_HEADERS | END_STREAM, stream: 1, payload: "header-block" } @@ -104,7 +104,7 @@ frame = { length: 16, type: :headers, - flags: [:end_headers], + flags: END_HEADERS, stream: 1, dependency: 15, weight: 12, @@ -154,7 +154,7 @@ let(:frame) do { type: :settings, - flags: [], + flags: 0, stream: 0, payload: [ [:settings_max_concurrent_streams, 10], @@ -242,7 +242,7 @@ frame = { length: 11, type: :push_promise, - flags: [:end_headers], + flags: END_HEADERS, stream: 1, promise_stream: 2, payload: "headers" @@ -260,7 +260,7 @@ length: 8, stream: 1, type: :ping, - flags: [:ack], + flags: ACK, payload: "12345678" } end @@ -340,7 +340,7 @@ length: 12, type: :continuation, stream: 1, - flags: [:end_headers], + flags: END_HEADERS, payload: "header-block" } @@ -443,7 +443,7 @@ it "should determine frame length" do frames = [ - [{ type: :data, stream: 1, flags: [:end_stream], payload: "abc" }, 3], + [{ type: :data, stream: 1, flags: END_STREAM, payload: "abc" }, 3], [{ type: :headers, stream: 1, payload: "abc" }, 3], [{ type: :priority, stream: 3, dependency: 30, exclusive: false, weight: 1 }, 5], [{ type: :rst_stream, stream: 3, error: 100 }, 4], @@ -465,7 +465,7 @@ it "should parse single frame at a time" do frames = [ { type: :headers, stream: 1, payload: "headers" }, - { type: :data, stream: 1, flags: [:end_stream], payload: "abc" } + { type: :data, stream: 1, flags: END_STREAM, payload: "abc" } ] buf = f.generate(frames[0]) << f.generate(frames[1]) diff --git a/spec/helper.rb b/spec/helper.rb index 802229a..488815d 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -34,7 +34,7 @@ module FrameHelpers def data_frame { type: :data, - flags: [:end_stream], + flags: END_STREAM, stream: 1, payload: "text" } @@ -43,7 +43,7 @@ def data_frame def headers_frame { type: :headers, - flags: [:end_headers].freeze, + flags: END_HEADERS, stream: 1, payload: Compressor.new.encode(REQUEST_HEADERS) } @@ -81,7 +81,7 @@ def settings_frame def push_promise_frame { type: :push_promise, - flags: [:end_headers], + flags: END_HEADERS, stream: 1, promise_stream: 2, payload: Compressor.new.encode(REQUEST_HEADERS) @@ -100,7 +100,7 @@ def pong_frame { stream: 0, type: :ping, - flags: [:ack], + flags: ACK, payload: "12345678" } end @@ -125,7 +125,7 @@ def continuation_frame { type: :continuation, stream: 1, - flags: [:end_headers], + flags: END_HEADERS, payload: "-second-block" } end diff --git a/spec/shared_examples/connection.rb b/spec/shared_examples/connection.rb index a2e9f20..b2e032d 100644 --- a/spec/shared_examples/connection.rb +++ b/spec/shared_examples/connection.rb @@ -29,7 +29,7 @@ frame = frames.last expect(frame[:type]).to eq :settings - expect(frame[:flags]).to eq [:ack] + expect(frame[:flags]).to eq ACK expect(frame[:payload]).to eq [] end end @@ -119,7 +119,7 @@ conn << f.generate(settings_frame) expect(conn).to receive(:send) do |frame| expect(frame[:type]).to eq :ping - expect(frame[:flags]).to eq [:ack] + expect(frame[:flags]).to eq ACK expect(frame[:payload]).to eq "12345678" end @@ -284,10 +284,10 @@ it "should chain continuation frames" do headers = headers_frame - headers[:flags] = [] + headers[:flags] = 0 continuation = continuation_frame continuation[:stream] = headers[:stream] - continuation[:flags] = [] + continuation[:flags] = 0 conn << f.generate(headers) conn << f.generate(continuation) @@ -298,14 +298,14 @@ max_frame_size = connected_conn.local_settings[:settings_max_frame_size] headers = headers_frame - headers[:flags] = [] + headers[:flags] = 0 conn << f.generate(headers) expect do max_frame_size.times do continuation = continuation_frame continuation[:stream] = headers[:stream] - continuation[:flags] = [] + continuation[:flags] = 0 conn << f.generate(continuation) end end.to raise_error(ProtocolError) @@ -313,7 +313,7 @@ it "should require that split header blocks are a contiguous sequence" do headers = headers_frame - headers[:flags] = [] + headers[:flags] = 0 conn << f.generate(headers) (frame_types - [continuation_frame]).each do |frame| @@ -323,7 +323,7 @@ it "should require that split promise blocks are a contiguous sequence" do headers = push_promise_frame - headers[:flags] = [] + headers[:flags] = 0 conn << f.generate(headers) (frame_types - [continuation_frame]).each do |frame| @@ -381,9 +381,9 @@ expect(headers[0][:type]).to eq :headers expect(headers[1][:type]).to eq :continuation expect(headers[2][:type]).to eq :continuation - expect(headers[0][:flags]).to eq [:end_stream] - expect(headers[1][:flags]).to eq [] - expect(headers[2][:flags]).to eq [:end_headers] + expect(headers[0][:flags]).to eq END_STREAM + expect(headers[1][:flags]).to eq 0 + expect(headers[2][:flags]).to eq END_HEADERS end it "should not generate CONTINUATION if HEADERS fits exactly in a frame" do @@ -404,8 +404,8 @@ expect(headers[0][:length]).to eq conn.remote_settings[:settings_max_frame_size] expect(headers.size).to eq 1 expect(headers[0][:type]).to eq :headers - expect(headers[0][:flags]).to include(:end_headers) - expect(headers[0][:flags]).to include(:end_stream) + expect(headers[0][:flags]).to be_anybits(END_HEADERS) + expect(headers[0][:flags]).to be_anybits(END_STREAM) end it "should not generate CONTINUATION if HEADERS fits exactly in a frame" do @@ -426,8 +426,8 @@ expect(headers[0][:length]).to eq conn.remote_settings[:settings_max_frame_size] expect(headers.size).to eq 1 expect(headers[0][:type]).to eq :headers - expect(headers[0][:flags]).to include(:end_headers) - expect(headers[0][:flags]).to include(:end_stream) + expect(headers[0][:flags]).to be_anybits(END_HEADERS) + expect(headers[0][:flags]).to be_anybits(END_STREAM) end it "should generate CONTINUATION if HEADERS exceed the max payload by one byte" do @@ -449,8 +449,8 @@ expect(headers.size).to eq 2 expect(headers[0][:type]).to eq :headers expect(headers[1][:type]).to eq :continuation - expect(headers[0][:flags]).to eq [:end_stream] - expect(headers[1][:flags]).to eq [:end_headers] + expect(headers[0][:flags]).to eq END_STREAM + expect(headers[1][:flags]).to eq END_HEADERS end end context "API" do diff --git a/spec/stream_spec.rb b/spec/stream_spec.rb index 692d36f..d8938c1 100644 --- a/spec/stream_spec.rb +++ b/spec/stream_spec.rb @@ -165,7 +165,7 @@ [data_frame, headers_frame].each do |frame| s = stream.dup - s.send frame.merge(flags: [:end_stream]) + s.send frame.merge(flags: END_STREAM) expect(s.state).to eq :half_closed_local end end @@ -174,7 +174,7 @@ [data_frame, headers_frame].each do |frame| s = stream.dup f = frame.dup - f[:flags] = [:end_stream] + f[:flags] = END_STREAM s.receive f expect(s.state).to eq :half_closed_remote @@ -184,7 +184,7 @@ it "should transition to half closed if remote opened with END_STREAM" do s = client.new_stream hclose = headers_frame - hclose[:flags] = [:end_stream] + hclose[:flags] = END_STREAM s.receive hclose expect(s.state).to eq :half_closed_remote @@ -193,7 +193,7 @@ it "should transition to half closed if local opened with END_STREAM" do s = client.new_stream hclose = headers_frame - hclose[:flags] = [:end_stream] + hclose[:flags] = END_STREAM s.send hclose expect(s.state).to eq :half_closed_local @@ -234,7 +234,7 @@ stream.on(:close) { order << :close } req = headers_frame - req[:flags] = [:end_headers] + req[:flags] = END_HEADERS stream.send req stream.send data_frame @@ -267,7 +267,7 @@ stream.on(:close) { order << :close } req = headers_frame - req[:flags] = %i[end_stream end_headers] + req[:flags] = END_HEADERS | END_STREAM stream.send req stream.receive headers_frame @@ -294,7 +294,7 @@ end context "half closed (local)" do - before { stream.send headers_frame.merge(flags: %i[end_headers end_stream]) } + before { stream.send headers_frame.merge(flags: END_HEADERS | END_STREAM) } it "should raise error on attempt to send invalid frames" do frame_types.reject { |frame| %i[priority rst_stream window_update].include?(frame[:type]) }.each do |frame| @@ -306,7 +306,7 @@ [data_frame, headers_frame, continuation_frame].each do |frame| s = stream.dup f = frame.dup - f[:flags] = [:end_stream] + f[:flags] = END_STREAM s.receive f expect(s.state).to eq :closed @@ -355,7 +355,7 @@ stream.on(:half_close) { order << :half_close } req = headers_frame - req[:flags] = %i[end_stream end_headers] + req[:flags] = END_HEADERS | END_STREAM stream.send req expect(order).to eq %i[active half_close] @@ -372,7 +372,7 @@ end context "half closed (remote)" do - before { stream.receive headers_frame.merge(flags: %i[end_headers end_stream]) } + before { stream.receive headers_frame.merge(flags: END_HEADERS | END_STREAM) } it "should raise STREAM_CLOSED error on reciept of frames" do (frame_types - [priority_frame, rst_stream_frame, window_update_frame]).each do |frame| @@ -387,16 +387,16 @@ s = stream.dup s.on(:close) { expect(s.state).to eq :closed } - s.send frame.merge(flags: [:end_stream]) + s.send frame.merge(flags: END_STREAM) expect(s.state).to eq :closed end end it "should not transition to closed if END_STREAM flag is sent when overflowing window" do stream.on(:close) { raise "should not have closed" } - data = { type: :data, flags: [], stream: stream.id } + data = { type: :data, flags: 0, stream: stream.id } 4.times do - data = data.merge(flags: [:end_stream]) if stream.remote_window < 16_384 + data = data.merge(flags: END_STREAM) if stream.remote_window < 16_384 stream.send data.merge(payload: "x" * 16_384) end end @@ -408,9 +408,9 @@ expect(stream.buffered_amount).to eq 0 o.tap end - data = { type: :data, flags: [], stream: stream.id } + data = { type: :data, flags: 0, stream: stream.id } 4.times do - data = data.merge(flags: [:end_stream]) if stream.remote_window < 16_384 + data = data.merge(flags: END_STREAM) if stream.remote_window < 16_384 stream.send data.merge(payload: "x" * 16_384) end client << f.generate(settings_frame) @@ -453,7 +453,7 @@ stream.on(:half_close) { order << :half_close } req = headers_frame - req[:flags] = %i[end_stream end_headers] + req[:flags] = END_HEADERS | END_STREAM stream.receive req expect(order).to eq %i[active half_close] @@ -472,8 +472,8 @@ context "closed" do context "remote closed stream" do before do - stream.send headers_frame.merge(flags: %i[end_headers end_stream]) # half closed local - stream.receive headers_frame.merge(flags: %i[end_headers end_stream]) # closed by remote + stream.send headers_frame.merge(flags: END_HEADERS | END_STREAM) # half closed local + stream.receive headers_frame.merge(flags: END_HEADERS | END_STREAM) # closed by remote end it "should raise STREAM_CLOSED on attempt to send frames" do @@ -602,7 +602,7 @@ s1 = client.new_stream s1.send headers_frame - s1.send data.merge(payload: "x" * 900, flags: []) + s1.send data.merge(payload: "x" * 900, flags: 0) expect(s1.remote_window).to eq 100 s1.send data.merge(payload: "x" * 200) @@ -626,7 +626,7 @@ end it "should update window when data received is over half of the maximum local window size" do - data1 = data_frame.merge(payload: "a" * 16_384, flags: []) + data1 = data_frame.merge(payload: "a" * 16_384, flags: 0) data2 = data_frame.merge(payload: "a" * 16_384) datalen = 16_384 * 2 expect(stream).to receive(:send) do |frame| @@ -649,13 +649,13 @@ s1 = client.new_stream s1.send headers_frame - s1.send data.merge(payload: "x" * 1000, flags: []) + s1.send data.merge(payload: "x" * 1000, flags: 0) # check if window is exhausted expect(s1.remote_window).to be(0) expect(s1.send_buffer).to be_empty - s1.send data.merge(payload: "", flags: [:end_stream]) + s1.send data.merge(payload: "", flags: END_STREAM) expect(s1.remote_window).to be(0) expect(s1.send_buffer).to be_empty @@ -691,7 +691,7 @@ expect(stream).to receive(:send) do |frame| expect(frame[:type]).to eq :headers expect(frame[:payload]).to eq payload - expect(frame[:flags]).to eq [:end_headers] + expect(frame[:flags]).to eq END_HEADERS end stream.headers(payload, end_stream: false, end_headers: true) @@ -701,12 +701,12 @@ expect(stream).to receive(:send) do |frame| expect(frame[:type]).to eq :data expect(frame[:payload]).to eq "text" - expect(frame[:flags]).to be_empty + expect(frame[:flags]).to be(0) end stream.data("text", end_stream: false) expect(stream).to receive(:send) do |frame| - expect(frame[:flags]).to eq [:end_stream] + expect(frame[:flags]).to eq END_STREAM end stream.data("text") end @@ -715,9 +715,9 @@ data = "x" * 16_384 * 2 want = [ - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [:end_stream], length: 1 } + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: END_STREAM, length: 1 } ] want.each do |w| expect(stream).to receive(:send) do |frame| @@ -734,11 +734,11 @@ data = "🐼" * 16_384 want = [ - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [], length: 16_384 }, - { type: :data, flags: [:end_stream], length: 1 } + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: 0, length: 16_384 }, + { type: :data, flags: END_STREAM, length: 1 } ] want.each do |w| expect(stream).to receive(:send) do |frame| From e0adbbca6f37aae21692becbdd7d4cff51ece4ac Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 13:15:22 +0100 Subject: [PATCH 07/28] improving typing --- .rubocop.yml | 2 ++ lib/http/2/connection.rb | 18 +++++++++++++----- lib/http/2/framer.rb | 7 ++++--- lib/http/2/stream.rb | 14 ++++++++++++-- sig/2.rbs | 31 +++++++++++++++++++------------ sig/connection.rbs | 14 +++++++------- sig/stream.rbs | 5 ++--- spec/client_spec.rb | 11 +++++++---- spec/helper.rb | 4 +++- 9 files changed, 69 insertions(+), 37 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 97abdd0..8a517b4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,6 +17,8 @@ Layout/HeredocIndentation: - 'lib/tasks/generate_huffman_table.rb' - 'example/*' +Layout/LeadingCommentSpace: + AllowRBSInlineAnnotation: true Metrics/BlockLength: Enabled: false diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 40f9965..8f66bb6 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -212,9 +212,8 @@ def receive(data) end while (frame = @framer.parse(@recv_buffer)) - # @type var stream_id: Integer - stream_id = frame[:stream] - frame_type = frame[:type] + stream_id = frame[:stream] #: Integer + frame_type = frame[:type] #: Symbol? if is_a?(Client) && !@received_frame connection_error(:protocol_error, msg: "didn't receive settings") if frame_type != :settings @@ -271,11 +270,13 @@ def receive(data) # anything other than 0x0, the endpoint MUST respond with a connection # error (Section 5.4.1) of type PROTOCOL_ERROR. if connection_frame?(frame) + # @type var frame: connection_frame connection_error(:protocol_error) unless stream_id.zero? connection_management(frame) else case frame_type when :headers + # @type var frame: headers_frame # When server receives even-numbered stream identifier, # the endpoint MUST respond with a connection error of type PROTOCOL_ERROR. connection_error if stream_id.even? && is_a?(Server) @@ -312,6 +313,7 @@ def receive(data) stream << frame when :push_promise + # @type var frame: push_promise_frame # The last frame in a sequence of PUSH_PROMISE/CONTINUATION # frames MUST have the END_HEADERS flag set unless frame[:flags].anybits?(END_HEADERS) @@ -457,6 +459,7 @@ def send(frame) # @param frame [Hash] def encode(frame) if HEADERS_FRAME_TYPES.include?(frame[:type]) + #: @type var frame: headers_frame | push_promise_frame encode_headers(frame) # HEADERS and PUSH_PROMISE may create more than one frame else emit(:frame, @framer.generate(frame)) @@ -493,12 +496,15 @@ def connection_management(frame) when :connected case frame_type when :settings + # @type var frame: settings_frame connection_settings(frame) when :window_update + # @type var frame: window_update_frame process_window_update(frame: frame, encode: true) when :ping ping_management(frame) when :goaway + # @type var frame: goaway_frame # Receivers of a GOAWAY frame MUST NOT open additional streams on # the connection, although a new connection can be established # for new streams. @@ -506,12 +512,14 @@ def connection_management(frame) @closed_since = Process.clock_gettime(Process::CLOCK_MONOTONIC) emit(:goaway, frame[:last_stream], frame[:error], frame[:payload]) when :altsvc + # @type var frame: altsvc_frame origin = frame[:origin] # 4. The ALTSVC HTTP/2 Frame # An ALTSVC frame on stream 0 with empty (length 0) "Origin" # information is invalid and MUST be ignored. emit(:altsvc, frame) if origin && !origin.empty? when :origin + # @type var frame: origin_frame return if @h2c_upgrade || !frame[:flags].zero? frame[:payload].each do |orig| @@ -609,8 +617,8 @@ def connection_settings(frame) settings = @pending_settings.shift side = :local else - validate_settings(@remote_role, frame[:payload]) settings = frame[:payload] + validate_settings(@remote_role, settings) side = :remote end @@ -738,7 +746,7 @@ def encode_headers(headers_frame) type: :continuation, flags: 0, payload: payload.byteslice(0, max_frame_size) - ) + ) #: continuation_frame payload = payload.byteslice(max_frame_size..-1) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index 7565cc7..1758362 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -187,6 +187,7 @@ def generate(frame) case frame[:type] when :data, :continuation + # @type var frame: data_frame | continuation_frame bytes = frame[:payload] length = bytes.bytesize @@ -360,9 +361,9 @@ def parse(buf) frame = read_common_header(buf) - type = frame[:type] - length = frame[:length] - flags = frame[:flags] + type = frame[:type] #: Symbol + length = frame[:length] #: Integer + flags = frame[:flags] #: Integer return if buf.size < 9 + length diff --git a/lib/http/2/stream.rb b/lib/http/2/stream.rb index c64c286..4bcb035 100644 --- a/lib/http/2/stream.rb +++ b/lib/http/2/stream.rb @@ -114,8 +114,10 @@ def closed? def receive(frame) transition(frame, false) - case frame[:type] + frame_type = frame[:type] #: Symbol + case frame_type when :data + # @type var frame: data_frame # 6.1. DATA # If a DATA frame is received whose stream is not in "open" or # "half closed (local)" state, the recipient MUST respond with a @@ -129,6 +131,7 @@ def receive(frame) emit(:data, frame[:payload]) unless frame[:ignore] calculate_window_update(@local_window_max_size) when :headers + # @type var frame: headers_frame stream_error(:stream_closed) if (@state == :closed && @closed != :local_rst) || @state == :remote_closed @_method ||= frame[:method] @@ -151,17 +154,20 @@ def receive(frame) stream_error(:stream_closed) if (@state == :closed && @closed != :local_rst) || @state == :remote_closed stream_error(:protocol_error) if @received_data when :priority + # @type var frame: priority_frame process_priority(frame) when :window_update + # @type var frame: window_update_frame process_window_update(frame: frame) when :altsvc + # @type var frame: origin_frame # 4. The ALTSVC HTTP/2 Frame # An ALTSVC frame on a # stream other than stream 0 containing non-empty "Origin" information # is invalid and MUST be ignored. - emit(frame[:type], frame) if !frame[:origin] || frame[:origin].empty? when :blocked emit(frame[:type], frame) + emit(frame_type, frame) if !frame[:origin] || frame[:origin].empty? end complete_transition(frame) @@ -199,11 +205,14 @@ def calculate_content_length(data_length) def send(frame) case frame[:type] when :data + # @type var frame: data_frame # stream state management is maintained in send_data return send_data(frame) when :window_update + # @type var frame: window_update_frame @local_window += frame[:increment] when :priority + # @type var frame: priority_frame process_priority(frame) end @@ -662,6 +671,7 @@ def process_priority(frame) def end_stream?(frame) case frame[:type] when :data, :headers, :continuation + # @type var frame: data_frame | headers_frame | continuation_frame frame[:flags] && frame[:flags].anybits?(END_STREAM) else false end diff --git a/sig/2.rbs b/sig/2.rbs index f8fcd6e..9f866eb 100644 --- a/sig/2.rbs +++ b/sig/2.rbs @@ -49,40 +49,47 @@ module HTTP2 # # FRAMES type frame_control_flags = Integer - type common_frame = { stream: Integer } + type common_frame = { stream: Integer, flags: frame_control_flags, ?ignore: bool } + + type connection_frame = settings_frame | ping_frame | goaway_frame | window_update_frame | altsvc_frame | origin_frame # # HEADERS - type headers_frame = common_frame & { - type: :headers, flags: frame_control_flags, payload: Enumerable[header_pair] | String, + type headers_frame_props = { + type: :headers, payload: Enumerable[header_pair] | String, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, ?padding: Integer } + type headers_frame = common_frame & headers_frame_props + + type continuation_frame = headers_frame # # DATA - type data_frame = { type: :data, flags: frame_control_flags, ?length: Integer, payload: String, ?padding: Integer } + type data_frame = common_frame & { type: :data, ?length: Integer, payload: String, ?padding: Integer } # # PUSH_PROMISE - type push_promise_frame = { type: :push_promise, promise_stream: Integer, flags: frame_control_flags, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, payload: Enumerable[header_pair], ?padding: Integer } + type push_promise_frame_props = { type: :push_promise, promise_stream: Integer, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, payload: Enumerable[header_pair] | String, ?padding: Integer } + type push_promise_frame = common_frame & push_promise_frame_props # # SETTINGS - type settings_frame = { type: :settings, ?payload: Array[[Symbol | Integer, Integer]], ?flags: frame_control_flags } + type settings_frame = common_frame & { type: :settings, payload: Array[[Symbol, Integer]] } # # WINDOW_UPDATE - type window_update_frame = { type: :window_update, increment: Integer } + type window_update_frame = common_frame & { type: :window_update, increment: Integer } # # PRIORITY - type priority_frame = { dependency: Integer, exclusive: bool, weight: Integer } + type priority_frame_props = { dependency: Integer, exclusive: bool, weight: Integer } + type priority_frame = common_frame & priority_frame_props # # ALTSVC - type altsvc_frame = { type: :altsvc, max_age: Integer, port: Integer, proto: "String", host: String } + type altsvc_frame = common_frame & { type: :altsvc, max_age: Integer, port: Integer, proto: String, host: String } # # ORIGIN - type origin_frame = { type: :origin, origin: Array[String] } + type origin_frame = common_frame & { type: :origin, length: Integer, payload: Array[String] } # # PING - type ping_frame = { type: :ping, payload: String, length: Integer, ?flags: frame_control_flags } + type ping_frame = common_frame & { type: :ping, payload: String, length: Integer } # # GOAWAY - type goaway_frame = { type: :goaway, last_stream: Integer, error: Symbol? } + type goaway_frame = common_frame & { type: :goaway, last_stream: Integer, error: Symbol? } # type frame = common_frame & (headers_frame | data_frame | push_promise_frame | # settings_frame | window_update_frame | priority_frame | altsvc_frame | diff --git a/sig/connection.rbs b/sig/connection.rbs index 4589bc7..aee9fb1 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -68,7 +68,7 @@ module HTTP2 def settings: (settings_enum payload) -> void - def receive: (string data) -> void + def receive: (String data) -> void alias << receive def initialize: (?connection_opts) -> void @@ -81,25 +81,25 @@ module HTTP2 def connection_frame?: (frame) -> bool - def connection_management: (frame) -> void + def connection_management: (connection_frame frame) -> void def ping_management: (frame) -> void def validate_settings: (role_type, settings_enum) -> void - def connection_settings: (frame) -> void + def connection_settings: (settings_frame) -> void - def decode_headers: (frame) -> void + def decode_headers: (headers_frame | push_promise_frame) -> void - def encode_headers: (frame headers_frame) -> void + def encode_headers: (headers_frame | push_promise_frame) -> void def activate_stream: (id: Integer, **untyped) -> Stream def verify_stream_order: (Integer id) -> void - def verify_pseudo_headers: (frame) -> void + def verify_pseudo_headers: (headers_frame | push_promise_frame) -> void - def _verify_pseudo_headers: (frame, Array[String]) -> void + def _verify_pseudo_headers: (headers_frame | push_promise_frame, Array[String]) -> void def connection_error: (?Symbol error, ?msg: String?, ?e: StandardError?) -> void end diff --git a/sig/stream.rbs b/sig/stream.rbs index 3f1864c..b43f660 100644 --- a/sig/stream.rbs +++ b/sig/stream.rbs @@ -80,12 +80,11 @@ module HTTP2 def complete_transition: (frame) -> void - def process_priority: (priority_frame frame) -> void + def process_priority: (priority_frame_props frame) -> void def end_stream?: (frame frame) -> boolish - def stream_error: (Symbol error, ?msg: String?) -> void - | () -> void + def stream_error: (?Symbol error, ?msg: String?) -> void alias error stream_error diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 3021d9e..8c2aaa4 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -114,10 +114,13 @@ end.to raise_error(NoMethodError) end - it "should raise error on PUSH_PROMISE against stream 0" do - expect do - client << set_stream_id(f.generate(push_promise_frame), 0) - end.to raise_error(ProtocolError) + unless defined?(RBS) + # RBS detects type unsafety at runtime + it "should raise error on PUSH_PROMISE against stream 0" do + expect do + client << set_stream_id(f.generate(push_promise_frame), 0) + end.to raise_error(ProtocolError) + end end it "should raise error on PUSH_PROMISE against bogus stream" do diff --git a/spec/helper.rb b/spec/helper.rb index 488815d..657c9aa 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -117,7 +117,9 @@ def goaway_frame def window_update_frame { type: :window_update, - increment: 10 + increment: 10, + flags: 0, + stream: 0 } end From 914870d04b1211c8ae3fd1c9750ce52847bf0495 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 13:16:30 +0100 Subject: [PATCH 08/28] removing support for blocked frame type this seems like a leftover from a WIP version of the HTTP/2 RFC that never made it to the final spec --- lib/http/2/connection.rb | 2 -- lib/http/2/stream.rb | 2 -- 2 files changed, 4 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 8f66bb6..d22a5fc 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -525,8 +525,6 @@ def connection_management(frame) frame[:payload].each do |orig| emit(:origin, orig) end - when :blocked - emit(:blocked, frame) else connection_error end diff --git a/lib/http/2/stream.rb b/lib/http/2/stream.rb index 4bcb035..3324f7d 100644 --- a/lib/http/2/stream.rb +++ b/lib/http/2/stream.rb @@ -165,8 +165,6 @@ def receive(frame) # An ALTSVC frame on a # stream other than stream 0 containing non-empty "Origin" information # is invalid and MUST be ignored. - when :blocked - emit(frame[:type], frame) emit(frame_type, frame) if !frame[:origin] || frame[:origin].empty? end From e746110eacab72512b9b85d5600fff64fcf22d24 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 13:17:32 +0100 Subject: [PATCH 09/28] create settings array via map instead of new array then append pattern avoids the whole array resizing logic --- lib/http/2/framer.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index 1758362..bf826f1 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -422,19 +422,18 @@ def parse(buf) when :settings # NOTE: frame[:length] might not match the number of frame[:payload] # because unknown extensions are ignored. - frame[:payload] = [] raise ProtocolError, "Invalid settings payload length" unless (length % 6).zero? raise ProtocolError, "Invalid stream ID (#{frame[:stream]})" if frame[:stream].nonzero? - (frame[:length] / 6).times do + frame[:payload] = (frame[:length] / 6).times.filter_map do id = read_str(payload, 2).unpack1(UINT16) val = read_uint32(payload) # Unsupported or unrecognized settings MUST be ignored. # Here we send it along. if (name = DEFINED_SETTINGS_BY_ID[id]) - frame[:payload] << [name, val] + [name, val] end end when :push_promise From cea03b1aa20f1a2e270f2b93b87abd0a93157868 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 13:27:50 +0100 Subject: [PATCH 10/28] took encode_headers logic away from #encode, which does raw frame encoding now --- lib/http/2/connection.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index d22a5fc..6c8918a 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -447,9 +447,13 @@ def send(frame) # endpoint MAY choose to treat a stream error as a connection error. goaway(:protocol_error) - else + elsif HEADERS_FRAME_TYPES.include?(frame[:type]) # HEADERS and PUSH_PROMISE may generate CONTINUATION. Also send # RST_STREAM that are not protocol errors + #: @type var frame: headers_frame | push_promise_frame + encode_headers(frame) # HEADERS and PUSH_PROMISE may create more than one frame + else + #: @type var frame: connection_frame encode(frame) end end @@ -458,12 +462,7 @@ def send(frame) # # @param frame [Hash] def encode(frame) - if HEADERS_FRAME_TYPES.include?(frame[:type]) - #: @type var frame: headers_frame | push_promise_frame - encode_headers(frame) # HEADERS and PUSH_PROMISE may create more than one frame - else - emit(:frame, @framer.generate(frame)) - end + emit(:frame, @framer.generate(frame)) end # Check if frame is a connection frame: SETTINGS, PING, GOAWAY, and any @@ -727,7 +726,7 @@ def encode_headers(headers_frame) # if single frame, return immediately if payload.bytesize <= max_frame_size - emit(:frame, @framer.generate(headers_frame)) + encode(headers_frame) return end @@ -737,7 +736,7 @@ def encode_headers(headers_frame) payload = payload.byteslice(max_frame_size..-1) # emit first HEADERS frame - emit(:frame, @framer.generate(headers_frame)) + encode(headers_frame) loop do continuation_frame = headers_frame.merge( @@ -750,11 +749,11 @@ def encode_headers(headers_frame) if payload.nil? || payload.empty? continuation_frame[:flags] |= END_HEADERS - emit(:frame, @framer.generate(continuation_frame)) + encode(continuation_frame) break end - emit(:frame, @framer.generate(continuation_frame)) + encode(continuation_frame) end end From afb5a7f1f5261bc853acb998390be3ea76b2668c Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 16:45:01 +0100 Subject: [PATCH 11/28] avoid slicing string too much when generating continuation frames while ruby allows sharing the underlying buffer string when slicing to the end of the string, it still allocates the 40b extra ruby objects, and the code doesn't get more complicated through it, so why not --- lib/http/2/connection.rb | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 6c8918a..648ab70 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -715,13 +715,16 @@ def decode_headers(frame) # # @param headers_frame [Hash] def encode_headers(headers_frame) - payload = headers_frame[:payload] + headers_payload = headers_frame[:payload] begin - payload = headers_frame[:payload] = @compressor.encode(payload) unless payload.is_a?(String) + payload = headers_payload.is_a?(String) ? headers_payload : @compressor.encode(headers_payload) rescue StandardError => e connection_error(:compression_error, e: e) end + #: @type var payload: String + headers_frame[:payload] = payload + max_frame_size = @remote_settings[:settings_max_frame_size] # if single frame, return immediately @@ -731,29 +734,27 @@ def encode_headers(headers_frame) end # split into multiple CONTINUATION frames + total = payload.bytesize headers_frame[:flags] ^= END_HEADERS headers_frame[:payload] = payload.byteslice(0, max_frame_size) - payload = payload.byteslice(max_frame_size..-1) + # payload = payload.byteslice(max_frame_size..-1) + offset = max_frame_size # emit first HEADERS frame encode(headers_frame) - loop do + while offset < total + chunk_end = offset + max_frame_size + is_last = chunk_end >= total + continuation_frame = headers_frame.merge( type: :continuation, - flags: 0, - payload: payload.byteslice(0, max_frame_size) + flags: is_last ? END_HEADERS : 0, + payload: payload.byteslice(offset, max_frame_size) ) #: continuation_frame - payload = payload.byteslice(max_frame_size..-1) - - if payload.nil? || payload.empty? - continuation_frame[:flags] |= END_HEADERS - encode(continuation_frame) - break - end - encode(continuation_frame) + offset = chunk_end end end From e68000c9ae0873583f74d3e13b3d6d077d2a64ee Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 16:56:33 +0100 Subject: [PATCH 12/28] when processing continuation frames, bookkeep size of payloads to avoid O(n)'ing on every size check --- lib/http/2/connection.rb | 7 +++++-- sig/connection.rbs | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 648ab70..56216ea 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -102,6 +102,7 @@ def initialize(settings = {}) @recv_buffer = "".b @continuation = [] + @continuation_size = 0 @error = nil @h2c_upgrade = nil @@ -236,15 +237,16 @@ def receive(data) # Header blocks MUST be transmitted as a contiguous sequence of frames # with no interleaved frames of any other type, or from any other stream. unless @continuation.empty? + # @type var frame: continuation_frame connection_error unless frame_type == :continuation && stream_id == @continuation.first[:stream] @continuation << frame + @continuation_size += frame[:payload].bytesize unless frame[:flags].anybits?(END_HEADERS) - buffered_payload = @continuation.sum { |f| f[:payload].bytesize } # prevent HTTP/2 CONTINUATION FLOOD # same heuristic as the one from HAProxy: https://www.haproxy.com/blog/haproxy-is-resilient-to-the-http-2-continuation-flood # different mitigation (connection closed, instead of 400 response) - unless buffered_payload < @local_settings[:settings_max_frame_size] + unless @continuation_size < @local_settings[:settings_max_frame_size] connection_error(:protocol_error, msg: "too many continuations received") end @@ -258,6 +260,7 @@ def receive(data) frame_type = frame[:type] @continuation.clear + @continuation_size = 0 frame.delete(:length) frame[:payload] = payload diff --git a/sig/connection.rbs b/sig/connection.rbs index aee9fb1..7b92624 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -50,6 +50,7 @@ module HTTP2 @recv_buffer: String @continuation: Array[frame] + @continuation_size: Integer @h2c_upgrade: Symbol? @closed_since: Float? From 00110d3b654d6f40cfa3f43ed40a8cb69ce37f1f Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 18:34:04 +0100 Subject: [PATCH 13/28] improve :shorter and :always header string encoding by avoiding memmove operations the prior version of this would append the huffman string THEN add the length prefix via String#insert, which needs to resize the string in the middle thereby doing a memmove operation, which is bad. this avoids it by either appending the length prefix before the string when possible, pre-append a null byte then change it after generating the huffman string, and in the rare case where the length may require more than one byte, remove the appended byte and use the old strategy relying on String#insert, because we can't have nice things --- lib/http/2/header/compressor.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/http/2/header/compressor.rb b/lib/http/2/header/compressor.rb index 03e26b6..ba2f7f4 100644 --- a/lib/http/2/header/compressor.rb +++ b/lib/http/2/header/compressor.rb @@ -83,8 +83,9 @@ def string(str, buffer = "".b) huffman = Huffman.encode(str) if huffman.bytesize < str.bytesize huffman_offset = buffer.bytesize + integer(huffman.bytesize, 7, buffer: buffer) + buffer.setbyte(huffman_offset, buffer.getbyte(huffman_offset) | 0x80) append_str(buffer, huffman) - set_huffman_size(buffer, huffman_offset) else plain_string(str, buffer) end @@ -147,8 +148,17 @@ def encode(headers) # @return [String] binary string def huffman_string(str, buffer = "".b) huffman_offset = buffer.bytesize + buffer << "\x00".b Huffman.encode(str, buffer) - set_huffman_size(buffer, huffman_offset) + size = buffer.bytesize - huffman_offset - 1 + + if size < 127 + buffer.setbyte(huffman_offset, 0x80 | size) + else + buffer.slice!(huffman_offset, 1) + set_huffman_size(buffer, huffman_offset) + end + buffer end # @param str [String] From 53e124558a580effaef20b1cd35f8cb3a3d47978 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 18:46:25 +0100 Subject: [PATCH 14/28] pack priority frame data in one single #pack call --- lib/http/2/framer.rb | 23 +++++++++++++++-------- sig/framer.rbs | 2 ++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index bf826f1..6f06929 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -96,10 +96,12 @@ class Framer UINT16 = "n" UINT8 = "C" HEADERPACK = (UINT8 + UINT16 + UINT8 + UINT8 + UINT32).freeze + PRIORITYPACK = (UINT32 + UINT8).freeze + ALTSVCPACK = (UINT32 + UINT16).freeze FRAME_LENGTH_HISHIFT = 16 FRAME_LENGTH_LOMASK = 0xFFFF - private_constant :RBIT, :RBYTE, :EBIT, :HEADERPACK, :UINT32, :UINT16, :UINT8 + private_constant :RBIT, :RBYTE, :EBIT, :HEADERPACK, :PRIORITYPACK, :UINT32, :UINT16, :UINT8 # Initializes new framer object. # @@ -205,8 +207,10 @@ def generate(frame) if frame[:flags].anybits?(PRIORITY) length = 5 + headers.bytesize bytes = String.new("", encoding: Encoding::BINARY, capacity: length) - pack([(frame[:exclusive] ? EBIT : 0) | (frame[:dependency] & RBIT)], UINT32, buffer: bytes) - pack([frame[:weight] - 1], UINT8, buffer: bytes) + pack( + [(frame[:exclusive] ? EBIT : 0) | (frame[:dependency] & RBIT), frame[:weight] - 1], + PRIORITYPACK, buffer: bytes + ) append_str(bytes, headers) else length = headers.bytesize @@ -220,8 +224,10 @@ def generate(frame) length = 5 bytes = String.new("", encoding: Encoding::BINARY, capacity: length) - pack([(frame[:exclusive] ? EBIT : 0) | (frame[:dependency] & RBIT)], UINT32, buffer: bytes) - pack([frame[:weight] - 1], UINT8, buffer: bytes) + pack( + [(frame[:exclusive] ? EBIT : 0) | (frame[:dependency] & RBIT), frame[:weight] - 1], + PRIORITYPACK, buffer: bytes + ) when :rst_stream length = 4 @@ -281,7 +287,7 @@ def generate(frame) when :altsvc length = 6 bytes = String.new("", encoding: Encoding::BINARY, capacity: length) - pack([frame[:max_age], frame[:port]], UINT32 + UINT16, buffer: bytes) + pack([frame[:max_age], frame[:port]], ALTSVCPACK, buffer: bytes) if frame[:proto] raise CompressionError, "Proto too long" if frame[:proto].bytesize > 255 @@ -383,9 +389,10 @@ def parse(buf) padded = flags.anybits?(PADDED) if padded padlen = read_str(payload, 1).unpack1(UINT8) - frame[:padding] = padlen + 1 raise ProtocolError, "padding too long" if padlen > payload.bytesize + frame[:padding] = padlen + 1 + payload = payload.byteslice(0, payload.bytesize - padlen) if padlen > 0 frame[:length] -= frame[:padding] frame[:flags] ^= PADDED @@ -450,7 +457,7 @@ def parse(buf) frame[:increment] = read_uint32(payload) & RBIT when :altsvc - frame[:max_age], frame[:port] = read_str(payload, 6).unpack(UINT32 + UINT16) + frame[:max_age], frame[:port] = read_str(payload, 6).unpack(ALTSVCPACK) len = payload.byteslice(0, 1).ord payload = payload.byteslice(1..-1) diff --git a/sig/framer.rbs b/sig/framer.rbs index 4ff70c6..f760e69 100644 --- a/sig/framer.rbs +++ b/sig/framer.rbs @@ -31,6 +31,8 @@ module HTTP2 UINT16: String UINT8: String HEADERPACK: String + PRIORITYPACK: String + ALTSVCPACK: String FRAME_LENGTH_HISHIFT: Integer FRAME_LENGTH_LOMASK: Integer From 83dbf4a42bc8ed55b5db01b1f3ab639b420e83f2 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 22 May 2026 19:17:52 +0100 Subject: [PATCH 15/28] improve head encoding when dealing with dynamic header table by keeping a side hash which keeps the offset of matched entries, lookups don't require table scans anymore, which makes this more scalable across different dynamic table sizes (at the expense of an extra container where to store things) --- lib/http/2/header/encoding_context.rb | 24 ++++++++++++++++++------ sig/header/encoding_context.rbs | 4 ++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/http/2/header/encoding_context.rb b/lib/http/2/header/encoding_context.rb index d59ffac..0dc06c5 100644 --- a/lib/http/2/header/encoding_context.rb +++ b/lib/http/2/header/encoding_context.rb @@ -118,6 +118,8 @@ class EncodingContext # :index Symbol :all, :static, :never def initialize(options = {}) @table = [] + @table_by_field = Hash.new { |hs, k| hs[k] = [] } + @unshifts = 0 @options = DEFAULT_OPTIONS.merge(options) @limit = @options[:table_size] @_table_updated = false @@ -129,9 +131,13 @@ def initialize(options = {}) def dup other = EncodingContext.new(@options) t = @table + tbf = @table_by_field.transform_values(&:dup) + unshifts = @unshifts l = @limit other.instance_eval do @table = t.dup # shallow copy + @table_by_field = tbf + @unshifts = unshifts @limit = l end other @@ -213,6 +219,8 @@ def process(cmd) # add to table if type == :incremental && size_check?(name.bytesize + value.bytesize + 32) @table.unshift(emit) + @unshifts += 1 + @table_by_field[name].unshift([value, @unshifts]) @current_table_size += name.bytesize + value.bytesize + 32 @_table_updated = true end @@ -279,14 +287,14 @@ def addcmd(field, value) end if index_type == :all && !exact - @table.each_with_index do |(hfield, hvalue), i| - next unless field == hfield + field_entries = @table_by_field[field] + field_entries&.each do |hvalue, unshift_id| + abs_index = (@unshifts - unshift_id) + STATIC_TABLE_SIZE + name_only ||= abs_index if value == hvalue - exact = i + STATIC_TABLE_SIZE + exact = abs_index break - else - name_only ||= i + STATIC_TABLE_SIZE end end end @@ -317,9 +325,13 @@ def resize_table(cmdsize) return if @table.empty? while @current_table_size + cmdsize > @limit - name, value = @table.pop @current_table_size -= name.bytesize + value.bytesize + 32 + + field_arr = @table_by_field[name] + field_arr.pop + @table_by_field.delete(name) if field_arr.empty? + break if @table.empty? end diff --git a/sig/header/encoding_context.rbs b/sig/header/encoding_context.rbs index 1085d4b..481acc5 100644 --- a/sig/header/encoding_context.rbs +++ b/sig/header/encoding_context.rbs @@ -22,6 +22,10 @@ module HTTP2 attr_reader current_table_size: Integer + @table_by_field: Hash[string, Array[[string, Integer]]] + + @unshifts: Integer + @limit: Integer @_table_updated: bool From 94fedb80d4e0b319f98c2d0fd2beb967951bf040 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sun, 24 May 2026 22:53:39 +0100 Subject: [PATCH 16/28] replace String#ord with String#getbyte for single character byte no intermediate 1 char string --- lib/http/2/framer.rb | 8 ++++---- lib/http/2/header/compressor.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index 6f06929..48669a9 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -407,7 +407,7 @@ def parse(buf) e_sd = read_uint32(payload) frame[:dependency] = e_sd & RBIT frame[:exclusive] = e_sd.anybits?(EBIT) - weight = payload.byteslice(0, 1).ord + 1 + weight = payload.getbyte(0) + 1 frame[:weight] = weight payload = payload.byteslice(1..-1) end @@ -418,7 +418,7 @@ def parse(buf) e_sd = read_uint32(payload) frame[:dependency] = e_sd & RBIT frame[:exclusive] = e_sd.anybits?(EBIT) - weight = payload.byteslice(0, 1).ord + 1 + weight = payload.getbyte(0) + 1 frame[:weight] = weight payload = payload.byteslice(1..-1) when :rst_stream @@ -459,11 +459,11 @@ def parse(buf) when :altsvc frame[:max_age], frame[:port] = read_str(payload, 6).unpack(ALTSVCPACK) - len = payload.byteslice(0, 1).ord + len = payload.getbyte(0) payload = payload.byteslice(1..-1) frame[:proto] = read_str(payload, len) if len > 0 - len = payload.byteslice(0, 1).ord + len = payload.getbyte(0) payload = payload.byteslice(1..-1) frame[:host] = read_str(payload, len) if len > 0 diff --git a/lib/http/2/header/compressor.rb b/lib/http/2/header/compressor.rb index ba2f7f4..a3343aa 100644 --- a/lib/http/2/header/compressor.rb +++ b/lib/http/2/header/compressor.rb @@ -120,7 +120,7 @@ def header(h, buffer = "".b) end # set header representation pattern on first byte - fb = buffer[offset].ord | rep[:pattern] + fb = buffer.getbyte(offset) | rep[:pattern] buffer.setbyte(offset, fb) buffer @@ -175,7 +175,7 @@ def plain_string(str, plain = "".b) # @return [String] binary string def set_huffman_size(buffer, huffman_offset) integer(buffer.bytesize - huffman_offset, 7, buffer: buffer, offset: huffman_offset) - buffer.setbyte(huffman_offset, buffer[huffman_offset].ord | 0x80) + buffer.setbyte(huffman_offset, buffer.getbyte(huffman_offset) | 0x80) buffer end end From a7fa55fe77785ffab0a6677af7444c69d57aeb4f Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 09:38:07 +0100 Subject: [PATCH 17/28] refactor Connection#send as a plain case/when block --- lib/http/2/connection.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 56216ea..85a3821 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -41,8 +41,6 @@ module HTTP2 CONNECTION_FRAME_TYPES = %i[settings ping goaway].freeze - HEADERS_FRAME_TYPES = %i[headers push_promise].freeze - STREAM_OPEN_STATES = %i[open half_closed_local].freeze # Connection encapsulates all of the connection, stream, flow-control, @@ -439,23 +437,26 @@ def <<(data) # @note all frames are currently delivered in FIFO order. # @param frame [Hash] def send(frame) - frame_type = frame[:type] - emit(:frame_sent, frame) - if frame_type == :data - send_data(frame, true) - elsif frame_type == :rst_stream && frame[:error] == :protocol_error - # An endpoint can end a connection at any time. In particular, an - # endpoint MAY choose to treat a stream error as a connection error. + frame_type = frame[:type] #: Symbol - goaway(:protocol_error) - elsif HEADERS_FRAME_TYPES.include?(frame[:type]) + case frame_type + when :data + #: @type var frame: data_frame + send_data(frame, true) + when :headers, :push_promise # HEADERS and PUSH_PROMISE may generate CONTINUATION. Also send # RST_STREAM that are not protocol errors #: @type var frame: headers_frame | push_promise_frame encode_headers(frame) # HEADERS and PUSH_PROMISE may create more than one frame else + if frame_type == :rst_stream && frame[:error] == :protocol_error + # An endpoint can end a connection at any time. In particular, an + # endpoint MAY choose to treat a stream error as a connection error. + + goaway(:protocol_error) + end #: @type var frame: connection_frame encode(frame) end From e8c4e85c478dc04d6a7cafcf09ea8a3288aac600 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 09:38:51 +0100 Subject: [PATCH 18/28] remove needless #force_encoding call the buffer will be already force-encoded after the method is called --- lib/http/2/header/huffman.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/2/header/huffman.rb b/lib/http/2/header/huffman.rb index 5033ed2..f086a41 100644 --- a/lib/http/2/header/huffman.rb +++ b/lib/http/2/header/huffman.rb @@ -62,7 +62,7 @@ def decode(buf) # Check whether partial input is correctly filled raise CompressionError, "Huffman decode error (EOS invalid)" unless state <= MAX_FINAL_STATE - emit.force_encoding(Encoding::BINARY) + emit end # Huffman table as specified in From 1377f3d5ab7b80a31b233fce63a46d3e19719256 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 10:04:57 +0100 Subject: [PATCH 19/28] removing redundant cast to to_a --- lib/http/2/server.rb | 2 +- sig/2.rbs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/http/2/server.rb b/lib/http/2/server.rb index 8f00ecb..a39ff54 100644 --- a/lib/http/2/server.rb +++ b/lib/http/2/server.rb @@ -157,7 +157,7 @@ def promise(parent, headers, flags) flags: flags, stream: parent.id, promise_stream: promise.id, - payload: headers.to_a + payload: headers ) yield(promise) diff --git a/sig/2.rbs b/sig/2.rbs index 9f866eb..c95fe2c 100644 --- a/sig/2.rbs +++ b/sig/2.rbs @@ -91,9 +91,9 @@ module HTTP2 # # GOAWAY type goaway_frame = common_frame & { type: :goaway, last_stream: Integer, error: Symbol? } - # type frame = common_frame & (headers_frame | data_frame | push_promise_frame | + # type frame = headers_frame | data_frame | push_promise_frame | # settings_frame | window_update_frame | priority_frame | altsvc_frame | - # origin_frame | ping_frame | goaway_frame) + # origin_frame | ping_frame | goaway_frame type frame_key = :type | :flags | :stream | :padding | :ignore | # headers From 69e48abaca4b2f7578fbcea17cc39257e36fcf42 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 15:56:59 +0100 Subject: [PATCH 20/28] adjusted frame usage to make type checking more accurate these were identified while running rbs runtime tests against the record union type for frames, something which is still making steep block... but is nevertheless where this should land --- lib/http/2/client.rb | 2 +- lib/http/2/connection.rb | 2 +- lib/http/2/server.rb | 3 +- sig/2.rbs | 60 ++++++++++++++++++++++++++++++--------- sig/connection.rbs | 4 +-- sig/flow_buffer.rbs | 4 +-- sig/frame_buffer.rbs | 4 +-- sig/framer.rbs | 4 +-- sig/header/compressor.rbs | 1 + sig/stream.rbs | 12 ++++---- spec/client_spec.rb | 14 +++++---- spec/framer_spec.rb | 33 ++++++++++++--------- spec/helper.rb | 12 ++++++++ spec/stream_spec.rb | 44 ++++++++++++++-------------- 14 files changed, 128 insertions(+), 71 deletions(-) diff --git a/lib/http/2/client.rb b/lib/http/2/client.rb index 3dfe39b..6529359 100644 --- a/lib/http/2/client.rb +++ b/lib/http/2/client.rb @@ -69,7 +69,7 @@ def send_connection_preface end def self.settings_header(settings) - frame = Framer.new.generate(type: :settings, stream: 0, payload: settings) + frame = Framer.new.generate(type: :settings, stream: 0, flags: 0, payload: settings) Base64.urlsafe_encode64(frame[9..-1]) end diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 85a3821..446175f 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -155,7 +155,7 @@ def ping(payload, &blk) # @param error [Symbol] # @param payload [String] def goaway(error = :no_error, payload = nil) - send(type: :goaway, last_stream: @last_stream_id, + send(type: :goaway, stream: 0, last_stream: @last_stream_id, error: error, payload: payload) @state = :closed @closed_since = Process.clock_gettime(Process::CLOCK_MONOTONIC) diff --git a/lib/http/2/server.rb b/lib/http/2/server.rb index a39ff54..94be68a 100644 --- a/lib/http/2/server.rb +++ b/lib/http/2/server.rb @@ -85,7 +85,8 @@ def upgrade(settings, headers, body) length: buf.bytesize, type: :settings, stream: 0, - flags: 0 + flags: 0, + payload: EMPTY }, buffer: buf ) diff --git a/sig/2.rbs b/sig/2.rbs index c95fe2c..b1f3bde 100644 --- a/sig/2.rbs +++ b/sig/2.rbs @@ -49,7 +49,7 @@ module HTTP2 # # FRAMES type frame_control_flags = Integer - type common_frame = { stream: Integer, flags: frame_control_flags, ?ignore: bool } + type common_frame = { stream: Integer, flags: frame_control_flags, ?length: Integer, ?ignore: bool } type connection_frame = settings_frame | ping_frame | goaway_frame | window_update_frame | altsvc_frame | origin_frame @@ -60,40 +60,72 @@ module HTTP2 } type headers_frame = common_frame & headers_frame_props - type continuation_frame = headers_frame + type continuation_frame_props = headers_frame_props | { type: :continuation } + + type continuation_frame = common_frame & continuation_frame_props # # DATA - type data_frame = common_frame & { type: :data, ?length: Integer, payload: String, ?padding: Integer } + type data_frame_props = { type: :data, ?length: Integer, payload: String, ?padding: Integer } + + type data_frame = common_frame & data_frame_props # # PUSH_PROMISE - type push_promise_frame_props = { type: :push_promise, promise_stream: Integer, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, payload: Enumerable[header_pair] | String, ?padding: Integer } + type push_promise_frame_props = { promise_stream: Integer, ?length: Integer, ?method: Symbol, ?trailer: Array[String], ?content_length: Integer, payload: Enumerable[header_pair] | String, ?padding: Integer } + type push_promise_frame = common_frame & push_promise_frame_props # # SETTINGS - type settings_frame = common_frame & { type: :settings, payload: Array[[Symbol, Integer]] } + type settings_frame_props = { type: :settings, payload: Enumerable[[Symbol | Integer, Integer]] } + + type settings_frame = common_frame & settings_frame_props # # WINDOW_UPDATE - type window_update_frame = common_frame & { type: :window_update, increment: Integer } + type window_update_frame_props = { type: :window_update, increment: Integer } + + type window_update_frame = common_frame & window_update_frame_props # # PRIORITY - type priority_frame_props = { dependency: Integer, exclusive: bool, weight: Integer } + type priority_frame_inner_props = { dependency: Integer, exclusive: bool, weight: Integer } + + type priority_frame_props = { type: :priority } & priority_frame_inner_props + type priority_frame = common_frame & priority_frame_props # # ALTSVC - type altsvc_frame = common_frame & { type: :altsvc, max_age: Integer, port: Integer, proto: String, host: String } + type altsvc_frame_props = { type: :altsvc, max_age: Integer, port: Integer, proto: String, host: String } + + type altsvc_frame = common_frame & altsvc_frame_props # # ORIGIN - type origin_frame = common_frame & { type: :origin, length: Integer, payload: Array[String] } + type origin_frame_props = { type: :origin, payload: Array[String] } + + type origin_frame = common_frame & origin_frame_props # # PING - type ping_frame = common_frame & { type: :ping, payload: String, length: Integer } + type ping_frame_props = { type: :ping, payload: String } + + type ping_frame = common_frame & ping_frame_props # # GOAWAY - type goaway_frame = common_frame & { type: :goaway, last_stream: Integer, error: Symbol? } + type goaway_frame_props = { type: :goaway, last_stream: Integer, error: (Symbol | Integer)? } + + type goaway_frame = common_frame & goaway_frame_props + + # # RST_STREAM + type rst_stream_frame_props = { type: :rst_stream, error: (Symbol | Integer)? } - # type frame = headers_frame | data_frame | push_promise_frame | + type rst_stream_frame = common_frame & rst_stream_frame_props + + # type frame = headers_frame | continuation_frame | push_promise_frame | data_frame | # settings_frame | window_update_frame | priority_frame | altsvc_frame | - # origin_frame | ping_frame | goaway_frame + # origin_frame | ping_frame | goaway_frame | rst_stream_frame + + # type connection_frame_props = rst_stream_frame_props | ping_frame_props | goaway_frame_props + # | window_update_frame_props | settings_frame_props + + # type stream_frame_props = headers_frame_props | continuation_frame_props | push_promise_frame_props + # | data_frame_props | rst_stream_frame_props | priority_frame_props + # | window_update_frame_props | origin_frame_props | altsvc_frame_props type frame_key = :type | :flags | :stream | :padding | :ignore | # headers @@ -124,4 +156,6 @@ module HTTP2 nil type frame = Hash[frame_key, frame_value] + type connection_frame_props = Hash[frame_key, frame_value] + type stream_frame_props = Hash[frame_key, frame_value] end diff --git a/sig/connection.rbs b/sig/connection.rbs index 7b92624..1b2f0c7 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -76,7 +76,7 @@ module HTTP2 private - def send: (frame frame) -> void + def send: (connection_frame_props | stream_frame_props frame) -> void def encode: (frame frame) -> void @@ -104,4 +104,4 @@ module HTTP2 def connection_error: (?Symbol error, ?msg: String?, ?e: StandardError?) -> void end -end \ No newline at end of file +end diff --git a/sig/flow_buffer.rbs b/sig/flow_buffer.rbs index 1b82e16..6fd8bd6 100644 --- a/sig/flow_buffer.rbs +++ b/sig/flow_buffer.rbs @@ -14,9 +14,9 @@ module HTTP2 def calculate_window_update: (Integer) -> void - def send_data: (?data_frame? frame, ?bool encode) -> void + def send_data: (?data_frame_props? frame, ?bool encode) -> void - def send_frame: (data_frame frame, bool encode) -> void + def send_frame: (data_frame_props frame, bool encode) -> void def process_window_update: (frame: window_update_frame, ?encode: bool) -> void end diff --git a/sig/frame_buffer.rbs b/sig/frame_buffer.rbs index 57cb53f..f259fac 100644 --- a/sig/frame_buffer.rbs +++ b/sig/frame_buffer.rbs @@ -6,10 +6,10 @@ module HTTP2 def clear: () -> void - def <<: (data_frame frame) -> void + def <<: (data_frame_props frame) -> void def empty?: () -> bool - def retrieve: (Integer) -> data_frame? + def retrieve: (Integer) -> data_frame_props? end end \ No newline at end of file diff --git a/sig/framer.rbs b/sig/framer.rbs index f760e69..80e27e9 100644 --- a/sig/framer.rbs +++ b/sig/framer.rbs @@ -45,13 +45,13 @@ module HTTP2 def common_header: (frame, buffer: String) -> String - def read_common_header: (String buf) -> frame + def read_common_header: (String buf) -> (common_frame | { length: Integer }) def read_common_frame: (String) -> frame def generate: (frame) -> String - def parse: (String) -> frame? + def parse: (String) -> (frame | { length: Integer })? private diff --git a/sig/header/compressor.rbs b/sig/header/compressor.rbs index 2999c91..1b6b8ed 100644 --- a/sig/header/compressor.rbs +++ b/sig/header/compressor.rbs @@ -2,6 +2,7 @@ module HTTP2 module Header class Compressor include PackingExtensions + include BufferUtils @cc: EncodingContext diff --git a/sig/stream.rbs b/sig/stream.rbs index b43f660..447b4b3 100644 --- a/sig/stream.rbs +++ b/sig/stream.rbs @@ -37,7 +37,7 @@ module HTTP2 def calculate_content_length: (Integer?) -> void - def send: (frame frame) -> void + def send: (stream_frame_props frame) -> void def headers: (Enumerable[header_pair] headers, ?end_headers: bool, ?end_stream: bool) -> void @@ -70,7 +70,7 @@ module HTTP2 ?state: Symbol ) -> untyped - def transition: (frame, bool sending) -> void + def transition: (stream_frame_props frame, bool sending) -> void def event: (Symbol newstate) -> void @@ -80,14 +80,14 @@ module HTTP2 def complete_transition: (frame) -> void - def process_priority: (priority_frame_props frame) -> void + def process_priority: (priority_frame_inner_props frame) -> void - def end_stream?: (frame frame) -> boolish + def end_stream?: (stream_frame_props frame) -> boolish def stream_error: (?Symbol error, ?msg: String?) -> void alias error stream_error - def manage_state: (frame) { () -> void } -> void + def manage_state: (stream_frame_props frame) { () -> void } -> void end -end \ No newline at end of file +end diff --git a/spec/client_spec.rb b/spec/client_spec.rb index 8c2aaa4..1d40f11 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -268,13 +268,15 @@ end end context "received in a stream" do - it "should be ignored" do - s = client.new_stream - s.send headers_frame + unless defined?(RBS) + it "should be ignored" do + s = client.new_stream + s.send headers_frame - expect do - client << set_stream_id(f.generate(orig_frame), s.id) - end.not_to raise_error + expect do + client << set_stream_id(f.generate(orig_frame), s.id) + end.not_to raise_error + end end end end diff --git a/spec/framer_spec.rb b/spec/framer_spec.rb index b62aa6c..9c7cf87 100644 --- a/spec/framer_spec.rb +++ b/spec/framer_spec.rb @@ -6,7 +6,7 @@ let(:f) { Framer.new } context "common header" do - let(:frame) do + let(:frame_headers) do { length: 4, type: :headers, @@ -14,6 +14,7 @@ stream: 15 } end + let(:frame) { frame_headers.merge(payload: "data") } let(:bytes) { [0, 0x04, 0x01, 0x5, 0x0000000F].pack("CnCCN") } @@ -22,7 +23,7 @@ end it "should parse common 9 byte header" do - expect(f.read_common_header(bytes)).to eq frame + expect(f.read_common_header(bytes)).to eq frame_headers end it "should generate a large frame" do @@ -35,15 +36,17 @@ stream: 15 } bytes = [5, 17, 0x01, 0x5, 0x0000000F].pack("CnCCN") - expect(f.common_header(frame, buffer: "".b)).to eq bytes + expect(f.common_header(frame.merge(payload: "data"), buffer: "".b)).to eq bytes expect(f.read_common_header(bytes)).to eq frame end - it "should raise exception on invalid frame type when sending" do - expect do - frame[:type] = :bogus - f.common_header(frame, buffer: "".b) - end.to raise_error(CompressionError, /invalid.*type/i) + unless defined?(RBS) + it "should raise exception on invalid frame type when sending" do + expect do + frame[:type] = :bogus + f.common_header(frame, buffer: "".b) + end.to raise_error(CompressionError, /invalid.*type/i) + end end it "should raise exception on invalid stream ID" do @@ -312,7 +315,8 @@ frame = { length: 4, type: :window_update, - increment: 10 + increment: 10, + stream: 0 } bytes = f.generate(frame) @@ -327,7 +331,8 @@ frame = { length: 4, type: :window_update, - increment: 0x7fffffff + 1 + increment: 0x7fffffff + 1, + stream: 0 } expect { f.generate(frame) }.to raise_error(CompressionError) @@ -447,10 +452,10 @@ [{ type: :headers, stream: 1, payload: "abc" }, 3], [{ type: :priority, stream: 3, dependency: 30, exclusive: false, weight: 1 }, 5], [{ type: :rst_stream, stream: 3, error: 100 }, 4], - [{ type: :settings, payload: [[:settings_max_concurrent_streams, 10]] }, 6], - [{ type: :push_promise, promise_stream: 5, payload: "abc" }, 7], - [{ type: :ping, payload: "blob" * 2 }, 8], - [{ type: :goaway, last_stream: 5, error: 20, payload: "blob" }, 12], + [{ type: :settings, stream: 0, payload: [[:settings_max_concurrent_streams, 10]] }, 6], + [{ type: :push_promise, stream: 1, promise_stream: 5, payload: "abc" }, 7], + [{ type: :ping, stream: 0, payload: "blob" * 2 }, 8], + [{ type: :goaway, stream: 0, last_stream: 5, error: 20, payload: "blob" }, 12], [{ type: :window_update, stream: 1, increment: 1024 }, 4], [{ type: :continuation, stream: 1, payload: "abc" }, 3] ] diff --git a/spec/helper.rb b/spec/helper.rb index 657c9aa..60e53d6 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -53,6 +53,7 @@ def priority_frame { type: :priority, stream: 1, + flags: 0, exclusive: false, dependency: 0, weight: 20 @@ -63,6 +64,7 @@ def rst_stream_frame { type: :rst_stream, stream: 1, + flags: 0, error: :stream_closed } end @@ -71,6 +73,7 @@ def settings_frame { type: :settings, stream: 0, + flags: 0, payload: [ [:settings_max_concurrent_streams, 10], [:settings_initial_window_size, 0x7fffffff] @@ -108,6 +111,7 @@ def pong_frame def goaway_frame { type: :goaway, + stream: 0, last_stream: 2, error: :no_error, payload: "debug" @@ -134,6 +138,7 @@ def continuation_frame def altsvc_frame { + stream: 0, type: :altsvc, max_age: 1_402_290_402, # 4 port: 8080, # 2 reserved 1 @@ -146,6 +151,7 @@ def altsvc_frame def origin_frame { type: :origin, + stream: 0, payload: %w[https://www.example.com https://www.example.org] } end @@ -162,6 +168,12 @@ def frame_types methods.select { |meth| meth.to_s.end_with?("_frame") } .map { |meth| __send__(meth) } end + + def stream_frame_types + %i[data headers priority rst_stream push_promise window_update continuation].map do |type| + __send__(:"#{type}_frame") + end + end end def set_stream_id(bytes, id) diff --git a/spec/stream_spec.rb b/spec/stream_spec.rb index d8938c1..cfccf8c 100644 --- a/spec/stream_spec.rb +++ b/spec/stream_spec.rb @@ -58,13 +58,13 @@ end it "should raise error if sending invalid frames" do - frame_types.reject { |frame| %i[headers rst_stream priority].include?(frame[:type]) }.each do |type| + stream_frame_types.reject { |frame| %i[headers rst_stream priority].include?(frame[:type]) }.each do |type| expect { stream.dup.send type }.to raise_error InternalError end end it "should raise error on receipt of invalid frames" do - what_types = frame_types.reject { |frame| %i[priority window_update rst_stream].include?(frame[:type]) } + what_types = stream_frame_types.reject { |frame| %i[priority window_update rst_stream].include?(frame[:type]) } what_types.each do |type| expect { stream.dup.receive type }.to raise_error InternalError end @@ -109,13 +109,13 @@ end it "should raise error if sending invalid frames" do - frame_types.reject { |frame| %i[priority rst_stream window_update].include?(frame[:type]) }.each do |type| + stream_frame_types.reject { |frame| %i[priority rst_stream window_update].include?(frame[:type]) }.each do |type| expect { stream.dup.send type }.to raise_error InternalError end end it "should raise error on receipt of invalid frames" do - frame_types.reject { |frame| %i[headers rst_stream priority].include?(frame[:type]) }.each do |type| + stream_frame_types.reject { |frame| %i[headers rst_stream priority].include?(frame[:type]) }.each do |type| expect { stream.dup.receive type }.to raise_error InternalError end end @@ -150,13 +150,13 @@ before { stream.receive headers_frame } it "should allow any valid frames types to be sent" do - (frame_types - [ping_frame, goaway_frame, settings_frame]).each do |type| + stream_frame_types.each do |type| expect { stream.dup.send type }.to_not raise_error end end it "should allow frames of any type to be received" do - frame_types.each do |type| + stream_frame_types.each do |type| expect { stream.dup.receive type }.to_not raise_error end end @@ -297,7 +297,7 @@ before { stream.send headers_frame.merge(flags: END_HEADERS | END_STREAM) } it "should raise error on attempt to send invalid frames" do - frame_types.reject { |frame| %i[priority rst_stream window_update].include?(frame[:type]) }.each do |frame| + stream_frame_types.reject { |frame| %i[priority rst_stream window_update].include?(frame[:type]) }.each do |frame| expect { stream.dup.send frame }.to raise_error InternalError end end @@ -375,7 +375,7 @@ before { stream.receive headers_frame.merge(flags: END_HEADERS | END_STREAM) } it "should raise STREAM_CLOSED error on reciept of frames" do - (frame_types - [priority_frame, rst_stream_frame, window_update_frame]).each do |frame| + (stream_frame_types - [priority_frame, rst_stream_frame, window_update_frame]).each do |frame| expect do stream.dup.receive frame end.to raise_error(StreamClosed) @@ -414,7 +414,7 @@ stream.send data.merge(payload: "x" * 16_384) end client << f.generate(settings_frame) - client << Framer.new.generate(type: :window_update, stream: stream.id, increment: 16_384) + client << Framer.new.generate(type: :window_update, stream: stream.id, increment: 16_384, flags: 0) end it "should transition to closed if RST_STREAM is sent" do @@ -477,7 +477,7 @@ end it "should raise STREAM_CLOSED on attempt to send frames" do - (frame_types - [priority_frame, rst_stream_frame]).each do |frame| + (stream_frame_types - [priority_frame, rst_stream_frame]).each do |frame| expect do stream.dup.send frame end.to raise_error(StreamClosed) @@ -485,7 +485,7 @@ end it "should raise STREAM_CLOSED on receipt of frame" do - (frame_types - [priority_frame, rst_stream_frame, window_update_frame]).each do |frame| + (stream_frame_types - [priority_frame, rst_stream_frame, window_update_frame]).each do |frame| expect do stream.dup.receive frame end.to raise_error(StreamClosed) @@ -523,15 +523,17 @@ stream.send rst_stream_frame # closed by local end - it "should ignore received frames" do - control_frames.each do |frame| - expect do - cb = [] - stream.on(:data) { cb << :data } - stream.on(:headers) { cb << :headers } - stream.dup.receive frame - expect(cb).to be_empty - end.to_not raise_error + unless defined?(RBS) + it "should ignore received frames" do + control_frames.each do |frame| + expect do + cb = [] + stream.on(:data) { cb << :data } + stream.on(:headers) { cb << :headers } + stream.dup.receive frame + expect(cb).to be_empty + end.to_not raise_error + end end end @@ -571,7 +573,7 @@ stream.send headers_frame # go to open expect(stream.remote_window).to eq DEFAULT_FLOW_WINDOW - (frame_types - [data_frame, ping_frame, goaway_frame, settings_frame]).each do |frame| + (frame_types - [data_frame, ping_frame, pong_frame, goaway_frame, settings_frame]).each do |frame| s = stream.dup s.send frame expect(s.remote_window).to eq DEFAULT_FLOW_WINDOW From 44556e13f82de6e2bd5d053a7e6787d8fd9a3c93 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 17:19:17 +0100 Subject: [PATCH 21/28] simplify h2c code by allowing frame to generate settings frame from an already encoded settings list --- lib/http/2/framer.rb | 29 ++++++++++++++++++----------- lib/http/2/server.rb | 19 ++++++------------- sig/2.rbs | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/http/2/framer.rb b/lib/http/2/framer.rb index 48669a9..3643810 100644 --- a/lib/http/2/framer.rb +++ b/lib/http/2/framer.rb @@ -240,20 +240,27 @@ def generate(frame) end settings = frame[:payload] - bytes = String.new("", encoding: Encoding::BINARY, capacity: settings.size * 6) - settings.each do |(k, v)| - if k.is_a? Integer # rubocop:disable Style/GuardClause - DEFINED_SETTINGS.value?(k) || next - else - k = DEFINED_SETTINGS[k] + case settings + when String + length = settings.bytesize + bytes = settings + else + bytes = String.new("", encoding: Encoding::BINARY, capacity: settings.size * 6) - raise CompressionError, "Unknown settings ID for #{k}" if k.nil? - end + settings.each do |(k, v)| + if k.is_a? Integer # rubocop:disable Style/GuardClause + DEFINED_SETTINGS.value?(k) || next + else + k = DEFINED_SETTINGS[k] - pack([k], UINT16, buffer: bytes) - pack([v], UINT32, buffer: bytes) - length += 6 + raise CompressionError, "Unknown settings ID for #{k}" if k.nil? + end + + pack([k], UINT16, buffer: bytes) + pack([v], UINT32, buffer: bytes) + length += 6 + end end when :push_promise diff --git a/lib/http/2/server.rb b/lib/http/2/server.rb index 94be68a..7761fe7 100644 --- a/lib/http/2/server.rb +++ b/lib/http/2/server.rb @@ -78,19 +78,12 @@ def upgrade(settings, headers, body) receive(CONNECTION_PREFACE_MAGIC) # Process received HTTP2-Settings payload - buf = "".b - append_str(buf, Base64.urlsafe_decode64(settings.to_s)) - @framer.common_header( - { - length: buf.bytesize, - type: :settings, - stream: 0, - flags: 0, - payload: EMPTY - }, - buffer: buf - ) - receive(buf) + receive(@framer.generate( + type: :settings, + stream: 0, + flags: 0, + payload: Base64.urlsafe_decode64(settings.to_s) + )) # Activate stream (id: 1) with on HTTP/1.1 request parameters stream = activate_stream(id: 1) diff --git a/sig/2.rbs b/sig/2.rbs index b1f3bde..81fac9b 100644 --- a/sig/2.rbs +++ b/sig/2.rbs @@ -75,7 +75,7 @@ module HTTP2 type push_promise_frame = common_frame & push_promise_frame_props # # SETTINGS - type settings_frame_props = { type: :settings, payload: Enumerable[[Symbol | Integer, Integer]] } + type settings_frame_props = { type: :settings, payload: (Enumerable[[Symbol | Integer, Integer]] | String) } type settings_frame = common_frame & settings_frame_props From 1d8745bfe1bd2949a41922bb8141fc2beede3bf4 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 17:27:06 +0100 Subject: [PATCH 22/28] replace #append_str usage for chr with String#<< which support appending byte numbers --- lib/http/2/header/huffman.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/2/header/huffman.rb b/lib/http/2/header/huffman.rb index f086a41..5e88602 100644 --- a/lib/http/2/header/huffman.rb +++ b/lib/http/2/header/huffman.rb @@ -56,7 +56,7 @@ def decode(buf) first, state = MACHINE.dig(state, branch) raise CompressionError, "Huffman decode error (EOS found)" if first == EOS - append_str(emit, first.chr) if first + emit << first if first end end # Check whether partial input is correctly filled From 04d110e38bb819df59458b05b3a73c73f32fe931 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 17:37:15 +0100 Subject: [PATCH 23/28] create continuation frame hash instead of merging with headers the resulting hash from merge was bigger than it needed to be --- lib/http/2/connection.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 446175f..0c3cd6b 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -747,15 +747,18 @@ def encode_headers(headers_frame) # emit first HEADERS frame encode(headers_frame) + stream_id = headers_frame[:stream] + while offset < total chunk_end = offset + max_frame_size is_last = chunk_end >= total - continuation_frame = headers_frame.merge( + continuation_frame = { type: :continuation, flags: is_last ? END_HEADERS : 0, + stream: stream_id, payload: payload.byteslice(offset, max_frame_size) - ) #: continuation_frame + } #: continuation_frame encode(continuation_frame) offset = chunk_end From fac31e70a0996334b849a536bc785a555e820095 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 18:14:50 +0100 Subject: [PATCH 24/28] use Hash#delete_if instead of Hash#delete_while for streams-recently-closed management with a twist though; instead of traversing the whole hash, break the loop as early as possible, thereby having the same algo advantage of Hash#delete_while while not having to tear down the list each time --- lib/http/2/connection.rb | 20 +++++++++++++------- sig/connection.rbs | 1 + 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/http/2/connection.rb b/lib/http/2/connection.rb index 0c3cd6b..6215b27 100644 --- a/lib/http/2/connection.rb +++ b/lib/http/2/connection.rb @@ -89,6 +89,7 @@ def initialize(settings = {}) @last_stream_id = 0 @streams = {} @streams_recently_closed = {} + @oldest_stream_recently_closed = nil @pending_settings = [] @framer = Framer.new(@local_settings[:settings_max_frame_size]) @@ -788,19 +789,24 @@ def activate_stream(id:, **args) # is closed, with a minimum of 15s RTT time window. now = Process.clock_gettime(Process::CLOCK_MONOTONIC) - _, closed_since = @streams_recently_closed.first - # forego recently closed recycling if empty or the first element # hasn't expired yet (it's ordered). - if closed_since && (now - closed_since) > 15 + if @oldest_stream_recently_closed && (now - @oldest_stream_recently_closed) > 15 + new_oldest = nil # discards all streams which have closed for a while. # TODO: use a drop_while! variant whenever there is one. - @streams_recently_closed = @streams_recently_closed.drop_while do |_, since| - (now - since) > 15 - end.to_h + @streams_recently_closed.delete_if do |_, since| + unless (now - since) > 15 + new_oldest ||= since + break + end + + true + end + @oldest_stream_recently_closed = new_oldest end - @streams_recently_closed[id] = Process.clock_gettime(Process::CLOCK_MONOTONIC) + @streams_recently_closed[id] = now end stream.on(:frame, &method(:send)) diff --git a/sig/connection.rbs b/sig/connection.rbs index 1b2f0c7..f87e139 100644 --- a/sig/connection.rbs +++ b/sig/connection.rbs @@ -33,6 +33,7 @@ module HTTP2 @streams: Hash[Integer, Stream] @streams_recently_closed: Hash[Integer, Numeric] + @oldest_stream_recently_closed: Numeric @framer: Framer From ab55a71dbf1d7069af144f96295ae1e0209a1b46 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Mon, 25 May 2026 18:41:20 +0100 Subject: [PATCH 25/28] frame buffer: forego allocation of intermediate string while parittioning max size chunks this uses #read_str helper, which will use String#bytesplice when available --- lib/http/2/flow_buffer.rb | 8 ++++---- sig/frame_buffer.rbs | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/http/2/flow_buffer.rb b/lib/http/2/flow_buffer.rb index 0989028..c687292 100644 --- a/lib/http/2/flow_buffer.rb +++ b/lib/http/2/flow_buffer.rb @@ -115,6 +115,8 @@ def process_window_update(frame:, encode: false) end class FrameBuffer + include BufferUtils + attr_reader :bytesize def initialize @@ -148,15 +150,13 @@ def retrieve(window_size) return if window_size <= 0 && !(frame_size.zero? && end_stream) if frame_size > window_size - chunk = frame.dup - payload = frame[:payload] + chunk = frame.dup # Split frame so that it fits in the window # TODO: consider padding! - chunk[:payload] = payload.byteslice(0, window_size) + chunk[:payload] = read_str(frame[:payload], window_size) # mutates frame[:payload] chunk[:length] = window_size - frame[:payload] = payload.byteslice(window_size..-1) frame[:length] = frame_size - window_size # if no longer last frame in sequence... diff --git a/sig/frame_buffer.rbs b/sig/frame_buffer.rbs index f259fac..ad15969 100644 --- a/sig/frame_buffer.rbs +++ b/sig/frame_buffer.rbs @@ -1,5 +1,7 @@ module HTTP2 class FrameBuffer + include BufferUtils + attr_reader bytesize: Integer @buffer: Array[data_frame] From 2bc6cbe958e1b11e1d0dcde953806b6f095a54b4 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Tue, 26 May 2026 09:22:34 +0100 Subject: [PATCH 26/28] make sure a string is returned. --- lib/http/2/header/compressor.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/http/2/header/compressor.rb b/lib/http/2/header/compressor.rb index a3343aa..8a6c280 100644 --- a/lib/http/2/header/compressor.rb +++ b/lib/http/2/header/compressor.rb @@ -86,6 +86,7 @@ def string(str, buffer = "".b) integer(huffman.bytesize, 7, buffer: buffer) buffer.setbyte(huffman_offset, buffer.getbyte(huffman_offset) | 0x80) append_str(buffer, huffman) + buffer else plain_string(str, buffer) end From 518582a6ed603f856c3fc3c82735ba8472eb4a5e Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Tue, 26 May 2026 09:24:11 +0100 Subject: [PATCH 27/28] turn off typecheck when running h2spec some of the functions are raising type errors instead of the error which would close the connection or stream. --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index d8cb632..79ff5ad 100644 --- a/Rakefile +++ b/Rakefile @@ -94,7 +94,7 @@ task :h2spec do "Or Download the binary from https://github.com/summerwind/h2spec/releases" end - server_pid = Process.spawn("ruby example/server.rb -p 9000", out: File::NULL) + server_pid = Process.spawn({ "RUBYOPT" => nil }, "ruby example/server.rb -p 9000", out: File::NULL) sleep RUBY_ENGINE == "ruby" ? 5 : 20 system("#{h2spec} -p 9000 -o 2 --strict") Process.kill("TERM", server_pid) From f6d7d094183e5c3bd433d8a3c972381125085e1c Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Tue, 26 May 2026 09:35:22 +0100 Subject: [PATCH 28/28] only post simplecov results if working from the main repo --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 750eae8..61c7db9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,5 +78,6 @@ jobs: bundle exec rake coverage:report - uses: joshmfrankel/simplecov-check-action@main + if: ${{ github.repository == 'igrigorik/http-2' }} with: github_token: ${{ secrets.GITHUB_TOKEN }}