Skip to content

Bugfix HTTPS SNI and IP Address #139

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

Merged
merged 7 commits into from
Dec 16, 2019
Merged

Bugfix HTTPS SNI and IP Address #139

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7893432
Bugfix HTTPS SNI and IP Address
StrangeDays Nov 29, 2019
0b77f94
swiftformat, rework isIPAddress and import regarding pull request com…
StrangeDays Nov 29, 2019
ae6836b
Merge branch 'master' into bugfix/https-sni-ip
StrangeDays Dec 8, 2019
38fdd44
Merge branch 'master' into bugfix/https-sni-ip
StrangeDays Dec 10, 2019
107bd4e
Merge branch 'master' into bugfix/https-sni-ip
StrangeDays Dec 11, 2019
00646a0
Merge branch 'master' into bugfix/https-sni-ip
StrangeDays Dec 16, 2019
ef404bf
Update HTTPClientTests.swift
weissi Dec 16, 2019
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
3 changes: 1 addition & 2 deletions Sources/AsyncHTTPClient/HTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ public class HTTPClient {
channel.writeAndFlush(request)
}
.cascadeFailure(to: task.promise)

return task
}

Expand Down Expand Up @@ -501,7 +500,7 @@ private extension ChannelPipeline {
do {
let tlsConfiguration = tlsConfiguration ?? TLSConfiguration.forClient()
let context = try NIOSSLContext(configuration: tlsConfiguration)
return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: request.host),
return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: request.host.isIPAddress ? nil : request.host),
position: .first)
} catch {
return self.eventLoop.makeFailedFuture(error)
Expand Down
26 changes: 26 additions & 0 deletions Sources/AsyncHTTPClient/Utils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,32 @@
import NIO
import NIOHTTP1

#if canImport(Network)
import Network

internal extension String {
var isIPAddress: Bool {
if IPv4Address(self) != nil || IPv6Address(self) != nil {
return true
}
return false
}
}

#else
internal extension String {
var isIPAddress: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Lukasa should we add smth like this to NIO? This is unrelated to this PR but we could make a 'good-first-issue'

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I wouldn't object, though we'd be extending a type we don't own so we'd have to prefix the method name, which is kinda gross.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And actually don't we have this already in the form of SocketAddress(ipAddress:port:)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Lukasa ish, need to give a port too. But maybe that's not too bad?

extension String {
    var isIPAddress: Bool {
        return SocketAddress(ipAddress: self, port: 0) != nil
   }
}

?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect it's fine, and the only reason not to use it here is that you do have to heap-allocate for that code.

var ipv4Addr = in_addr()
var ipv6Addr = in6_addr()

return self.withCString { ptr in
inet_pton(AF_INET, ptr, &ipv4Addr) == 1 ||
inet_pton(AF_INET6, ptr, &ipv6Addr) == 1
}
}
}
#endif

public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate {
public typealias Response = Void

Expand Down
1 change: 1 addition & 0 deletions Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ extension HTTPClientTests {
("testGetWithDifferentEventLoopBackpressure", testGetWithDifferentEventLoopBackpressure),
("testPost", testPost),
("testGetHttps", testGetHttps),
("testGetHttpsWithIP", testGetHttpsWithIP),
("testPostHttps", testPostHttps),
("testHttpRedirect", testHttpRedirect),
("testHttpHostRedirect", testHttpHostRedirect),
Expand Down
15 changes: 14 additions & 1 deletion Tests/AsyncHTTPClientTests/HTTPClientTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,19 @@ class HTTPClientTests: XCTestCase {
XCTAssertEqual(.ok, response.status)
}

func testGetHttpsWithIP() throws {
let httpBin = HTTPBin(ssl: true)
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
configuration: HTTPClient.Configuration(certificateVerification: .none))
defer {
XCTAssertNoThrow(try httpClient.syncShutdown())
XCTAssertNoThrow(try httpBin.shutdown())
}

let response = try httpClient.get(url: "https://127.0.0.1:\(httpBin.port)/get").wait()
XCTAssertEqual(.ok, response.status)
}

func testPostHttps() throws {
let httpBin = HTTPBin(ssl: true)
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
Expand Down Expand Up @@ -831,7 +844,7 @@ class HTTPClientTests: XCTestCase {
XCTAssertNoThrow(try web.writeOutbound(.head(.init(version: .init(major: 1, minor: 0),
status: .ok,
headers: HTTPHeaders([("connection",
i % 2 == 0 ? "close" : "keep-alive")])))))
i % 2 == 0 ? "close" : "keep-alive")])))))
XCTAssertNoThrow(try web.writeOutbound(.end(nil)))

var response: HTTPClient.Response?
Expand Down