Browse Source

Multipart Form Data Encoding Fix (#3438)

* Fixed issue where multipart form data was encoded before URLRequest was created

* Added test to verify body parts can be appended to MFD in URLRequest creation

* Ran swiftformat to fix up formatting issues
Christian Noon 4 years ago
parent
commit
3fbfb08039
2 changed files with 68 additions and 13 deletions
  1. 20 13
      Source/Session.swift
  2. 48 0
      Tests/UploadTests.swift

+ 20 - 13
Source/Session.swift

@@ -420,7 +420,7 @@ open class Session {
                             requestModifier: RequestModifier? = nil) -> DataStreamRequest {
                             requestModifier: RequestModifier? = nil) -> DataStreamRequest {
         let convertible = RequestEncodableConvertible(url: convertible,
         let convertible = RequestEncodableConvertible(url: convertible,
                                                       method: method,
                                                       method: method,
-                                                      parameters: Optional<Empty>.none,
+                                                      parameters: Empty?.none,
                                                       encoder: URLEncodedFormParameterEncoder.default,
                                                       encoder: URLEncodedFormParameterEncoder.default,
                                                       headers: headers,
                                                       headers: headers,
                                                       requestModifier: requestModifier)
                                                       requestModifier: requestModifier)
@@ -1019,13 +1019,15 @@ open class Session {
     func performUploadRequest(_ request: UploadRequest) {
     func performUploadRequest(_ request: UploadRequest) {
         dispatchPrecondition(condition: .onQueue(requestQueue))
         dispatchPrecondition(condition: .onQueue(requestQueue))
 
 
-        do {
-            let uploadable = try request.upload.createUploadable()
-            rootQueue.async { request.didCreateUploadable(uploadable) }
-
-            performSetupOperations(for: request, convertible: request.convertible)
-        } catch {
-            rootQueue.async { request.didFailToCreateUploadable(with: error.asAFError(or: .createUploadableFailed(error: error))) }
+        performSetupOperations(for: request, convertible: request.convertible) {
+            do {
+                let uploadable = try request.upload.createUploadable()
+                self.rootQueue.async { request.didCreateUploadable(uploadable) }
+                return true
+            } catch {
+                self.rootQueue.async { request.didFailToCreateUploadable(with: error.asAFError(or: .createUploadableFailed(error: error))) }
+                return false
+            }
         }
         }
     }
     }
 
 
@@ -1040,7 +1042,10 @@ open class Session {
         }
         }
     }
     }
 
 
-    func performSetupOperations(for request: Request, convertible: URLRequestConvertible) {
+    func performSetupOperations(for request: Request,
+                                convertible: URLRequestConvertible,
+                                shouldCreateTask: @escaping () -> Bool = { true })
+    {
         dispatchPrecondition(condition: .onQueue(requestQueue))
         dispatchPrecondition(condition: .onQueue(requestQueue))
 
 
         let initialRequest: URLRequest
         let initialRequest: URLRequest
@@ -1058,6 +1063,7 @@ open class Session {
         guard !request.isCancelled else { return }
         guard !request.isCancelled else { return }
 
 
         guard let adapter = adapter(for: request) else {
         guard let adapter = adapter(for: request) else {
+            guard shouldCreateTask() else { return }
             rootQueue.async { self.didCreateURLRequest(initialRequest, for: request) }
             rootQueue.async { self.didCreateURLRequest(initialRequest, for: request) }
             return
             return
         }
         }
@@ -1067,10 +1073,11 @@ open class Session {
                 let adaptedRequest = try result.get()
                 let adaptedRequest = try result.get()
                 try adaptedRequest.validate()
                 try adaptedRequest.validate()
 
 
-                self.rootQueue.async {
-                    request.didAdaptInitialRequest(initialRequest, to: adaptedRequest)
-                    self.didCreateURLRequest(adaptedRequest, for: request)
-                }
+                self.rootQueue.async { request.didAdaptInitialRequest(initialRequest, to: adaptedRequest) }
+
+                guard shouldCreateTask() else { return }
+
+                self.rootQueue.async { self.didCreateURLRequest(adaptedRequest, for: request) }
             } catch {
             } catch {
                 self.rootQueue.async { request.didFailToAdaptURLRequest(initialRequest, withError: .requestAdaptationFailed(error: error)) }
                 self.rootQueue.async { request.didFailToAdaptURLRequest(initialRequest, withError: .requestAdaptationFailed(error: error)) }
             }
             }

+ 48 - 0
Tests/UploadTests.swift

@@ -580,6 +580,54 @@ final class UploadMultipartFormDataTestCase: BaseTestCase {
         XCTAssertTrue(response?.result.isSuccess == false)
         XCTAssertTrue(response?.result.isSuccess == false)
     }
     }
 
 
+    func testThatUploadingMultipartFormDataWorksWhenAppendingBodyPartsInURLRequestConvertible() {
+        // Given
+        struct MultipartFormDataRequest: URLRequestConvertible {
+            let multipartFormData = MultipartFormData()
+
+            func asURLRequest() throws -> URLRequest {
+                appendBodyParts()
+                return try Endpoint.method(.post).asURLRequest()
+            }
+
+            func appendBodyParts() {
+                let frenchData = Data("français".utf8)
+                multipartFormData.append(frenchData, withName: "french")
+
+                let japaneseData = Data("日本語".utf8)
+                multipartFormData.append(japaneseData, withName: "japanese")
+            }
+        }
+
+        let request = MultipartFormDataRequest()
+
+        let expectation = self.expectation(description: "multipart form data upload should succeed")
+        var response: DataResponse<Data?, AFError>?
+
+        // When
+        let uploadRequest = AF.upload(multipartFormData: request.multipartFormData, with: request)
+            .response { resp in
+                response = resp
+                expectation.fulfill()
+            }
+
+        waitForExpectations(timeout: timeout)
+
+        // Then
+        XCTAssertNotNil(response?.request)
+        XCTAssertNotNil(response?.response)
+        XCTAssertNotNil(response?.data)
+        XCTAssertNil(response?.error)
+
+        switch uploadRequest.uploadable {
+        case let .data(data):
+            XCTAssertEqual(data.count, 241)
+
+        default:
+            XCTFail("Uploadable should be of type data and not be empty")
+        }
+    }
+
     #if os(macOS)
     #if os(macOS)
     func disabled_testThatUploadingMultipartFormDataOnBackgroundSessionWritesDataToFileToAvoidCrash() {
     func disabled_testThatUploadingMultipartFormDataOnBackgroundSessionWritesDataToFileToAvoidCrash() {
         // Given
         // Given