Unify CONNECT semantics.

Author: ioquatix

async-http.gemspec

@@ -28,8 +28,8 @@ Gem::Specification.new do |spec|
 	spec.add_dependency "async-pool", "~> 0.9"
 	spec.add_dependency "io-endpoint", "~> 0.14"
 	spec.add_dependency "io-stream", "~> 0.6"
-	spec.add_dependency "protocol-http", "~> 0.43"
-	spec.add_dependency "protocol-http1", "~> 0.29"
+	spec.add_dependency "protocol-http", "~> 0.49"
+	spec.add_dependency "protocol-http1", "~> 0.30"
 	spec.add_dependency "protocol-http2", "~> 0.22"
 	spec.add_dependency "traces", "~> 0.10"
 	spec.add_dependency "metrics", "~> 0.12"

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

@@ -39,7 +39,16 @@ def call(request, task: Task.current)
 
 						# We carefully interpret https://tools.ietf.org/html/rfc7230#section-6.3.1 to implement this correctly.
 						begin
-							write_request(request.authority, request.method, request.path, @version, request.headers)
+							target = request.path
+							authority = request.authority
+							
+							# If we are using a CONNECT request, we need to use the authority as the target:
+							if request.connect?
+								target = authority
+								authority = nil
+							end
+							
+							write_request(authority, request.method, target, @version, request.headers)
 						rescue
 							# If we fail to fully write the request and body, we can retry this request.
 							raise RequestFailed

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

@@ -10,21 +10,45 @@ module HTTP
 		module Protocol
 			module HTTP1
 				class Request < Protocol::Request
+					def self.valid_path?(target)
+						if target.start_with?("/")
+							return true
+						elsif target == '*'
+							return true
+						else
+							return false
+						end
+					end
+					
+					URI_PATTERN = %r{\A(?<scheme>[^:/]+)://(?<authority>[^/]+)(?<path>.*)\z}
+					
 					def self.read(connection)
 						connection.read_request do |authority, method, target, version, headers, body|
-							self.new(connection, authority, method, target, version, headers, body)
+							if method == ::Protocol::HTTP::Methods::CONNECT
+								# We put the target into the authority field for CONNECT requests, as per HTTP/2 semantics.
+								self.new(connection, nil, target, method, nil, version, headers, body)
+							elsif valid_path?(target)
+								# This is a valid request.
+								self.new(connection, nil, authority, method, target, version, headers, body)
+							elsif match = target.match(URI_PATTERN)
+								# We map the incoming absolute URI target to the scheme, authority, and path fields of the request.
+								self.new(connection, match[:scheme], match[:authority], method, match[:path], version, headers, body)
+							else
+								# This is an invalid request.
+								raise ::Protocol::HTTP1::BadRequest.new("Invalid request target: #{target}")
+							end
 						end
 					end
 
 					UPGRADE = "upgrade"
 
-					def initialize(connection, authority, method, path, version, headers, body)
+					def initialize(connection, scheme, authority, method, path, version, headers, body)
 						@connection = connection
 
 						# HTTP/1 requests with an upgrade header (which can contain zero or more values) are extracted into the protocol field of the request, and we expect a response to select one of those protocols with a status code of 101 Switching Protocols.
 						protocol = headers.delete("upgrade")
 
-						super(nil, authority, method, path, version, headers, body, protocol, self.public_method(:write_interim_response))
+						super(scheme, authority, method, path, version, headers, body, protocol, self.public_method(:write_interim_response))
 					end
 
 					def connection

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

@@ -99,7 +99,7 @@ def connection
 					end
 
 					def valid?
-						@scheme and @method and @path
+						@scheme and @method and (@path or @method == ::Protocol::HTTP::Methods::CONNECT)
 					end
 
 					def hijack?

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

@@ -204,9 +204,12 @@ def send_request(request)
 						pseudo_headers = [
 							[SCHEME, request.scheme],
 							[METHOD, request.method],
-							[PATH, request.path],
 						]
 
+						if path = request.path
+							pseudo_headers << [PATH, path]
+						end
+						
 						# To ensure that the HTTP/1.1 request line can be reproduced accurately, this pseudo-header field MUST be omitted when translating from an HTTP/1.1 request that has a request target in origin or asterisk form (see [RFC7230], Section 5.3). Clients that generate HTTP/2 requests directly SHOULD use the :authority pseudo-header field instead of the Host header field.
 						if authority = request.authority
 							pseudo_headers << [AUTHORITY, authority]

lib/async/http/proxy.rb

@@ -82,7 +82,7 @@ def close
 			def connect(&block)
 				input = Body::Writable.new
 
-				response = @client.connect(@address.to_s, @headers, input)
+				response = @client.connect(authority: @address, headers: @headers, body: input)
 
 				if response.success?
 					pipe = Body::Pipe.new(response.body, input)

releases.md

@@ -1,5 +1,31 @@
 # Releases
 
+## Unreleased
+
+### Unify HTTP/1 and HTTP/2 `CONNECT` semantics
+
+HTTP/1 has a request line "target" which takes different forms depending on the kind of request. For `CONNECT` requests, the target is the authority (host and port) of the target server, e.g.
+
+```
+CONNECT example.com:443 HTTP/1.1
+```
+
+In HTTP/2, the `CONNECT` method uses the `:authority` pseudo-header to specify the target, e.g.
+
+```http
+[HEADERS FRAME]
+:method: connect
+:authority: example.com:443
+```
+
+In HTTP/1, the `Request#path` attribute was previously used to store the target, and this was incorrectly mapped to the `:path` pseudo-header in HTTP/2. This has been corrected, and the `Request#authority` attribute is now used to store the target for both HTTP/1 and HTTP/2, and mapped accordingly. Thus, to make a `CONNECT` request, you should set the `Request#authority` attribute, e.g.
+
+```ruby
+response = client.connect(authority: "example.com:443")
+```
+
+For HTTP/1, the authority is mapped back to the request line target, and for HTTP/2, it is mapped to the `:authority` pseudo-header.
+
 ## v0.86.0
 
   - Add support for HTTP/2 `NO_RFC7540_PRIORITIES`. See <https://www.rfc-editor.org/rfc/rfc9218.html> for more details.

test/async/http/proxy.rb

@@ -54,7 +54,7 @@
 		it "can connect and hijack connection" do
 			input = Async::HTTP::Body::Writable.new
 
-			response = client.connect("127.0.0.1:1234", [], input)
+			response = client.connect(body: input, authority: "127.0.0.1:1234")
 
 			expect(response).to be(:success?)
 
@@ -68,7 +68,7 @@
 	with "echo server" do
 		let(:app) do
 			Protocol::HTTP::Middleware.for do |request|
-				expect(request.path).to be == "localhost:1"
+				expect(request.authority).to be == "localhost:1"
 
 				Async::HTTP::Body::Hijack.response(request, 200, {}) do |stream|
 					while chunk = stream.read_partial(1024)
@@ -125,7 +125,7 @@
 					next Protocol::HTTP::Response[407, [], nil]
 				end
 
-				host, port = request.path.split(":", 2)
+				host, port = request.authority.split(":", 2)
 				endpoint = IO::Endpoint.tcp(host, port)
 
 				Console.logger.debug(self) {"Making connection to #{endpoint}..."}