Ignore bad requests.

async-http.gemspec

@@ -30,7 +30,7 @@ Gem::Specification.new do |spec|
 	spec.add_dependency "async-pool", "~> 0.12"
 	spec.add_dependency "io-endpoint", "~> 0.18"
 	spec.add_dependency "io-stream", "~> 0.14"
-	spec.add_dependency "protocol-http", "~> 0.66"
+	spec.add_dependency "protocol-http", "~> 0.72"
 	spec.add_dependency "protocol-http1", "~> 0.41"
 	spec.add_dependency "protocol-http2", "~> 0.28"
 	spec.add_dependency "protocol-url", "~> 0.2"

fixtures/async/http/a_protocol.rb

@@ -44,6 +44,28 @@ module HTTP
 				end
 			end
 
+			with "bad requests" do
+				let(:app) do
+					::Protocol::HTTP::Middleware.for do
+						@app_called = true
+
+						::Protocol::HTTP::Response[200]
+					end
+				end
+
+				it "responds consistently without invoking the application" do
+					@app_called = false
+					response = client.get("/", [["range", "bytes=4-1"]])
+
+					expect(response.status).to be == 400
+					expect(response.headers["content-type"]).to be == "text/plain; charset=utf-8"
+					expect(response.read).to be == "Protocol::HTTP::Header::Range::ParseError"
+					expect(@app_called).to be == false
+				ensure
+					response&.close
+				end
+			end
+
 			with "interim response" do
 				let(:app) do
 					::Protocol::HTTP::Middleware.for do |request|

lib/async/http/protocol/http1/server.rb

@@ -12,6 +12,7 @@
 
 require "async/promise"
 require "console/event/failure"
+require "protocol/http/body/buffered"
 
 module Async
 	module HTTP
@@ -33,12 +34,15 @@ def closed(error = nil)
 						@ready.resolve(nil)
 					end
 
-					# Write a failure response with the given status code.
+					# Write a failure response with the given status code and error class name.
 					# @parameter status [Integer] The HTTP status code to send.
-					def fail_request(status)
+					# @parameter error [Exception] The error which caused the request to fail.
+					def fail_request(status, error)
+						body = ::Protocol::HTTP::Body::Buffered.wrap(error.class.name)
+
 						@persistent = false
-						write_response(@version, status, {})
-						write_body(@version, nil)
+						write_response(@version, status, {"content-type" => "text/plain; charset=utf-8"})
+						write_body(@version, body)
 					rescue => error
 						# At this point, there is very little we can do to recover:
 						Console.debug(self, "Failed to write failure response!", error)
@@ -61,8 +65,8 @@ def next_request
 						end
 
 						return request
-					rescue ::Protocol::HTTP1::BadRequest
-						fail_request(400)
+					rescue ::Protocol::HTTP::BadRequest => error
+						fail_request(400, error)
 						# Conceivably we could retry here, but we don't really know how bad the error is, so it's better to just fail:
 						raise
 					end

lib/async/http/protocol/http2/request.rb

@@ -24,6 +24,29 @@ def initialize(*)
 
 						attr :request
 
+						# Write a failure response with the given status code and error class name.
+						# @parameter status [Integer] The HTTP status code to send.
+						# @parameter error [Exception] The error which caused the request to fail.
+						def fail_request(status, error)
+							body = error.class.name
+							headers = [
+								[STATUS, status.to_s],
+								["content-type", "text/plain; charset=utf-8"],
+							]
+
+							if @request.head?
+								send_headers(headers, ::Protocol::HTTP2::END_STREAM)
+							else
+								send_headers(headers)
+								send_data(body, ::Protocol::HTTP2::END_STREAM)
+							end
+
+							# The peer may still be sending a request body. The response is complete, so release the stream without reporting a protocol error.
+							send_reset_stream(::Protocol::HTTP2::Error::NO_ERROR) unless closed?
+						rescue => error
+							Console.debug(self, "Failed to write failure response!", error)
+						end
+
 						# Process the initial headers received from the client and construct the request.
 						# @parameter headers [Array] The list of header key-value pairs.
 						# @parameter end_stream [Boolean] Whether the stream is complete after these headers.
@@ -67,11 +90,13 @@ def receive_initial_headers(headers, end_stream)
 								end
 							end
 
-							@request.headers = @headers
-
 							unless @request.valid?
 								raise ::Protocol::HTTP2::HeaderError, "Request is missing required headers!"
 							else
+								# Validate structured headers before exposing the request to the application:
+								@headers.to_h
+								@request.headers = @headers
+
 								# We only construct the input/body if data is coming.
 								unless end_stream
 									@request.body = prepare_input(@length)
@@ -82,6 +107,8 @@ def receive_initial_headers(headers, end_stream)
 							end
 
 							return headers
+						rescue ::Protocol::HTTP::BadRequest => error
+							fail_request(400, error)
 						end
 
 						# Called when the stream is closed.

lib/async/http/server.rb

@@ -70,6 +70,8 @@ def accept(peer, address, task: Task.current)
 					# If this returns nil, we assume that the connection has been hijacked.
 					self.call(request)
 				end
+			rescue ::Protocol::HTTP::BadRequest
+				# Ignore bad requests, just close the connection.
 			ensure
 				connection&.close
 			end

releases.md

@@ -1,5 +1,9 @@
 # Releases
 
+## Unreleased
+
+  - Respond with `400 Bad Request` for any `Protocol::HTTP::BadRequest` raised while parsing HTTP/1 or HTTP/2 requests, include the exception class name without reflecting request data, and avoid reporting them as unhandled server errors.
+
 ## v0.103.0
 
   - Handle `RST_STREAM(NO_ERROR)` as an orderly HTTP/2 stream closure while still failing requests whose streams close before any response headers are received.

test/async/http/protocol/http11.rb

@@ -102,7 +102,9 @@ def around
 				response = client.get("")
 
 				expect(response.status).to be == 400
+				expect(response.read).to be == "Protocol::HTTP1::InvalidRequest"
 			end
+
 		end
 
 		with "head request" do

test/async/http/protocol/http2.rb

@@ -43,6 +43,75 @@
 					client.post("/", [[":authority", "foo"]])
 				end.to raise_exception(Protocol::HTTP2::StreamError)
 			end
+
+			with "a bad request header" do
+				let(:app) do
+					Protocol::HTTP::Middleware.for do
+						@app_called = true
+						Protocol::HTTP::Response[200]
+					end
+				end
+
+				it "keeps the connection reusable" do
+					@app_called = false
+					response = client.get("/", [["range", "bytes=4-1"]])
+
+					expect(response.status).to be == 400
+					expect(@app_called).to be == false
+
+					response.finish
+					response = client.get("/")
+
+					expect(response.status).to be == 200
+					expect(@app_called).to be == true
+				ensure
+					response&.close
+				end
+
+				it "does not send response data for HEAD" do
+					response = client.head("/", [["range", "bytes=4-1"]])
+
+					expect(response.status).to be == 400
+					expect(response.body.length).to be_nil
+					expect(response.read).to be_nil
+				ensure
+					response&.close
+				end
+
+				it "terminates an unfinished request body" do
+					body = Async::HTTP::Body::Writable.new
+					response = client.post("/", [["range", "bytes=4-1"]], body)
+					connection = response.connection
+
+					expect(response.status).to be == 400
+					expect(response.read).to be == "Protocol::HTTP::Header::Range::ParseError"
+					expect(response.stream).to be(:closed?)
+					expect(connection.streams).to be(:empty?)
+					expect(connection).to be(:reusable?)
+				ensure
+					body&.close
+					response&.close
+				end
+			end
+
+			with "a malformed request" do
+				it "prioritizes protocol validation over bad request handling" do
+					client.pool.acquire do |connection|
+						response = connection.create_response
+						response.stream.send_headers([
+							[":method", "GET"],
+							[":path", "/"],
+							["range", "bytes=4-1"],
+						], ::Protocol::HTTP2::END_STREAM)
+
+						expect do
+							connection.read_response(response)
+						end.to raise_exception(Protocol::HTTP2::StreamError).and(
+							have_attributes(code: be == Protocol::HTTP2::Error::STREAM_CLOSED)
+						)
+					end
+				end
+			end
 		end
 
 		with "closed streams" do