From 8d38220862ef6603353fb90f550604436e925644 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Fri, 12 Jun 2020 10:53:54 +0100 Subject: [PATCH 1/5] The host header should also include the port --- Sources/AsyncHTTPClient/HTTPHandler.swift | 7 ++++++- .../HTTPClientInternalTests.swift | 4 ++-- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 14 +++++++------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 3dc32bf84..6770cb35e 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -253,6 +253,11 @@ extension HTTPClient { return self.scheme == "https" || self.scheme == "https+unix" } + /// Specified port. + public var specifiedPort: Int? { + return self.url.port + } + /// Resolved port. public var port: Int { return self.url.port ?? (self.useTLS ? 443 : 80) @@ -772,7 +777,7 @@ extension TaskHandler: ChannelDuplexHandler { var headers = request.headers if !request.headers.contains(name: "Host") { - headers.add(name: "Host", value: request.host) + headers.add(name: "Host", value: "\(request.host)\(request.specifiedPort.map { ":\($0)" } ?? "")") } do { diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 5656fbe46..20e5d1938 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -49,7 +49,7 @@ class HTTPClientInternalTests: XCTestCase { ignoreUncleanSSLShutdown: false, logger: HTTPClient.loggingDisabled)).wait() - var request = try Request(url: "http://localhost/get") + var request = try Request(url: "http://localhost:8080/get") request.headers.add(name: "X-Test-Header", value: "X-Test-Value") request.body = .string("1234") @@ -57,7 +57,7 @@ class HTTPClientInternalTests: XCTestCase { XCTAssertEqual(3, recorder.writes.count) var head = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") head.headers.add(name: "X-Test-Header", value: "X-Test-Value") - head.headers.add(name: "Host", value: "localhost") + head.headers.add(name: "Host", value: "localhost:8080") head.headers.add(name: "Content-Length", value: "4") XCTAssertEqual(HTTPClientRequestPart.head(head), recorder.writes[0]) let buffer = ByteBuffer.of(string: "1234") diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 4cde4ab9f..15e36cf93 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -216,7 +216,7 @@ class HTTPClientTests: XCTestCase { return } let hostName = try? JSONDecoder().decode(RequestInfo.self, from: body).data - XCTAssertEqual("127.0.0.1", hostName) + XCTAssertEqual("127.0.0.1:\(self.defaultHTTPBin.port)", hostName) } func testPercentEncoded() throws { @@ -763,7 +763,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) @@ -787,7 +787,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) @@ -808,7 +808,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) @@ -831,7 +831,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) @@ -859,7 +859,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) @@ -1036,7 +1036,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(XCTAssertEqual(.head(.init(version: .init(major: 1, minor: 1), method: .GET, uri: "/foo", - headers: HTTPHeaders([("Host", "localhost")]))), + headers: HTTPHeaders([("Host", "localhost:\(web.serverPort)")]))), try web.readInbound())) XCTAssertNoThrow(XCTAssertEqual(.end(nil), try web.readInbound())) From b159a0bbd0136020052c7301539b3453a6ecfc37 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Fri, 12 Jun 2020 13:57:53 +0100 Subject: [PATCH 2/5] Remove public from specifiedPort --- Sources/AsyncHTTPClient/HTTPHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 6770cb35e..13b81295a 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -254,7 +254,7 @@ extension HTTPClient { } /// Specified port. - public var specifiedPort: Int? { + var specifiedPort: Int? { return self.url.port } From d4a3722712e3a830e736832aaeb837c986da2741 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Mon, 15 Jun 2020 11:20:28 +0100 Subject: [PATCH 3/5] Changed to only output port when it is not 80 or 443 Also added test --- Sources/AsyncHTTPClient/HTTPHandler.swift | 9 ++---- .../HTTPClientInternalTests+XCTest.swift | 1 + .../HTTPClientInternalTests.swift | 31 +++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 13b81295a..935e233fe 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -253,11 +253,6 @@ extension HTTPClient { return self.scheme == "https" || self.scheme == "https+unix" } - /// Specified port. - var specifiedPort: Int? { - return self.url.port - } - /// Resolved port. public var port: Int { return self.url.port ?? (self.useTLS ? 443 : 80) @@ -777,7 +772,9 @@ extension TaskHandler: ChannelDuplexHandler { var headers = request.headers if !request.headers.contains(name: "Host") { - headers.add(name: "Host", value: "\(request.host)\(request.specifiedPort.map { ":\($0)" } ?? "")") + let port = request.port + let host = (port != 80 && port != 443) ? "\(request.host):\(port)" : request.host + headers.add(name: "Host", value: host) } do { diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift index 8122df7a2..9a3e11b50 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift @@ -27,6 +27,7 @@ extension HTTPClientInternalTests { return [ ("testHTTPPartsHandler", testHTTPPartsHandler), ("testBadHTTPRequest", testBadHTTPRequest), + ("testHostPort", testHostPort), ("testHTTPPartsHandlerMultiBody", testHTTPPartsHandlerMultiBody), ("testProxyStreaming", testProxyStreaming), ("testProxyStreamingFailure", testProxyStreamingFailure), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 20e5d1938..7f30f47da 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -90,6 +90,37 @@ class HTTPClientInternalTests: XCTestCase { } } + func testHostPort() throws { + let channel = EmbeddedChannel() + let recorder = RecordingHandler() + let task = Task(eventLoop: channel.eventLoop, logger: HTTPClient.loggingDisabled) + + try channel.pipeline.addHandler(recorder).wait() + try channel.pipeline.addHandler(TaskHandler(task: task, + kind: .host, + delegate: TestHTTPDelegate(), + redirectHandler: nil, + ignoreUncleanSSLShutdown: false, + logger: HTTPClient.loggingDisabled)).wait() + + let request1 = try Request(url: "http://localhost:80/get") + XCTAssertNoThrow(try channel.writeOutbound(request1)) + let request2 = try Request(url: "https://localhost/get") + XCTAssertNoThrow(try channel.writeOutbound(request2)) + let request3 = try Request(url: "http://localhost:8080/get") + XCTAssertNoThrow(try channel.writeOutbound(request3)) + + var head1 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") + head1.headers.add(name: "Host", value: "localhost") + XCTAssertEqual(HTTPClientRequestPart.head(head1), recorder.writes[0]) + var head2 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") + head2.headers.add(name: "Host", value: "localhost") + XCTAssertEqual(HTTPClientRequestPart.head(head2), recorder.writes[2]) + var head3 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") + head3.headers.add(name: "Host", value: "localhost:8080") + XCTAssertEqual(HTTPClientRequestPart.head(head3), recorder.writes[4]) + } + func testHTTPPartsHandlerMultiBody() throws { let channel = EmbeddedChannel() let delegate = TestHTTPDelegate() From c27176c1c6db2e3ce191645b5c7083d25427e050 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Mon, 15 Jun 2020 12:51:49 +0100 Subject: [PATCH 4/5] Don't output port when it is default for current scheme --- Sources/AsyncHTTPClient/HTTPHandler.swift | 9 ++++++--- .../HTTPClientInternalTests.swift | 17 +++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 935e233fe..6c757119d 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -771,10 +771,13 @@ extension TaskHandler: ChannelDuplexHandler { uri: request.uri) var headers = request.headers - if !request.headers.contains(name: "Host") { + if !request.headers.contains(name: "host") { let port = request.port - let host = (port != 80 && port != 443) ? "\(request.host):\(port)" : request.host - headers.add(name: "Host", value: host) + var host = request.host + if (port != 80 || request.scheme != "http") && (port != 443 || request.scheme != "https") { + host += ":\(port)" + } + headers.add(name: "host", value: host) } do { diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 7f30f47da..08258d96a 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -109,16 +109,21 @@ class HTTPClientInternalTests: XCTestCase { XCTAssertNoThrow(try channel.writeOutbound(request2)) let request3 = try Request(url: "http://localhost:8080/get") XCTAssertNoThrow(try channel.writeOutbound(request3)) + let request4 = try Request(url: "http://localhost:443/get") + XCTAssertNoThrow(try channel.writeOutbound(request4)) + let request5 = try Request(url: "https://localhost:80/get") + XCTAssertNoThrow(try channel.writeOutbound(request5)) - var head1 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") - head1.headers.add(name: "Host", value: "localhost") + let head1 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get", headers: ["host": "localhost"]) XCTAssertEqual(HTTPClientRequestPart.head(head1), recorder.writes[0]) - var head2 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") - head2.headers.add(name: "Host", value: "localhost") + let head2 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get", headers: ["host": "localhost"]) XCTAssertEqual(HTTPClientRequestPart.head(head2), recorder.writes[2]) - var head3 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get") - head3.headers.add(name: "Host", value: "localhost:8080") + let head3 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get", headers: ["host": "localhost:8080"]) XCTAssertEqual(HTTPClientRequestPart.head(head3), recorder.writes[4]) + let head4 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get", headers: ["host": "localhost:443"]) + XCTAssertEqual(HTTPClientRequestPart.head(head4), recorder.writes[6]) + let head5 = HTTPRequestHead(version: HTTPVersion(major: 1, minor: 1), method: .GET, uri: "/get", headers: ["host": "localhost:80"]) + XCTAssertEqual(HTTPClientRequestPart.head(head5), recorder.writes[8]) } func testHTTPPartsHandlerMultiBody() throws { From 5b4ec0b26c70d7bf2c17105f850ba89cddd1993c Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Mon, 15 Jun 2020 14:26:04 +0100 Subject: [PATCH 5/5] Update port/scheme if statement --- Sources/AsyncHTTPClient/HTTPHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift index 6c757119d..733bfbb2b 100644 --- a/Sources/AsyncHTTPClient/HTTPHandler.swift +++ b/Sources/AsyncHTTPClient/HTTPHandler.swift @@ -774,7 +774,7 @@ extension TaskHandler: ChannelDuplexHandler { if !request.headers.contains(name: "host") { let port = request.port var host = request.host - if (port != 80 || request.scheme != "http") && (port != 443 || request.scheme != "https") { + if !(port == 80 && request.scheme == "http"), !(port == 443 && request.scheme == "https") { host += ":\(port)" } headers.add(name: "host", value: host)