Skip to content

Commit 1e3e29f

Browse files
committed
improve request validation
motivation: safer handling of request validation and mutation changes: * fix scheme logic to be non-case sensitive * made request version, method and url immutable * made request scheme and host internal * adjusted redirect handler implementation to stricter request immutabllity * adjust and add tests
1 parent 80cd7ce commit 1e3e29f

File tree

3 files changed

+56
-30
lines changed

3 files changed

+56
-30
lines changed

Sources/AsyncHTTPClient/HTTPHandler.swift

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,13 @@ extension HTTPClient {
9292
public var version: HTTPVersion
9393
/// Request HTTP method, defaults to `GET`.
9494
public var method: HTTPMethod
95-
/// Remote URL.
96-
public var url: URL
97-
/// Remote HTTP scheme, resolved from `URL`.
98-
public var scheme: String
99-
/// Remote host, resolved from `URL`.
100-
public var host: String
10195
/// Request custom HTTP Headers, defaults to no headers.
10296
public var headers: HTTPHeaders
10397
/// Request body, defaults to no body.
10498
public var body: Body?
99+
private var _url: URL!
100+
private var _scheme: String!
101+
private var _host: String!
105102

106103
/// Create HTTP request.
107104
///
@@ -137,7 +134,15 @@ extension HTTPClient {
137134
/// - `unsupportedScheme` if URL does contains unsupported HTTP scheme.
138135
/// - `emptyHost` if URL does not contains a host.
139136
public init(url: URL, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
140-
guard let scheme = url.scheme else {
137+
self.version = version
138+
self.method = method
139+
self.headers = headers
140+
self.body = body
141+
try self.setURL(url)
142+
}
143+
144+
public mutating func setURL(_ url: URL) throws {
145+
guard let scheme = url.scheme?.lowercased() else {
141146
throw HTTPClientError.emptyScheme
142147
}
143148

@@ -149,26 +154,37 @@ extension HTTPClient {
149154
throw HTTPClientError.emptyHost
150155
}
151156

152-
self.version = version
153-
self.method = method
154-
self.url = url
155-
self.scheme = scheme
156-
self.host = host
157-
self.headers = headers
158-
self.body = body
157+
self._url = url
158+
self._scheme = scheme
159+
self._host = host
160+
}
161+
162+
/// Remote URL.
163+
public var url: URL {
164+
return self._url
165+
}
166+
167+
/// Remote HTTP scheme, resolved from `URL`.
168+
public var scheme: String {
169+
return self._scheme
170+
}
171+
172+
/// Remote host, resolved from `URL`.
173+
public var host: String {
174+
return self._host
159175
}
160176

161177
/// Whether request will be executed using secure socket.
162178
public var useTLS: Bool {
163-
return self.url.scheme == "https"
179+
return self.scheme == "https"
164180
}
165181

166182
/// Resolved port.
167183
public var port: Int {
168184
return self.url.port ?? (self.useTLS ? 443 : 80)
169185
}
170186

171-
static func isSchemeSupported(scheme: String?) -> Bool {
187+
static func isSchemeSupported(scheme: String) -> Bool {
172188
return scheme == "http" || scheme == "https"
173189
}
174190
}
@@ -637,7 +653,7 @@ internal struct RedirectHandler<T> {
637653
return nil
638654
}
639655

640-
guard HTTPClient.Request.isSchemeSupported(scheme: url.scheme) else {
656+
guard HTTPClient.Request.isSchemeSupported(scheme: request.scheme) else {
641657
return nil
642658
}
643659

@@ -652,18 +668,10 @@ internal struct RedirectHandler<T> {
652668
let originalURL = self.request.url
653669

654670
var request = self.request
655-
request.url = redirectURL
656-
657-
if let redirectHost = redirectURL.host {
658-
request.host = redirectHost
659-
} else {
660-
preconditionFailure("redirectURL doesn't contain a host")
661-
}
662-
663-
if let redirectScheme = redirectURL.scheme {
664-
request.scheme = redirectScheme
665-
} else {
666-
preconditionFailure("redirectURL doesn't contain a scheme")
671+
do {
672+
try request.setURL(redirectURL)
673+
} catch {
674+
return promise.fail(error)
667675
}
668676

669677
var convertToGet = false

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ extension HTTPClientTests {
2626
static var allTests: [(String, (HTTPClientTests) -> () throws -> Void)] {
2727
return [
2828
("testRequestURI", testRequestURI),
29+
("testBadRequestURI", testBadRequestURI),
30+
("testSchemaCasing", testSchemaCasing),
2931
("testGet", testGet),
3032
("testGetWithSharedEventLoopGroup", testGetWithSharedEventLoopGroup),
3133
("testPost", testPost),

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class HTTPClientTests: XCTestCase {
2323

2424
func testRequestURI() throws {
2525
let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar")
26-
XCTAssertEqual(request1.host, "someserver.com")
26+
XCTAssertEqual(request1.url.host, "someserver.com")
2727
XCTAssertEqual(request1.url.path, "/some/path")
2828
XCTAssertEqual(request1.url.query!, "foo=bar")
2929
XCTAssertEqual(request1.port, 8888)
@@ -33,6 +33,22 @@ class HTTPClientTests: XCTestCase {
3333
XCTAssertEqual(request2.url.path, "")
3434
}
3535

36+
func testBadRequestURI() throws {
37+
XCTAssertThrowsError(try Request(url: "some/path"), "should throw") { error in
38+
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyScheme)
39+
}
40+
XCTAssertThrowsError(try Request(url: "file://somewhere/some/path?foo=bar"), "should throw") { error in
41+
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.unsupportedScheme("file"))
42+
}
43+
XCTAssertThrowsError(try Request(url: "https:/foo"), "should throw") { error in
44+
XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyHost)
45+
}
46+
}
47+
48+
func testSchemaCasing() throws {
49+
XCTAssertNoThrow(try Request(url: "hTTpS://someserver.com:8888/some/path?foo=bar"))
50+
}
51+
3652
func testGet() throws {
3753
let httpBin = HttpBin()
3854
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)

0 commit comments

Comments
 (0)