Squashed 'apollo-ios/' changes from 7122421c..bc5a0cdf

bc5a0cdf fix: Multipart delimiter boundary parsing (#502)

git-subtree-dir: apollo-ios
git-subtree-split: bc5a0cdfd11388824aace092701cbdd19ac1c575

Sources/Apollo/MultipartResponseParsingInterceptor.swift

@@ -85,8 +85,14 @@ public struct MultipartResponseParsingInterceptor: ApolloInterceptor {
       return
     }
 
-    for chunk in dataString.components(separatedBy: "--\(boundary)") {
-      if chunk.isEmpty || chunk.isBoundaryMarker { continue }
+    // Parsing Notes:
+    //
+    // Multipart messages arriving here may consist of more than one chunk, but they are always
+    // expected to be complete chunks. Downstream protocol specification parsers are only built
+    // to handle the protocol specific message formats, i.e.: data between the multipart delimiter.
+    let boundaryDelimiter = Self.boundaryDelimiter(with: boundary)
+    for chunk in dataString.components(separatedBy: boundaryDelimiter) {
+      if chunk.isEmpty || chunk.isDashBoundaryPrefix || chunk.isMultipartNewLine { continue }
 
       switch parser.parse(chunk) {
       case let .success(data):
@@ -119,6 +125,8 @@ public struct MultipartResponseParsingInterceptor: ApolloInterceptor {
   }
 }
 
+// MARK: Specification Parser Protocol
+
 /// A protocol that multipart response parsers must conform to in order to be added to the list of
 /// available response specification parsers.
 protocol MultipartResponseSpecificationParser {
@@ -140,6 +148,38 @@ extension MultipartResponseSpecificationParser {
   static var dataLineSeparator: StaticString { "\r\n\r\n" }
 }
 
-fileprivate extension String {
-  var isBoundaryMarker: Bool { self == "--" }
+// MARK: Helpers
+
+extension MultipartResponseParsingInterceptor {
+  static func boundaryDelimiter(with boundary: String) -> String {
+    "\r\n--\(boundary)"
+  }
+
+  static func closeBoundaryDelimiter(with boundary: String) -> String {
+    boundaryDelimiter(with: boundary) + "--"
+  }
+}
+
+extension String {
+  fileprivate var isDashBoundaryPrefix: Bool { self == "--" }
+  fileprivate var isMultipartNewLine: Bool { self == "\r\n" }
+
+  /// Returns the range of a complete multipart chunk.
+  func multipartRange(using boundary: String) -> String.Index? {
+    // The end boundary marker indicates that no further chunks will follow so if this delimiter
+    // if found then include the delimiter in the index. Search for this first.
+    let closeBoundaryDelimiter = MultipartResponseParsingInterceptor.closeBoundaryDelimiter(with: boundary)
+    if let endIndex = range(of: closeBoundaryDelimiter, options: .backwards)?.upperBound {
+      return endIndex
+    }
+
+    // A chunk boundary indicates there may still be more chunks to follow so the index need not
+    // include the chunk boundary in the index.
+    let boundaryDelimiter = MultipartResponseParsingInterceptor.boundaryDelimiter(with: boundary)
+    if let chunkIndex = range(of: boundaryDelimiter, options: .backwards)?.lowerBound {
+      return chunkIndex
+    }
+
+    return nil
+  }
 }

Sources/Apollo/URLSessionClient.swift

@@ -292,23 +292,28 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
     taskData.append(additionalData: data)
 
     if let httpResponse = dataTask.response as? HTTPURLResponse, httpResponse.isMultipart {
-      let multipartHeaderComponents = httpResponse.multipartHeaderComponents
-      guard let boundaryString = multipartHeaderComponents.boundary else {
+      guard let boundary = httpResponse.multipartHeaderComponents.boundary else {
         taskData.completionBlock(.failure(URLSessionClientError.missingMultipartBoundary))
         return
       }
 
-      let boundaryMarker = "--\(boundaryString)"
+      // Parsing Notes:
+      //
+      // Multipart messages are parsed here only to look for complete chunks to pass on to the downstream
+      // parsers. Any leftover data beyond a delimited chunk is held back for more data to arrive.
+      //
+      // Do not return `.failure` here simply because there was no boundary delimiter found; the
+      // data may still be arriving. If the request ends without more data arriving it will get handled
+      // in urlSession(_:task:didCompleteWithError:).
       guard
-        let dataString = String(data: taskData.data, encoding: .utf8)?.trimmingCharacters(in: .newlines),
-        let lastBoundaryIndex = dataString.range(of: boundaryMarker, options: .backwards)?.upperBound,
-        let boundaryData = dataString.prefix(upTo: lastBoundaryIndex).data(using: .utf8)
+        let dataString = String(data: taskData.data, encoding: .utf8),
+        let lastBoundaryDelimiterIndex = dataString.multipartRange(using: boundary),
+        let boundaryData = dataString.prefix(upTo: lastBoundaryDelimiterIndex).data(using: .utf8)
       else {
-        taskData.completionBlock(.failure(URLSessionClientError.cannotParseBoundaryData))
         return
       }
 
-      let remainingData = dataString.suffix(from: lastBoundaryIndex).data(using: .utf8)
+      let remainingData = dataString.suffix(from: lastBoundaryDelimiterIndex).data(using: .utf8)
       taskData.reset(data: remainingData)
 
       if let rawCompletion = taskData.rawCompletion {