improve request validation

motivation: safer handling of request validation and mutation

changes:
* fix schema logic to be non-case sensitive
* made request version, method and url immutable
* made request scheme and host internal
* adjusted redirect ahndler implementation to stricter request immutabllity
* adjust and add tests

Author: tomerd

Sources/AsyncHTTPClient/HTTPHandler.swift

@@ -60,13 +60,14 @@ extension HTTPClient {
     }
 
     public struct Request {
-        public var version: HTTPVersion
-        public var method: HTTPMethod
-        public var url: URL
-        public var scheme: String
-        public var host: String
+        public let version: HTTPVersion
+        public let method: HTTPMethod
+        public let url: URL
         public var headers: HTTPHeaders
         public var body: Body?
+        
+        internal let scheme: String
+        internal let host: String
 
         public init(url: String, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
             guard let url = URL(string: url) else {
@@ -77,7 +78,7 @@ extension HTTPClient {
         }
 
         public init(url: URL, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
-            guard let scheme = url.scheme else {
+            guard let scheme = url.scheme?.lowercased() else {
                 throw HTTPClientError.emptyScheme
             }
 
@@ -99,14 +100,14 @@ extension HTTPClient {
         }
 
         public var useTLS: Bool {
-            return self.url.scheme == "https"
+            return self.scheme == "https"
         }
 
         public var port: Int {
             return self.url.port ?? (self.useTLS ? 443 : 80)
         }
 
-        static func isSchemeSupported(scheme: String?) -> Bool {
+        static func isSchemeSupported(scheme: String) -> Bool {
             return scheme == "http" || scheme == "https"
         }
     }
@@ -514,7 +515,7 @@ internal struct RedirectHandler<T> {
             return nil
         }
 
-        guard HTTPClient.Request.isSchemeSupported(scheme: url.scheme) else {
+        guard HTTPClient.Request.isSchemeSupported(scheme: request.scheme) else {
             return nil
         }
 
@@ -526,44 +527,38 @@ internal struct RedirectHandler<T> {
     }
 
     func redirect(status: HTTPResponseStatus, to redirectURL: URL, promise: EventLoopPromise<T>) {
-        let originalURL = self.request.url
-
-        var request = self.request
-        request.url = redirectURL
-
-        if let redirectHost = redirectURL.host {
-            request.host = redirectHost
-        } else {
-            preconditionFailure("redirectURL doesn't contain a host")
-        }
-
-        if let redirectScheme = redirectURL.scheme {
-            request.scheme = redirectScheme
-        } else {
-            preconditionFailure("redirectURL doesn't contain a scheme")
-        }
+        let originalRequest = self.request
 
         var convertToGet = false
         if status == .seeOther, request.method != .HEAD {
             convertToGet = true
         } else if status == .movedPermanently || status == .found, request.method == .POST {
             convertToGet = true
         }
-
+        
+        var method = originalRequest.method
+        var headers = originalRequest.headers
+        var body = originalRequest.body
+        
         if convertToGet {
-            request.method = .GET
-            request.body = nil
-            request.headers.remove(name: "Content-Length")
-            request.headers.remove(name: "Content-Type")
+            method = .GET
+            body = nil
+            headers.remove(name: "Content-Length")
+            headers.remove(name: "Content-Type")
         }
 
-        if !originalURL.hasTheSameOrigin(as: redirectURL) {
-            request.headers.remove(name: "Origin")
-            request.headers.remove(name: "Cookie")
-            request.headers.remove(name: "Authorization")
-            request.headers.remove(name: "Proxy-Authorization")
+        if !originalRequest.url.hasTheSameOrigin(as: redirectURL) {
+            headers.remove(name: "Origin")
+            headers.remove(name: "Cookie")
+            headers.remove(name: "Authorization")
+            headers.remove(name: "Proxy-Authorization")
+        }
+        
+        do {
+            let request = try HTTPClient.Request(url: redirectURL, version: originalRequest.version,  method: method, headers: headers, body: body)
+            return self.execute(request).futureResult.cascade(to: promise)
+        } catch {
+            return promise.fail(error)
         }
-
-        return self.execute(request).futureResult.cascade(to: promise)
     }
 }

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

@@ -26,6 +26,8 @@ extension HTTPClientTests {
     static var allTests: [(String, (HTTPClientTests) -> () throws -> Void)] {
         return [
             ("testRequestURI", testRequestURI),
+            ("testBadRequestURI", testBadRequestURI),
+            ("testSchemaCasing", testSchemaCasing),
             ("testGet", testGet),
             ("testGetWithSharedEventLoopGroup", testGetWithSharedEventLoopGroup),
             ("testPost", testPost),

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -12,7 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import AsyncHTTPClient
+@testable import AsyncHTTPClient
 import NIO
 import NIOFoundationCompat
 import NIOHTTP1
@@ -33,6 +33,26 @@ class HTTPClientTests: XCTestCase {
         XCTAssertEqual(request2.url.path, "")
     }
 
+    func testBadRequestURI() throws {
+        XCTAssertThrowsError(try Request(url: "some/path"), "should throw") { error in
+            XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyScheme)
+        }
+        XCTAssertThrowsError(try Request(url: "file://somewhere/some/path?foo=bar"), "should throw") { error in
+            XCTAssertEqual(error as! HTTPClientError, HTTPClientError.unsupportedScheme("file"))
+        }
+        XCTAssertThrowsError(try Request(url: "https:/foo"), "should throw") { error in
+            XCTAssertEqual(error as! HTTPClientError, HTTPClientError.emptyHost)
+        }
+    }
+
+    func testSchemaCasing() throws {
+        let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar")
+        XCTAssertNotNil(request1)
+
+        let request2 = try Request(url: "hTTpS://someserver.com:8888/some/path?foo=bar")
+        XCTAssertNotNil(request2)
+    }
+
     func testGet() throws {
         let httpBin = HttpBin()
         let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)