Add Host header (#650)

[MihaelIsaev]

I realized that Host header is missing which causes 400 Bad Request: missing required Host header.

I added missing header into HTTP1ProxyConnectionHandler and now it works like a charm!

Also added Host header check into tests.

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[swift-server-bot]

Can one of the admins verify this patch?

[Lukasa]

Really nice change, thanks!

[Lukasa]

@swift-server-bot test this please

[dnadoba]

This is not the right host in all cases. We allow users to overwrite the host and also allow non-default ports.
We already have a method which can take care of this for us:

async-http-client/Sources/AsyncHTTPClient/RequestValidation.swift

Lines 114 to 129 in 49abfc3

	extension HTTPHeaders {
	mutating func addHostIfNeeded(for url: DeconstructedURL) {
	// if no host header was set, let's use the url host
	guard !self.contains(name: "host"),
	var host = url.connectionTarget.host
	else {
	return
	}
	// if the request uses a non-default port, we need to add it after the host
	if let port = url.connectionTarget.port,
	port != url.scheme.defaultPort {
	host += ":\(port)"
	}
	self.add(name: "host", value: host)
	}
	}

[MihaelIsaev]

It seems I can't use this nice method cause I don't have DeconstructedURL in this context.

Tried to build DeconstructedURL but it requires many lines of code.
I just implemented check on place using what we have.

Sending one more pull request #652 since this one is closed already.
Please let me know if I should build DeconstructedURL and use addHostIfNeeded instead.

[MihaelIsaev]

@dnadoba could you please take a look at #652

[Lukasa]

I'm pretty sure this is the right host in all cases: the Host header for CONNECT should be the Host of the target URI, the same thing we put in the request line. That's what this should be.

[dnadoba]

Given the following request:

let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
var request = HTTPClientRequest(url: "http://127.0.0.1/")
request.headers.add(name: "host", value: "swift.org")
let response = try await httpClient.execute(request, timeout: .seconds(10))

This will currently send a request header with

Host: swift.org

and not

Host: 127.0.0.1

This can be useful to e.g. to test a local nginx configuration without modifying /etc/hosts. AFAIK browsers don't support this for security reasons.

If we now add a CONNECT proxy into the mix with the following request:

var configuration = HTTPClient.Configuration()
configuration.proxy = .server(host: "example.com", port: 80)
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
var request = HTTPClientRequest(url: "http://127.0.0.1/")
request.headers.add(name: "host", value: "swift.org")

we will make a connection to example.com and send it this request:

CONNECT 127.0.0.1:80 HTTP/1.1
Host: 127.0.0.1

and afterwards will send a HTTP request with the following header:

Host: swift.org

The HTTP/1.1 RFC doesn't really mention what the Host headers value should be for in the initial CONNECT request. I thought that the Host should be the same as the Host in the subsequent request. This would allow the proxy to connect you to a server that can handle the request. However I might be wrong, that is just a guess on my side.

curl doesn't use the Host in the header and also not the host of the proxy we connect to but will use the host of the request URL:

% curl -v -curl -H "Host: swift.org" --proxy "http://[::1]:8888" https://127.0.0.1/ 
*   Trying ::1:8888...
* Connected to ::1 (::1) port 8888 (#0)
* allocate connect buffer
* Establish HTTP proxy tunnel to 127.0.0.1:443
> CONNECT 127.0.0.1:443 HTTP/1.1
> Host: 127.0.0.1:443
> User-Agent: curl/7.85.0
> Proxy-Connection: Keep-Alive

I have tried setting up a proxy with URLSession to also see its behaviour but have failed to do so.

I think we should at least follow curl here and also use the host specified in the request URL for the initial CONNECT and not the host & port of the proxy.

What you you think @Lukasa?

[Lukasa]

We are following curl here, no? Looking at the test, we appear to be doing exactly the same thing as curl.