Skip to content

Commit

Permalink
Fix bug with creating requests from a URL
Browse files Browse the repository at this point in the history 
Any request created from a `URL` was not inheriting environmental
defaults even if the `URL` was relative.

Fixes #52.
  • Loading branch information
@lilyball
lilyball committed Nov 15, 2018
1 parent dd2de0a commit 9cba44c
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 49 deletions.
8 changes: 7 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
7 changes: 3 additions & 4 deletions Sources/HTTPManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1173,15 +1173,14 @@ extension HTTPManager {
private func constructRequest<T: HTTPManagerRequest>(_ 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
}
Expand Down
39 changes: 24 additions & 15 deletions Tests/AuthTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
69 changes: 40 additions & 29 deletions Tests/PMHTTPTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 9cba44c

Please sign in to comment.