diff --git a/lib/protocol/http2/connection.rb b/lib/protocol/http2/connection.rb index 1128908..01fe31c 100644 --- a/lib/protocol/http2/connection.rb +++ b/lib/protocol/http2/connection.rb @@ -237,6 +237,15 @@ def receive_goaway(frame) self.close! + # Streams above the last stream ID were not processed by the remote peer and are safe to retry (RFC 9113 ยง6.8). + error = ::Protocol::HTTP::RequestRefusedError.new("GOAWAY: request not processed.") + + @streams.each_value do |stream| + if stream.id > @remote_stream_id + stream.close(error) + end + end + if error_code != 0 # Shut down immediately. raise GoawayError.new(message, error_code) diff --git a/protocol-http2.gemspec b/protocol-http2.gemspec index d8f084e..7d730b0 100644 --- a/protocol-http2.gemspec +++ b/protocol-http2.gemspec @@ -25,5 +25,5 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.3" spec.add_dependency "protocol-hpack", "~> 1.4" - spec.add_dependency "protocol-http", "~> 0.47" + spec.add_dependency "protocol-http", "~> 0.61" end diff --git a/releases.md b/releases.md index dfcf0b3..f5dff11 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - On GOAWAY, proactively close unprocessed streams (ID above `last_stream_id`) with `Protocol::HTTP::RequestRefusedError`, enabling safe retry of non-idempotent requests. + ## v0.24.0 - When closing a connection with active streams, if an error is not provided, it will default to `EOFError` so that streams propagate the closure correctly. diff --git a/test/protocol/http2/connection.rb b/test/protocol/http2/connection.rb index cf8cf34..3541452 100644 --- a/test/protocol/http2/connection.rb +++ b/test/protocol/http2/connection.rb @@ -391,6 +391,79 @@ def before expect(stream.state).to be == :closed end + let(:stream_class) do + Class.new(Protocol::HTTP2::Stream) do + attr_reader :error + + def closed(error) + @error = error + super + end + end + end + + it "closes unprocessed streams with RequestRefusedError on graceful GOAWAY" do + stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM) + + # Establish request stream on server: + server.read_frame + + another_stream = client.create_stream do |connection, id| + stream_class.create(connection, id) + end + another_stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM) + + # Server sends GOAWAY with last_stream_id=1, meaning stream 3 was not processed: + server.send_goaway(0) + + client.read_frame + + # The unprocessed stream (id=3) should be closed with RequestRefusedError: + expect(another_stream.state).to be == :closed + expect(another_stream.error).to be_a(Protocol::HTTP::RequestRefusedError) + + # The processed stream (id=1) should still be open: + expect(stream.state).not.to be == :closed + end + + it "closes all streams with RequestRefusedError on GOAWAY with last_stream_id=0" do + another_stream = client.create_stream do |connection, id| + stream_class.create(connection, id) + end + another_stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM) + + # Server sends GOAWAY with last_stream_id=0, meaning no streams were processed: + server.send_goaway(0) + + client.read_frame + + expect(another_stream.state).to be == :closed + expect(another_stream.error).to be_a(Protocol::HTTP::RequestRefusedError) + end + + it "closes unprocessed streams with RequestRefusedError on non-graceful GOAWAY" do + stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM) + + # Establish request stream on server: + server.read_frame + + another_stream = client.create_stream do |connection, id| + stream_class.create(connection, id) + end + another_stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM) + + # Server sends non-graceful GOAWAY with last_stream_id=1: + server.send_goaway(1, "Shutting down") + + expect do + client.read_frame + end.to raise_exception(Protocol::HTTP2::GoawayError) + + # The unprocessed stream should still have been closed with RequestRefusedError: + expect(another_stream.state).to be == :closed + expect(another_stream.error).to be_a(Protocol::HTTP::RequestRefusedError) + end + it "client can handle non-graceful shutdown" do stream.send_headers(request_headers, Protocol::HTTP2::END_STREAM)