Skip to content

Commit

Permalink
Do away with using URLComponents for urlURL (#199)
Browse files Browse the repository at this point in the history
  • Loading branch information
Pushkar N Kulkarni authored and djones6 committed Jun 5, 2019
1 parent 61b29ed commit 43d9383
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 39 deletions.
31 changes: 24 additions & 7 deletions Sources/KituraNet/ClientRequest.swift
Expand Up @@ -80,6 +80,8 @@ public class ClientRequest {
*/
public private(set) var url: String = ""

private var percentEncodedURL: String = ""

/**
The HTTP method (i.e. GET, POST, PUT, DELETE) for the request.
Expand Down Expand Up @@ -226,14 +228,30 @@ public class ClientRequest {
case useHTTP2
}

private func percentEncode(_ url: String) -> String {
var _url = url
let isPercentEncoded = _url.contains("%") && URL(string: _url) != nil
if !isPercentEncoded {
_url = _url.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) ?? _url
}
return _url
}

/// Initializes a `ClientRequest` instance
///
/// - Parameter url: url for the request
/// - Parameter callback: The closure of type `Callback` to be used for the callback.
init(url: String, callback: @escaping Callback) {
self.callback = callback
self.url = url
let url = URL(string: url)!
self.percentEncodedURL = percentEncode(url)

if let url = URL(string: self.percentEncodedURL) {
initialize(url)
}
}

private func initialize(_ url: URL) {
if let host = url.host {
self.hostName = host
}
Expand All @@ -257,7 +275,6 @@ public class ClientRequest {
if let password = url.password {
self.password = password
}
self.callback = callback
}

/**
Expand Down Expand Up @@ -329,7 +346,7 @@ public class ClientRequest {
if thePath.first != "/" {
thePath = "/" + thePath
}
path = thePath.addingPercentEncoding(withAllowedCharacters: NSCharacterSet.urlQueryAllowed) ?? thePath
path = thePath
self.path = path
case .username(let userName):
self.userName = userName
Expand All @@ -348,8 +365,8 @@ public class ClientRequest {
}

//the url string
url = "\(theSchema)\(authenticationClause)\(hostName)\(port)\(path)"

self.url = "\(theSchema)\(authenticationClause)\(hostName)\(port)\(path)"
self.percentEncodedURL = percentEncode(self.url)
}

/**
Expand Down Expand Up @@ -520,7 +537,7 @@ public class ClientRequest {
var isHTTPS = false

let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
if (URL(string: url)?.scheme)! == "https" {
if (URL(string: percentEncodedURL)?.scheme)! == "https" {
isHTTPS = true
self.sslConfig = TLSConfiguration.forClient(certificateVerification: .none)
}
Expand All @@ -531,7 +548,7 @@ public class ClientRequest {
initializeClientBootstrap(eventLoopGroup: group)
}

let hostName = URL(string: url)?.host ?? "" //TODO: what could be the failure path here
let hostName = URL(string: percentEncodedURL)?.host ?? "" //TODO: what could be the failure path here
if self.headers["Host"] == nil {
self.headers["Host"] = hostName
}
Expand Down
44 changes: 14 additions & 30 deletions Sources/KituraNet/HTTP/HTTPServerRequest.swift
Expand Up @@ -65,7 +65,7 @@ public class HTTPServerRequest: ServerRequest {
*/
@available(*, deprecated, message: "This contains just the path and query parameters starting with '/'. use 'urlURL' instead")
public var urlString: String {
return _urlString
return rawURLString
}

/**
Expand All @@ -80,7 +80,7 @@ public class HTTPServerRequest: ServerRequest {
*/
public var url: Data {
//The url needs to retain the percent encodings. URL.path doesn't, so we do this.
return Data(_urlString.utf8)
return Data(rawURLString.utf8)
}

/**
Expand All @@ -99,8 +99,6 @@ public class HTTPServerRequest: ServerRequest {

private var _url: URL?

private var _urlComponents: URLComponents?

/**
Create and validate the full URL.
Expand All @@ -114,8 +112,8 @@ public class HTTPServerRequest: ServerRequest {
return _url
}

_urlComponents = URLComponents()
_urlComponents?.scheme = self.enableSSL ? "https" : "http"
var url = ""
url.append(self.enableSSL ? "https://" : "http://")

var localAddress = ""
var localAddressPort = 0
Expand All @@ -131,31 +129,26 @@ public class HTTPServerRequest: ServerRequest {

if let hostname = headers["Host"]?.first {
// Handle Host header values of the kind "localhost:8080"
_urlComponents?.host = String(hostname.split(separator: ":")[0])
url.append(hostname)
if !hostname.contains(":") {
url.append(":")
url.append(String(describing: localAddressPort))
}
} else {
Log.error("Host header not received")
let hostname = localAddress
_urlComponents?.host = hostname == "127.0.0.1" ? "localhost" : hostname
}

_urlComponents?.port = Int(localAddressPort)

let uriComponents = _urlString.split(separator: "?")
if uriComponents.count > 0 {
_urlComponents?.path = String(uriComponents[0])
url.append(hostname == "127.0.0.1" ? "localhost" : hostname)
url.append(":")
url.append(String(describing: localAddressPort))
}
url.append(rawURLString)

if uriComponents.count > 1 {
_urlComponents?.percentEncodedQuery = String(uriComponents[1])
}

if let urlURL = _urlComponents?.url {
if let urlURL = URL(string: url) {
self._url = urlURL
} else {
Log.error("URL init failed from: \(url)")
self._url = URL(string: "http://not_available/")!
}

return self._url!
}

Expand Down Expand Up @@ -220,15 +213,6 @@ public class HTTPServerRequest: ServerRequest {

private var urlStringPercentEncodingRemoved: String?

private var _urlString: String {
guard let urlStringPercentEncodingRemoved = self.urlStringPercentEncodingRemoved else {
let _urlStringPercentEncodingRemoved = rawURLString.removingPercentEncoding ?? rawURLString
self.urlStringPercentEncodingRemoved = _urlStringPercentEncodingRemoved
return _urlStringPercentEncodingRemoved
}
return urlStringPercentEncodingRemoved
}

private static func host(socketAddress: SocketAddress?) -> String {
guard let socketAddress = socketAddress else {
return ""
Expand Down
30 changes: 29 additions & 1 deletion Tests/KituraNetTests/ClientE2ETests.swift
Expand Up @@ -36,6 +36,7 @@ class ClientE2ETests: KituraNetTest {
("testUrlURL", testUrlURL),
("testQueryParameters", testQueryParameters),
("testRedirect", testRedirect),
("testPercentEncodedQuery", testPercentEncodedQuery),
]
}

Expand Down Expand Up @@ -265,7 +266,7 @@ class ClientE2ETests: KituraNetTest {
let delegate = TestURLDelegate()
performServerTest(delegate, socketType: .tcp) { expectation in
delegate.port = self.port
let headers = ["Host": "localhost:8080"]
let headers = ["Host": "localhost:\(self.port)"]
self.performRequest("post", path: ClientE2ETests.urlPath, callback: { response in
XCTAssertEqual(response?.statusCode, HTTPStatusCode.OK, "Status code wasn't .Ok was \(String(describing: response?.statusCode))")
expectation.fulfill()
Expand Down Expand Up @@ -344,6 +345,33 @@ class ClientE2ETests: KituraNetTest {
}
}

func testPercentEncodedQuery() {
class TestDelegate: ServerDelegate {
func handle(request: ServerRequest, response: ServerResponse) {
do {
let urlComponents = URLComponents(url: request.urlURL, resolvingAgainstBaseURL: false) ?? URLComponents()
XCTAssertNotNil(urlComponents.queryItems)
for queryItem in urlComponents.queryItems! {
XCTAssertEqual(queryItem.name, "parameter", "Query name should have been parameter, received \(queryItem.name)")
XCTAssertEqual(queryItem.value, "Hi There", "Query value should have been Hi There, received \(queryItem.value ?? "")")
}
response.statusCode = .OK
try response.end()
} catch {
XCTFail("Error while writing response")
}
}
}

let delegate = TestDelegate()
performServerTest(delegate) { expectation in
self.performRequest("get", path: "/zxcv?parameter=Hi%20There", callback: { response in
XCTAssertEqual(response?.statusCode, .OK, "Expected response code OK(200), but received \"(response?.statusCode)")
expectation.fulfill()
})
}
}

class TestServerDelegate: ServerDelegate {
let remoteAddress = ["127.0.0.1", "::1", "::ffff:127.0.0.1", "uds"]

Expand Down
2 changes: 1 addition & 1 deletion Tests/KituraNetTests/ClientRequestTests.swift
Expand Up @@ -104,7 +104,7 @@ class ClientRequestTests: KituraNetTest {
.path("/view/matching?key=\"viewTest\"")
]
let testRequest1 = ClientRequest(options: options1, callback: testCallback)
XCTAssertEqual(testRequest1.url, "https://66o.tech/view/matching?key=%22viewTest%22")
XCTAssertEqual(testRequest1.url, "https://66o.tech/view/matching?key=\"viewTest\"")

let options2: [ClientRequest.Options] = [ .schema("https"),
.hostname("66o.tech"),
Expand Down

0 comments on commit 43d9383

Please sign in to comment.