diff --git a/README.md b/README.md index 11ee5a9..c2d2af6 100644 --- a/README.md +++ b/README.md @@ -410,9 +410,15 @@ work by you shall be dual licensed as above, without any additional terms or con ## Version History +#### Development + +* Fix bug where requests constructed from a `URL` would not inherit environmental defaults (e.g. auth, headers, etc) ([#52][]). + +[#52]: https://github.com/postmates/PMHTTP/issues/52 "Requests constructed with relative URLs do not inherit environment configuration · Issue #52 · postmates/PMHTTP" + #### v4.3.1 (2018-08-01) -* Add `URLProtocol` method overloads to query and set protocol properties on `HTTPManagerRequest`s ([#43][]) +* Add `URLProtocol` method overloads to query and set protocol properties on `HTTPManagerRequest`s ([#43][]). [#43]: https://github.com/postmates/PMHTTP/issues/43 "Let me attach arbitrary URLProtocol properties to a request · Issue #43 · postmates/PMHTTP" diff --git a/Sources/HTTPManager.swift b/Sources/HTTPManager.swift index a7aad76..587192a 100644 --- a/Sources/HTTPManager.swift +++ b/Sources/HTTPManager.swift @@ -1173,15 +1173,14 @@ extension HTTPManager { private func constructRequest(_ url: URL, f: (URL) -> T) -> T { let info = _configureRequestInfo() let request: T + var url = url if url.scheme == nil { // try to make it relative to the environment. This shouldn't fail, but in case it does // for some reason, just fall back to using the relative URL, which should produce a URL // loading error later. This allows us to preserve the non-optional return type. - let url_ = NSURL(string: url.absoluteString, relativeTo: environment?.baseURL) as URL? ?? url - request = f(url_) - } else { - request = f(url) + url = NSURL(string: url.absoluteString, relativeTo: environment?.baseURL) as URL? ?? url } + request = f(url) _configureRequest(request, url: url, with: info) return request } diff --git a/Tests/AuthTests.swift b/Tests/AuthTests.swift index bd21ebc..e95b1ce 100644 --- a/Tests/AuthTests.swift +++ b/Tests/AuthTests.swift @@ -86,23 +86,32 @@ final class AuthTests: PMHTTPTestCase { func testAuthEnvironmentDefaults() { HTTP.environment = HTTPManagerEnvironment(string: "http://\(httpServer.address)/v1")! - HTTP.defaultAuth = HTTPBasicAuth(username: "alice", password: "secure") - var req = HTTP.request(GET: "foo")! - XCTAssertNotNil(req.auth, "request auth") - req = HTTP.request(GET: "")! - XCTAssertNotNil(req.auth, "request auth") - req = HTTP.request(GET: "/foo")! - XCTAssertNil(req.auth, "request auth") - req = HTTP.request(GET: "/v1/foo")! - XCTAssertNotNil(req.auth, "request auth") - req = HTTP.request(GET: "http://\(httpServer.address)/v1/")! - XCTAssertNotNil(req.auth, "request auth") - req = HTTP.request(GET: "http://apple.com/v1")! - XCTAssertNil(req.auth, "request auth") - req.setDefaultEnvironmentalProperties() - XCTAssertNotNil(req.auth, "request auth") + func assertAuthNotNil(for path: String, preprocess: ((HTTPManagerDataRequest) -> Void)? = nil, line: UInt = #line) { + var req = HTTP.request(GET: path)! + preprocess?(req) + XCTAssertNotNil(req.auth, "request.auth - string", line: line) + req = HTTP.request(GET: NSURL(string: path)! as URL) + preprocess?(req) + XCTAssertNotNil(req.auth, "request.auth - URL", line: line) + } + func assertAuthNil(for path: String, preprocess: ((HTTPManagerDataRequest) -> Void)? = nil, line: UInt = #line) { + var req = HTTP.request(GET: path)! + preprocess?(req) + XCTAssertNil(req.auth, "request.auth - string", line: line) + req = HTTP.request(GET: NSURL(string: path)! as URL) + preprocess?(req) + XCTAssertNil(req.auth, "request.auth - URL", line: line) + } + + assertAuthNotNil(for: "foo") + assertAuthNotNil(for: "") + assertAuthNil(for: "/foo") + assertAuthNotNil(for: "/v1/foo") + assertAuthNotNil(for: "http://\(httpServer.address)/v1/") + assertAuthNil(for: "http://apple.com/v1") + assertAuthNotNil(for: "http://apple.com/v1", preprocess: { $0.setDefaultEnvironmentalProperties() }) } func testRetryAuthInteraction() { diff --git a/Tests/PMHTTPTests.swift b/Tests/PMHTTPTests.swift index 970daf8..862fa99 100644 --- a/Tests/PMHTTPTests.swift +++ b/Tests/PMHTTPTests.swift @@ -1791,43 +1791,54 @@ final class PMHTTPTests: PMHTTPTestCase { func testServerRequiresContentLengthEnvironmentDefaults() { HTTP.environment = HTTPManagerEnvironment(string: "http://\(httpServer.address)/v1")! - HTTP.defaultServerRequiresContentLength = true - var req = HTTP.request(GET: "foo")! - XCTAssertTrue(req.serverRequiresContentLength, "request serverRequiresContentLength") - req = HTTP.request(GET: "")! - XCTAssertTrue(req.serverRequiresContentLength, "request serverRequiresContentLength") - req = HTTP.request(GET: "/foo")! - XCTAssertFalse(req.serverRequiresContentLength, "request serverRequiresContentLength") - req = HTTP.request(GET: "/v1/foo")! - XCTAssertTrue(req.serverRequiresContentLength, "request serverRequiresContentLength") - req = HTTP.request(GET: "http://\(httpServer.address)/v1/")! - XCTAssertTrue(req.serverRequiresContentLength, "request serverRequiresContentLength") - req = HTTP.request(GET: "http://apple.com/v1")! - XCTAssertFalse(req.serverRequiresContentLength, "request serverRequiresContentLength") + func assertServerRequiresContentLength(for path: String, preprocess: ((HTTPManagerDataRequest) -> Void)? = nil, equals expected: Bool, line: UInt = #line) { + HTTP.defaultServerRequiresContentLength = true + let assert = (expected ? XCTAssertTrue : XCTAssertFalse) + var req = HTTP.request(GET: path)! + preprocess?(req) + assert(req.serverRequiresContentLength, "request.serverRequiresContentLength - string", #file, line) + req = HTTP.request(GET: NSURL(string: path)! as URL) + preprocess?(req) + assert(req.serverRequiresContentLength, "request.serverRequiresContentLength - URL", #file, line) + + HTTP.defaultServerRequiresContentLength = false + req = HTTP.request(GET: path) + preprocess?(req) + XCTAssertFalse(req.serverRequiresContentLength, "request.serverRequiresContentLength default false - string", line: line) + req = HTTP.request(GET: NSURL(string: path)! as URL) + preprocess?(req) + XCTAssertFalse(req.serverRequiresContentLength, "request.serverRequiresContentLength default false - URL", line: line) + } - req.setDefaultEnvironmentalProperties() - XCTAssertTrue(req.serverRequiresContentLength, "request serverRequiresContentLength") + assertServerRequiresContentLength(for: "foo", equals: true) + assertServerRequiresContentLength(for: "", equals: true) + assertServerRequiresContentLength(for: "/foo", equals: false) + assertServerRequiresContentLength(for: "/v1/foo", equals: true) + assertServerRequiresContentLength(for: "http://\(httpServer.address)/v1/", equals: true) + assertServerRequiresContentLength(for: "http://apple.com/v1", equals: false) + assertServerRequiresContentLength(for: "http://apple.com/v1", preprocess: { $0.setDefaultEnvironmentalProperties() }, equals: true) } func testHeaderFieldsEnvironmentDefaults() { HTTP.environment = HTTPManagerEnvironment(string: "http://\(httpServer.address)/v1")! HTTP.defaultHeaderFields = ["X-Foo": "Bar"] - var req = HTTP.request(GET: "foo")! - XCTAssertEqual(req.headerFields, ["X-Foo": "Bar"], "request headerFields") - req = HTTP.request(GET: "")! - XCTAssertEqual(req.headerFields, ["X-Foo": "Bar"], "request headerFields") - req = HTTP.request(GET: "/foo")! - XCTAssertEqual(req.headerFields, [:], "request headerFields") - req = HTTP.request(GET: "/v1/foo")! - XCTAssertEqual(req.headerFields, ["X-Foo": "Bar"], "request headerFields") - req = HTTP.request(GET: "http://\(httpServer.address)/v1/")! - XCTAssertEqual(req.headerFields, ["X-Foo": "Bar"], "request headerFields") - req = HTTP.request(GET: "http://apple.com/v1")! - XCTAssertEqual(req.headerFields, [:], "request headerFields") + func assertHeaders(for path: String, preprocess: ((HTTPManagerDataRequest) -> Void)? = nil, equals expected: HTTPManagerRequest.HTTPHeaders, line: UInt = #line) { + var req = HTTP.request(GET: path)! + preprocess?(req) + XCTAssertEqual(req.headerFields, expected, "request.headerFields - string", line: line) + req = HTTP.request(GET: NSURL(string: path)! as URL) + preprocess?(req) + XCTAssertEqual(req.headerFields, expected, "request.headerFields - URL", line: line) + } - req.setDefaultEnvironmentalProperties() - XCTAssertEqual(req.headerFields, ["X-Foo": "Bar"], "request headerFields") + assertHeaders(for: "foo", equals: ["X-Foo": "Bar"]) + assertHeaders(for: "", equals: ["X-Foo": "Bar"]) + assertHeaders(for: "/foo", equals: [:]) + assertHeaders(for: "/v1/foo", equals: ["X-Foo": "Bar"]) + assertHeaders(for: "http://\(httpServer.address)/v1/", equals: ["X-Foo": "Bar"]) + assertHeaders(for: "http://apple.com/v1", equals: [:]) + assertHeaders(for: "http://apple.com/v1", preprocess: { $0.setDefaultEnvironmentalProperties() }, equals: ["X-Foo": "Bar"]) } func testSetDefaultEnvironmentalPropertiesHeaderFieldsMerge() {