Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Host header (#650) #652

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix Host header (#650) #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e91ceb8
Add `Host` header (#650)
MihaelIsaev Dec 6, 2022
f5b686d
Fix `Host` header (#650)
MihaelIsaev Dec 6, 2022
1329e84
Merge branch 'main' into imike-host-fix
MihaelIsaev Dec 6, 2022
c72a3eb
Fix `Host` header (#650)
MihaelIsaev Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand

private var state: State = .initialized

private let scheme: Scheme
private let targetHost: String
private let targetPort: Int
private let proxyAuthorization: HTTPClient.Authorization?
Expand All @@ -46,6 +47,7 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
}

convenience init(
scheme: Scheme,
target: ConnectionTarget,
proxyAuthorization: HTTPClient.Authorization?,
deadline: NIODeadline
Expand All @@ -63,17 +65,20 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
fatalError("Unix Domain Sockets do not support proxies")
}
self.init(
scheme: scheme,
targetHost: targetHost,
targetPort: targetPort,
proxyAuthorization: proxyAuthorization,
deadline: deadline
)
}

init(targetHost: String,
init(scheme: Scheme,
targetHost: String,
targetPort: Int,
proxyAuthorization: HTTPClient.Authorization?,
deadline: NIODeadline) {
self.scheme = scheme
self.targetHost = targetHost
self.targetPort = targetPort
self.proxyAuthorization = proxyAuthorization
Expand Down Expand Up @@ -155,7 +160,13 @@ final class HTTP1ProxyConnectHandler: ChannelDuplexHandler, RemovableChannelHand
method: .CONNECT,
uri: "\(self.targetHost):\(self.targetPort)"
)
head.headers.replaceOrAdd(name: "host", value: "\(self.targetHost)")
if !head.headers.contains(name: "host") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HTTPRequestHead is created in the lines above and we can therefore say for sure we will never have a host set. This makes sense because we don't need to look into the headers we have created here but from the original HTTP request from the user. This will require plumping the HTTPHeaders from the original request through the stack.

FYI: @rnro is currently working on moving HTTP1ProxyConnectHandler to swift-nio-extras in this PR. In this new version the HTTPHeaders are passed into the HTTP1ProxyConnectHandler.init which makes this a bit easier. We still need to get the original request headers to where we create the HTTP1ProxyConnectHandler in AHC.

var host = self.targetHost
if (self.targetPort != self.scheme.defaultPort) {
host += ":\(self.targetPort)"
}
head.headers.add(name: "host", value: host)
}
if let authorization = self.proxyAuthorization {
head.headers.replaceOrAdd(name: "proxy-authorization", value: authorization.headerValue)
}
Expand Down
1 change: 1 addition & 0 deletions Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ extension HTTPConnectionPool.ConnectionFactory {
let encoder = HTTPRequestEncoder()
let decoder = ByteToMessageHandler(HTTPResponseDecoder(leftOverBytesStrategy: .dropBytes))
let proxyHandler = HTTP1ProxyConnectHandler(
scheme: self.key.scheme,
target: self.key.connectionTarget,
proxyAuthorization: proxy.authorization,
deadline: deadline
Expand Down
5 changes: 5 additions & 0 deletions Tests/AsyncHTTPClientTests/HTTP1ProxyConnectHandlerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())

let proxyConnectHandler = HTTP1ProxyConnectHandler(
scheme: .https,
targetHost: "swift.org",
targetPort: 443,
proxyAuthorization: .none,
Expand Down Expand Up @@ -61,6 +62,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())

let proxyConnectHandler = HTTP1ProxyConnectHandler(
scheme: .https,
targetHost: "swift.org",
targetPort: 443,
proxyAuthorization: .basic(credentials: "abc123"),
Expand Down Expand Up @@ -95,6 +97,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())

let proxyConnectHandler = HTTP1ProxyConnectHandler(
scheme: .https,
targetHost: "swift.org",
targetPort: 443,
proxyAuthorization: .none,
Expand Down Expand Up @@ -135,6 +138,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())

let proxyConnectHandler = HTTP1ProxyConnectHandler(
scheme: .https,
targetHost: "swift.org",
targetPort: 443,
proxyAuthorization: .none,
Expand Down Expand Up @@ -175,6 +179,7 @@ class HTTP1ProxyConnectHandlerTests: XCTestCase {
XCTAssertNoThrow(try embedded.connect(to: socketAddress).wait())

let proxyConnectHandler = HTTP1ProxyConnectHandler(
scheme: .https,
targetHost: "swift.org",
targetPort: 443,
proxyAuthorization: .none,
Expand Down