Skip to content

Commit

Permalink
Add new property serverRequiresContentLength
Browse files Browse the repository at this point in the history
This property turns off body streaming and causes us to encode JSON and
multipart bodies synchronously instead, so we can provide a
Content-Length.

As usual, there is a corresponding `defaultServerRequiresContentLength`
property on the `HTTPManager`, which only applies to requests contained
within the environment.

Add a method `HTTPManagerRequest.setDefaultEnvironmentalProperties()`.
This sets `auth` and `serverRequiresContentLength` to the `HTTPManager`
defaults regardless of the environment.
  • Loading branch information
Kevin Ballard committed Aug 18, 2017
1 parent 4c2ab1b commit 3792555
Show file tree
Hide file tree
Showing 11 changed files with 515 additions and 177 deletions.
8 changes: 8 additions & 0 deletions PMHTTP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
9ED4FA181CC072F2001A0693 /* MultipartTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ED4FA171CC072F2001A0693 /* MultipartTests.swift */; };
9ED5D3621D936295007C2A65 /* Deprecations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ED5D3611D936295007C2A65 /* Deprecations.swift */; };
9ED901301E2EDB4E00332D39 /* ImageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9ED9012F1E2EDB4E00332D39 /* ImageTests.swift */; };
9EDBA9B21F477360005EDC9F /* InputStream+ReadAll.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EDBA9B11F47735F005EDC9F /* InputStream+ReadAll.swift */; };
9EDBA9B41F478BB9005EDC9F /* InputStreamTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EDBA9B31F478BB9005EDC9F /* InputStreamTests.swift */; };
9EEF318B1E4A74C40086AAFF /* HTTPAuth.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EEF318A1E4A74C40086AAFF /* HTTPAuth.swift */; };
9EEF318D1E4ABF050086AAFF /* AuthTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EEF318C1E4ABF050086AAFF /* AuthTests.swift */; };
9EEF318F1E4D4F440086AAFF /* SSLTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EEF318E1E4D4F440086AAFF /* SSLTests.swift */; };
Expand Down Expand Up @@ -103,6 +105,8 @@
9ED4FA171CC072F2001A0693 /* MultipartTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultipartTests.swift; sourceTree = "<group>"; };
9ED5D3611D936295007C2A65 /* Deprecations.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Deprecations.swift; sourceTree = "<group>"; };
9ED9012F1E2EDB4E00332D39 /* ImageTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImageTests.swift; sourceTree = "<group>"; };
9EDBA9B11F47735F005EDC9F /* InputStream+ReadAll.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "InputStream+ReadAll.swift"; sourceTree = "<group>"; };
9EDBA9B31F478BB9005EDC9F /* InputStreamTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InputStreamTests.swift; sourceTree = "<group>"; };
9EEF318A1E4A74C40086AAFF /* HTTPAuth.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPAuth.swift; sourceTree = "<group>"; };
9EEF318C1E4ABF050086AAFF /* AuthTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthTests.swift; sourceTree = "<group>"; };
9EEF318E1E4D4F440086AAFF /* SSLTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SSLTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -185,6 +189,7 @@
9E4C77CC1C3C696D000FF8AC /* UploadSupport.swift */,
9ED4FA151CC01E54001A0693 /* HTTPBodyStream.swift */,
9E29514A1C4D95CB001D38AC /* Utilities.swift */,
9EDBA9B11F47735F005EDC9F /* InputStream+ReadAll.swift */,
9E39E9BF1C3E100D005F7A95 /* NetworkActivityManager.swift */,
9ED5D3611D936295007C2A65 /* Deprecations.swift */,
9E22DD291C88CEF200C49993 /* DateParsing.swift */,
Expand Down Expand Up @@ -216,6 +221,7 @@
9EEF318C1E4ABF050086AAFF /* AuthTests.swift */,
9E11A8DB1D1B4AC100D63318 /* NetworkActivityTests.swift */,
9EEF69F51D2C343E00C6E603 /* KVOTests.swift */,
9EDBA9B31F478BB9005EDC9F /* InputStreamTests.swift */,
9E22DD2B1C88D09100C49993 /* DateParsingTests.swift */,
9E681CB91C56FBF100422CE4 /* SipHashTests.swift */,
9E7DDF321C18F2B600EA43AD /* Info.plist */,
Expand Down Expand Up @@ -383,6 +389,7 @@
9E29514B1C4D95CB001D38AC /* Utilities.swift in Sources */,
9E4C77CB1C3B56D6000FF8AC /* HTTPManagerTask.swift in Sources */,
9E22DD2A1C88CEF200C49993 /* DateParsing.swift in Sources */,
9EDBA9B21F477360005EDC9F /* InputStream+ReadAll.swift in Sources */,
9E4C77A91C360714000FF8AC /* ObjectiveC.swift in Sources */,
9E14D4FF1DC81E4800BE34D5 /* PlatformSpecific.swift in Sources */,
9EEF318B1E4A74C40086AAFF /* HTTPAuth.swift in Sources */,
Expand All @@ -395,6 +402,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
9EDBA9B41F478BB9005EDC9F /* InputStreamTests.swift in Sources */,
9EEF69F61D2C343E00C6E603 /* KVOTests.swift in Sources */,
9ED901301E2EDB4E00332D39 /* ImageTests.swift in Sources */,
9EA4EE371CBC197D00E4E531 /* MockingTests.swift in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,8 @@ Unless you explicitly state otherwise, any contribution intentionally submitted
#### Development

* Add overloads to the request creation methods that take a `URL`. These overloads return a non-optional request.
* Add new property `HTTPManagerRequest.serverRequiresContentLength`. This disables streaming body support (for JSON and multipart/mixed) and instead encodes the body synchronously so it can provide a `"Content-Length"` header to the server. There is a corresponding `HTTPManager.defaultServerRequiresContentLength` property as well.
* Add a method `HTTPManagerRequest.setDefaultEnvironmentalProperties()` that sets properties to the `HTTPManager`-defined defaults that otherwise are only set if the request's path matches the environment. This is primarily intended for requests constructed using absolute paths (e.g. `HTTP.request(GET: "/foo")`) that should still use the environment defaults. Right now this method only sets `auth` and `serverRequiresContentLength`.

#### v3.0.2 (2017-05-01)

Expand Down
73 changes: 65 additions & 8 deletions Sources/HTTPManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public final class HTTPManager: NSObject {
/// but requests created with absolute URLs will continue to work. See `HTTPManagerConfigurable`
/// for how to configure the shared `HTTPManager` prior to first use.
///
/// - SeeAlso: `resetSession()`, `HTTPManagerConfigurable`, `defaultAuth`.
/// - SeeAlso: `resetSession()`, `HTTPManagerConfigurable`, `defaultAuth`,
/// `defaultServerRequiresContentLength`.
public var environment: Environment? {
get {
return inner.sync({ $0.environment })
Expand Down Expand Up @@ -208,6 +209,32 @@ public final class HTTPManager: NSObject {
}
}

/// If `true`, assume the server requires the `Content-Length` header for uploads. The default
/// value is `false`.
///
/// Setting this to `true` forces JSON and multipart/mixed uploads to be encoded synchronously
/// when the request is performed rather than happening in the background.
///
/// - Note: This property is only used for HTTP requests that are located within the current
/// environment's base URL. If a request is created with an absolute path or absolute URL, and
/// the resulting URL does not represent a resource found within the environment's base URL,
/// the request will not be assigned this value.
///
/// Changes to this property affect any newly-created requests but do not affect any existing
/// requests or any tasks that are in-progress.
///
/// - SeeAlso: `environment`, `HTTPManagerRequest.serverRequiresContentLength`.
public var defaultServerRequiresContentLength: Bool {
get {
return inner.sync({ $0.defaultServerRequiresContentLength })
}
set {
inner.asyncBarrier {
$0.defaultServerRequiresContentLength = newValue
}
}
}

/// The user agent that's passed to every request.
public var userAgent: String {
return inner.sync({
Expand Down Expand Up @@ -303,6 +330,7 @@ public final class HTTPManager: NSObject {
var defaultAuth: HTTPAuth?
var defaultRetryBehavior: HTTPManagerRetryBehavior?
var defaultAssumeErrorsAreJSON: Bool = false
var defaultServerRequiresContentLength: Bool = false

var session: URLSession!
var sessionDelegate: SessionDelegate!
Expand Down Expand Up @@ -1049,27 +1077,40 @@ extension HTTPManager {
return request
}

private typealias ConfigureRequestInfo = (environment: Environment?, auth: HTTPAuth?, defaultRetryBehavior: HTTPManagerRetryBehavior?, assumeErrorsAreJSON: Bool)
private typealias ConfigureRequestInfo = (environment: Environment?, auth: HTTPAuth?, defaultRetryBehavior: HTTPManagerRetryBehavior?, assumeErrorsAreJSON: Bool, serverRequiresContentLength: Bool)

private func _configureRequestInfo() -> ConfigureRequestInfo {
return inner.sync({ inner in
return (inner.environment, inner.defaultAuth, inner.defaultRetryBehavior, inner.defaultAssumeErrorsAreJSON)
return (inner.environment, inner.defaultAuth, inner.defaultRetryBehavior, inner.defaultAssumeErrorsAreJSON, inner.defaultServerRequiresContentLength)
})
}

private func _configureRequest<T: HTTPManagerRequest>(_ request: T, url: URL, with info: ConfigureRequestInfo) {
if let auth = info.auth, let environment = info.environment,
// make sure we aren't suppressing this auth
!HTTPManager.isAuthSuppressed(auth),
if let environment = info.environment,
// make sure the requested entity is within the space defined by baseURL
environment.isPrefix(of: url)
{
request.auth = auth
// NB: If you add more properties here, also update
// `applyEnvironmentDefaultValues(to:)`.
if let auth = info.auth, !HTTPManager.isAuthSuppressed(auth) {
request.auth = auth
}
request.serverRequiresContentLength = info.serverRequiresContentLength
}
request.retryBehavior = info.defaultRetryBehavior
request.assumeErrorsAreJSON = info.assumeErrorsAreJSON
}

internal func applyEnvironmentDefaultValues(to request: HTTPManagerRequest) {
inner.sync { inner in
let auth = inner.defaultAuth
if auth.map({ !HTTPManager.isAuthSuppressed($0) }) ?? true {
request.auth = auth
}
request.serverRequiresContentLength = inner.defaultServerRequiresContentLength
}
}

private func expandParameters(_ parameters: [String: Any]) -> [URLQueryItem] {
var queryItems: [URLQueryItem] = []
queryItems.reserveCapacity(parameters.count) // optimize for no expansion
Expand Down Expand Up @@ -1606,8 +1647,24 @@ extension HTTPManager {
internal func createNetworkTaskWithRequest(_ request: HTTPManagerRequest, uploadBody: UploadBody?, processor: @escaping (HTTPManagerTask, HTTPManagerTaskResult<Data>, _ authToken: Any??, _ attempt: Int, _ retry: @escaping (_ isAuthRetry: Bool) -> Bool) -> Void) -> HTTPManagerTask {
var urlRequest = request._preparedURLRequest
var uploadBody = uploadBody
if case .formUrlEncoded(let queryItems)? = uploadBody {
switch uploadBody {
case nil, .data?: break
case .formUrlEncoded(let queryItems)?:
uploadBody = .data(FormURLEncoded.data(for: queryItems))
case .json(let json)? where request.serverRequiresContentLength:
uploadBody = .data(JSON.encodeAsData(json))
case .json?: break
case let .multipartMixed(boundary, parameters, bodyParts)? where request.serverRequiresContentLength:
let stream = HTTPBody.createMultipartMixedStream(boundary, parameters: parameters, bodyParts: bodyParts)
stream.open()
do {
uploadBody = try .data(stream.readAll())
} catch {
// If we can't read it now (and it really shouldn't be able to fail anyway), just
// leave it as a streaming body, where it will presumably fail again and produce a
// URL error.
}
case .multipartMixed?: break
}
uploadBody?.evaluatePending()
// NB: We are evaluating the mock before adding the auth headers. If we ever add the ability
Expand Down
36 changes: 36 additions & 0 deletions Sources/HTTPManagerRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,17 @@ public class HTTPManagerRequest: NSObject, NSCopying {
/// - SeeAlso: `HTTPManager.defaultAssumeErrorsAreJSON`.
public var assumeErrorsAreJSON: Bool = false

/// If `true`, assume the server requires the `Content-Length` header for uploads. The default
/// value is `false`.
///
/// Setting this to `true` forces JSON and multipart/mixed uploads to be encoded synchronously
/// when the request is performed rather than happening in the background.
///
/// The default value is provided by `HTTPManager.defaultServerRequiresContentLength`.
///
/// - SeeAlso: `HTTPManager.defaultServerRequiresContentLength`.
public var serverRequiresContentLength: Bool = false

/// Whether tasks created from this request should affect the visiblity of the
/// network activity indicator. Default is `true`.
///
Expand Down Expand Up @@ -191,6 +202,29 @@ public class HTTPManagerRequest: NSObject, NSCopying {
return self
}

/// Sets properties whose default values depend on the environment.
///
/// This will set all properties whose default value depends on the environment to the value
/// they would have if the request was located within the environment. For example, this will
/// set the `auth` property to `HTTP.defaultAuth`.
///
/// This is intended for use with requests that are constructed using an absolute path (and
/// therefore are still at the same domain), but want to be treated as though they're within the
/// environment path.
///
/// **Example:**
///
/// ```
/// HTTP.request(GET: "/foo")
/// .with({ $0.setDefaultEnvironmentalProperties() })
/// .performRequest { task, result in
/// // ....
/// }
/// ```
public func setDefaultEnvironmentalProperties() {
apiManager.applyEnvironmentDefaultValues(to: self)
}

public func copy(with _: NSZone? = nil) -> Any {
return type(of: self).init(__copyOfRequest: self)
}
Expand Down Expand Up @@ -227,6 +261,7 @@ public class HTTPManagerRequest: NSObject, NSCopying {
userInitiated = request.userInitiated
retryBehavior = request.retryBehavior
assumeErrorsAreJSON = request.assumeErrorsAreJSON
serverRequiresContentLength = request.serverRequiresContentLength
mock = request.mock
affectsNetworkActivityIndicator = request.affectsNetworkActivityIndicator
headerFields = request.headerFields
Expand Down Expand Up @@ -955,6 +990,7 @@ public final class HTTPManagerParseRequest<T>: HTTPManagerRequest, HTTPManagerRe
userInitiated = request.userInitiated
retryBehavior = request.retryBehavior
assumeErrorsAreJSON = request.assumeErrorsAreJSON
serverRequiresContentLength = request.serverRequiresContentLength
mock = request.mock
affectsNetworkActivityIndicator = request.affectsNetworkActivityIndicator
headerFields = request.headerFields
Expand Down
65 changes: 65 additions & 0 deletions Sources/InputStream+ReadAll.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//
// InputStream+ReadAll.swift
// PMHTTP
//
// Created by Kevin Ballard on 8/18/17.
// Copyright © 2017 Postmates. All rights reserved.
//

import Foundation

internal extension InputStream {
/// An error thrown from `readAll()` if the stream indicates that an error occurred but
/// `streamError` is `nil`.
struct UnknownError: Error {}

/// Reads the entire stream and returns the contents as a `Data`.
///
/// - Requires: The stream must be opened before invoking this method.
///
/// - Note: This method automatically calls `close()` on the stream when it's done.
///
/// - Throws: An error if the stream returns an error while reading.
@nonobjc
func readAll() throws -> Data {
let cap = 64 * 1024
var len = 0
var buf = UnsafeMutablePointer<UInt8>.allocate(capacity: cap)
var data = DispatchData.empty
while true {
switch read(buf, maxLength: cap - len) {
case 0: // EOF
if len > 0 {
data.append(DispatchData(bytesNoCopy: UnsafeBufferPointer(start: buf, count: len), deallocator: DispatchData.Deallocator.custom(nil, { [buf] in
buf.deallocate(capacity: cap)
})))
} else {
buf.deallocate(capacity: cap)
}
// This is an ugly hack, but DispatchData doesn't have any way to go to Data directly.
// This relies on the fact that OS_dispatch_data is toll-free bridged to NSData, even though Swift doesn't understand that.
let nsdata = unsafeDowncast(data as AnyObject, to: NSData.self)
return Data(referencing: nsdata)
case let n where n > 0:
let overflow: Bool
(len, overflow) = Int.addWithOverflow(len, n)
guard len <= cap && !overflow else {
// The stream claims to have written more bytes than is available. We don't know
// how to handle this.
buf.deallocate(capacity: cap)
throw UnknownError()
}
if len == cap {
data.append(DispatchData(bytesNoCopy: UnsafeBufferPointer(start: buf, count: len), deallocator: DispatchData.Deallocator.custom(nil, { [buf] in
buf.deallocate(capacity: cap)
})))
buf = UnsafeMutablePointer<UInt8>.allocate(capacity: cap)
len = 0
}
default: // -1
buf.deallocate(capacity: cap)
throw streamError ?? UnknownError()
}
}
}
}
5 changes: 5 additions & 0 deletions Sources/ObjectiveC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,11 @@ public final class HTTPManagerObjectParseRequest: HTTPManagerRequest, HTTPManage
set { _request.assumeErrorsAreJSON = newValue }
}

public override var serverRequiresContentLength: Bool {
get { return _request.serverRequiresContentLength }
set { _request.serverRequiresContentLength = newValue }
}

public override var affectsNetworkActivityIndicator: Bool {
get { return _request.affectsNetworkActivityIndicator }
set { _request.affectsNetworkActivityIndicator = newValue }
Expand Down
Loading

3 comments on commit 3792555

@sgruby
Copy link

@sgruby sgruby commented on 3792555 Aug 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, you're fast! On the surface, this looks good.

With the change, the POSTs are working well with the broken server I'm using.

Thanks!

@lilyball
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to hear it! I'll go ahead and publish a release with this.

@lilyball
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Released as v3.0.3.

Please sign in to comment.