diff --git a/async-http.gemspec b/async-http.gemspec index cb325cf..3a710a2 100644 --- a/async-http.gemspec +++ b/async-http.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |spec| spec.add_dependency "async", ">= 2.35.1" spec.add_dependency "async-pool", "~> 0.11" spec.add_dependency "io-endpoint", "~> 0.14" - spec.add_dependency "io-stream", "~> 0.6" + spec.add_dependency "io-stream", "~> 0.14" spec.add_dependency "protocol-http", "~> 0.64" spec.add_dependency "protocol-http1", "~> 0.39" spec.add_dependency "protocol-http2", "~> 0.26" diff --git a/gems.rb b/gems.rb index 3cec837..1399a40 100644 --- a/gems.rb +++ b/gems.rb @@ -42,6 +42,7 @@ gem "sus-fixtures-async" gem "sus-fixtures-async-http", "~> 0.8" + gem "sus-fixtures-console" gem "sus-fixtures-openssl" gem "sus-fixtures-benchmark" diff --git a/lib/async/http/protocol/http1/connection.rb b/lib/async/http/protocol/http1/connection.rb index 69348c4..02e4752 100755 --- a/lib/async/http/protocol/http1/connection.rb +++ b/lib/async/http/protocol/http1/connection.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Released under the MIT License. -# Copyright, 2018-2025, by Samuel Williams. +# Copyright, 2018-2026, by Samuel Williams. require_relative "request" require_relative "response" @@ -67,7 +67,24 @@ def concurrency # Can we use this connection to make requests? def viable? - self.idle? && @stream&.readable? + unless self.idle? + return false + end + + unless @stream + return false + end + + # `nil` indicates that the connection has no data, but is still alive. An empty string means the connection is closed, while a non-empty string indicates application data on a idle HTTP/1 connection, both are failure cases. + if @stream.peek_partial(1) + return false + end + + return @stream.readable? + rescue => error + Console.debug(self, "Connection viability probe failed!", exception: error) + + return false end # @returns [Boolean] Whether the connection can be reused for another request. diff --git a/releases.md b/releases.md index c43810e..c7d66c2 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Probe idle HTTP/1 connections before reuse, avoiding requests on connections already closed by the peer. + ## v0.98.0 - Rewind request bodies before retrying requests. diff --git a/test/async/http/protocol/http1/connection.rb b/test/async/http/protocol/http1/connection.rb new file mode 100644 index 0000000..75b1cd6 --- /dev/null +++ b/test/async/http/protocol/http1/connection.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +# Released under the MIT License. +# Copyright, 2026, by Samuel Williams. + +require "async/http/protocol/http1/connection" + +require "io/stream" + +require "sus/fixtures/async/reactor_context" +require "sus/fixtures/console" +require "sus/fixtures/openssl/verified_certificate_context" +require "sus/fixtures/openssl/valid_certificate_context" + +describe Async::HTTP::Protocol::HTTP1::Connection do + include Sus::Fixtures::Async::ReactorContext + include Sus::Fixtures::Console::CapturedLogger + include Sus::Fixtures::OpenSSL::VerifiedCertificateContext + include Sus::Fixtures::OpenSSL::ValidCertificateContext + + before do + listener = TCPServer.new("localhost", 0) + port = listener.local_address.ip_port + + @sockets = [ + TCPSocket.new("localhost", port), + listener.accept, + ] + listener.close + + client_socket = OpenSSL::SSL::SSLSocket.new(@sockets[0], client_context) + server_socket = OpenSSL::SSL::SSLSocket.new(@sockets[1], server_context) + + client_socket.sync_close = true + server_socket.sync_close = true + + accept = reactor.async do + server_socket.accept + end + + connect = reactor.async do + client_socket.connect + end + + [accept, connect].each(&:wait) + + @client = IO::Stream::Buffered.wrap(client_socket) + @server = IO::Stream::Buffered.wrap(server_socket) + @connection = subject.new(@client, "HTTP/1.1") + end + + after do + @client&.close + @server&.close + + @sockets.each do |socket| + unless socket.closed? + socket.close + end + end + end + + attr :client + attr :server + attr :connection + + it "remains viable when an idle TLS connection would block" do + expect(connection).to be(:viable?) + end + + it "is not viable while active" do + connection.state = :open + + expect(connection).not.to be(:viable?) + end + + it "is not viable without a stream" do + connection = subject.new(nil, "HTTP/1.1") + + expect(connection).not.to be(:viable?) + end + + it "is not viable after a TLS close notification" do + closing = reactor.async do + server.close + end + + @sockets.first.wait_readable(1) + + expect(connection).not.to be(:viable?) + closing.wait + end + + it "is not viable after an abrupt TLS disconnect" do + @sockets.last.close + @server = nil + + @sockets.first.wait_readable(1) + + expect(connection).not.to be(:viable?) + expect_console.to have_logged( + severity: be == :debug, + subject: be_equal(connection), + message: be == "Connection viability probe failed!", + event: have_keys(type: be == :failure) + ) + end + + it "is not viable when application data is pending while idle" do + server.write("H") + server.flush + + @sockets.first.wait_readable(1) + + expect(connection).not.to be(:viable?) + expect(client.read(1)).to be == "H" + end +end diff --git a/test/async/http/ssl.rb b/test/async/http/ssl.rb index bbff241..249fe32 100644 --- a/test/async/http/ssl.rb +++ b/test/async/http/ssl.rb @@ -1,15 +1,16 @@ # frozen_string_literal: true # Released under the MIT License. -# Copyright, 2018-2025, by Samuel Williams. +# Copyright, 2018-2026, by Samuel Williams. require "async/http/server" require "async/http/client" require "async/http/endpoint" require "sus/fixtures/async" -require "sus/fixtures/openssl" require "sus/fixtures/async/http" +require "sus/fixtures/console" +require "sus/fixtures/openssl" describe Async::HTTP::Server do include Sus::Fixtures::Async::HTTP::ServerContext @@ -52,5 +53,43 @@ def make_client_endpoint(bound_endpoint) expect(response).to be(:success?) expect(response.read).to be == "Hello World!" end + + with "an idle connection closed by the server" do + include Sus::Fixtures::Console::NullLogger + + let(:connections) {[]} + + let(:app) do + connections = self.connections + + Protocol::HTTP::Middleware.for do |request| + connections << request.connection + + Protocol::HTTP::Response[200, {}, ["Hello World!"]] + end + end + + it "uses a fresh connection for a non-idempotent request" do + first_response = client.get("/") + first_response.read + first_connection = first_response.connection + + closing = reactor.async do + connections.first.close + end + + first_connection.stream.io.to_io.wait_readable(1) + + second_response = client.post("/") + + expect(second_response).to be(:success?) + expect(second_response.connection).not.to be == first_connection + expect(connections.size).to be == 2 + closing.wait + ensure + first_response&.close + second_response&.close + end + end end end