From 6a9074ee9048cec726976b0b498c4433e4795a40 Mon Sep 17 00:00:00 2001 From: Anton Zhuravsky Date: Wed, 13 Mar 2024 12:24:23 +0000 Subject: [PATCH 1/4] Handling partially and fully hijacked connections gracefully --- lib/async/http/protocol/http1/connection.rb | 10 ++++++++ lib/async/http/protocol/http1/server.rb | 5 +--- test/async/http/protocol/http11.rb | 26 ++++++++++++++++++++- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/lib/async/http/protocol/http1/connection.rb b/lib/async/http/protocol/http1/connection.rb index 1275c6a9..753e04e7 100755 --- a/lib/async/http/protocol/http1/connection.rb +++ b/lib/async/http/protocol/http1/connection.rb @@ -18,6 +18,7 @@ def initialize(stream, version) @ready = true @version = version + @hijacked = false end attr :version @@ -47,6 +48,15 @@ def peer def concurrency 1 end + + def hijack! + @hijacked = true + super + end + + def hijacked? + @hijacked + end # Can we use this connection to make requests? def viable? diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb index 901d25c3..c9d60751 100644 --- a/lib/async/http/protocol/http1/server.rb +++ b/lib/async/http/protocol/http1/server.rb @@ -50,10 +50,7 @@ def each(task: Task.current) response = yield(request, self) body = response&.body - if @stream.nil? and body.nil? - # Full hijack. - return - end + return if hijacked? task.defer_stop do # If a response was generated, send it: diff --git a/test/async/http/protocol/http11.rb b/test/async/http/protocol/http11.rb index 67448c2a..31ec6da3 100755 --- a/test/async/http/protocol/http11.rb +++ b/test/async/http/protocol/http11.rb @@ -65,7 +65,7 @@ def around "Hello World!" ) peer.close - + nil end end @@ -82,5 +82,29 @@ def around expect(response.reason).to be == "It worked!" end end + + with 'empty response after hijack' do + let(:app) do + Protocol::HTTP::Middleware.for do |request| + peer = request.hijack! + + peer.write( + "#{request.version} 200 It worked!\r\n" + + "connection: close\r\n" + + "\r\n" + + "Hello World!" + ) + peer.close + + ::Protocol::HTTP::Response[-1, {}, []] + end + end + + it "reads raw response" do + response = client.get("/") + + expect(response.read).to be == "Hello World!" + end + end end end From dcfd5942c1a3385912e12fc6888a6e0ed27db817 Mon Sep 17 00:00:00 2001 From: Anton Zhuravsky Date: Wed, 13 Mar 2024 12:58:06 +0000 Subject: [PATCH 2/4] A more precise test of hijacked body --- test/async/http/protocol/http11.rb | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/async/http/protocol/http11.rb b/test/async/http/protocol/http11.rb index 31ec6da3..b452f312 100755 --- a/test/async/http/protocol/http11.rb +++ b/test/async/http/protocol/http11.rb @@ -83,24 +83,28 @@ def around end end - with 'empty response after hijack' do + with 'full hijack with empty response' do + let(:body) {Async::HTTP::Body::Buffered.new([], 0)} + let(:app) do - Protocol::HTTP::Middleware.for do |request| + ::Protocol::HTTP::Middleware.for do |request| peer = request.hijack! - peer.write( - "#{request.version} 200 It worked!\r\n" + - "connection: close\r\n" + - "\r\n" + - "Hello World!" - ) - peer.close + peer.write( + "#{request.version} 200 It worked!\r\n" + + "connection: close\r\n" + + "\r\n" + + "Hello World!" + ) + peer.close - ::Protocol::HTTP::Response[-1, {}, []] + ::Protocol::HTTP::Response[-1, {}, body] end end - it "reads raw response" do + it "works properly" do + expect(body).not.to receive(:close) + response = client.get("/") expect(response.read).to be == "Hello World!" From 418ca044ac8dcf86652a6831da882200608633a6 Mon Sep 17 00:00:00 2001 From: Anton Zhuravsky Date: Fri, 5 Apr 2024 12:20:13 +0100 Subject: [PATCH 3/4] Bumping protocol-http1 and updating hijack check --- async-http.gemspec | 2 +- lib/async/http/protocol/http1/connection.rb | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/async-http.gemspec b/async-http.gemspec index 5247d9c5..7bd0bcd7 100644 --- a/async-http.gemspec +++ b/async-http.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |spec| spec.add_dependency "async-io", ">= 1.28" spec.add_dependency "async-pool", ">= 0.2" spec.add_dependency "protocol-http", "~> 0.26.0" - spec.add_dependency "protocol-http1", "~> 0.18.0" + spec.add_dependency "protocol-http1", "~> 0.19.0" spec.add_dependency "protocol-http2", "~> 0.16.0" spec.add_dependency "traces", ">= 0.10.0" end diff --git a/lib/async/http/protocol/http1/connection.rb b/lib/async/http/protocol/http1/connection.rb index 753e04e7..1275c6a9 100755 --- a/lib/async/http/protocol/http1/connection.rb +++ b/lib/async/http/protocol/http1/connection.rb @@ -18,7 +18,6 @@ def initialize(stream, version) @ready = true @version = version - @hijacked = false end attr :version @@ -48,15 +47,6 @@ def peer def concurrency 1 end - - def hijack! - @hijacked = true - super - end - - def hijacked? - @hijacked - end # Can we use this connection to make requests? def viable? From 73c6b389d97a48ab5abf496558bb87da37a5d7f7 Mon Sep 17 00:00:00 2001 From: Anton Zhuravsky Date: Fri, 5 Apr 2024 12:56:12 +0100 Subject: [PATCH 4/4] Closing the body after hijack --- lib/async/http/protocol/http1/server.rb | 5 ++++- test/async/http/protocol/http11.rb | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb index c9d60751..79bc2486 100644 --- a/lib/async/http/protocol/http1/server.rb +++ b/lib/async/http/protocol/http1/server.rb @@ -50,7 +50,10 @@ def each(task: Task.current) response = yield(request, self) body = response&.body - return if hijacked? + if hijacked? + body&.close + return + end task.defer_stop do # If a response was generated, send it: diff --git a/test/async/http/protocol/http11.rb b/test/async/http/protocol/http11.rb index b452f312..ab36016c 100755 --- a/test/async/http/protocol/http11.rb +++ b/test/async/http/protocol/http11.rb @@ -103,7 +103,7 @@ def around end it "works properly" do - expect(body).not.to receive(:close) + expect(body).to receive(:close) response = client.get("/")