Răsfoiți Sursa

Don't read downloaded files during validation (#3899)

### Issue Link :link:
An alternate take on #3888.

### Goals :soccer:
This PR modifies `DownloadRequest` validation to read the file size from
disk, rather than reading the whole file. This prevents large memory
spikes when using downloads.

### Implementation Details :construction:
Rather than reading the `Data`, validation now reads the `.fileSizeKey`
resource value.

### Testing Details :mag:
No new tests, existing tests pass.
Jon Shier 1 an în urmă
părinte
comite
079dc27206
3 a modificat fișierele cu 21 adăugiri și 8 ștergeri
  1. 1 0
      Package.swift
  2. 12 0
      Source/Core/DownloadRequest.swift
  3. 8 8
      Source/Features/Validation.swift

+ 1 - 0
Package.swift

@@ -38,6 +38,7 @@ let package = Package(name: "Alamofire",
                                         path: "Source",
                                         exclude: ["Info.plist"],
                                         resources: [.process("PrivacyInfo.xcprivacy")],
+                                        swiftSettings: [.enableUpcomingFeature("ExistentialAny")],
                                         linkerSettings: [.linkedFramework("CFNetwork",
                                                                           .when(platforms: [.iOS,
                                                                                             .macOS,

+ 12 - 0
Source/Core/DownloadRequest.swift

@@ -433,6 +433,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - queue:              The queue on which the completion handler is dispatched. `.main` by default.
     ///   - responseSerializer: The response serializer responsible for serializing the request, response, and data
@@ -450,6 +452,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - queue:              The queue on which the completion handler is dispatched. `.main` by default.
     ///   - responseSerializer: The response serializer responsible for serializing the request, response, and data
@@ -481,6 +485,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler using a `DataResponseSerializer` to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - queue:               The queue on which the completion handler is called. `.main` by default.
     ///   - dataPreprocessor:    `DataPreprocessor` which processes the received `Data` before calling the
@@ -506,6 +512,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler using a `StringResponseSerializer` to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - queue:               The queue on which the completion handler is dispatched. `.main` by default.
     ///   - dataPreprocessor:    `DataPreprocessor` which processes the received `Data` before calling the
@@ -535,6 +543,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler using a `JSONResponseSerializer` to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - queue:               The queue on which the completion handler is dispatched. `.main` by default.
     ///   - dataPreprocessor:    `DataPreprocessor` which processes the received `Data` before calling the
@@ -565,6 +575,8 @@ public final class DownloadRequest: Request, @unchecked Sendable {
 
     /// Adds a handler using a `DecodableResponseSerializer` to be called once the request has finished.
     ///
+    /// - Note: This handler will read the entire downloaded file into memory, use with caution.
+    ///
     /// - Parameters:
     ///   - type:                `Decodable` type to decode from response data.
     ///   - queue:               The queue on which the completion handler is dispatched. `.main` by default.

+ 8 - 8
Source/Features/Validation.swift

@@ -94,10 +94,10 @@ extension Request {
 
     fileprivate func validate<S: Sequence>(contentType acceptableContentTypes: S,
                                            response: HTTPURLResponse,
-                                           data: Data?)
+                                           isEmpty: Bool)
         -> ValidationResult
         where S.Iterator.Element == String {
-        guard let data, !data.isEmpty else { return .success(()) }
+        guard !isEmpty else { return .success(()) }
 
         return validate(contentType: acceptableContentTypes, response: response)
     }
@@ -172,7 +172,7 @@ extension DataRequest {
     @discardableResult
     public func validate<S: Sequence>(contentType acceptableContentTypes: @escaping @autoclosure () -> S) -> Self where S.Iterator.Element == String {
         validate { [unowned self] _, response, data in
-            self.validate(contentType: acceptableContentTypes(), response: response, data: data)
+            self.validate(contentType: acceptableContentTypes(), response: response, isEmpty: (data == nil || data?.isEmpty == true))
         }
     }
 
@@ -263,7 +263,7 @@ extension DownloadRequest {
         }
     }
 
-    /// Validates that the response has a content type in the specified sequence.
+    /// Validates that the response has a `Content-Type` in the specified sequence.
     ///
     /// If validation fails, subsequent calls to response handlers will have an associated error.
     ///
@@ -273,15 +273,15 @@ extension DownloadRequest {
     @discardableResult
     public func validate<S: Sequence>(contentType acceptableContentTypes: @escaping @autoclosure () -> S) -> Self where S.Iterator.Element == String {
         validate { [unowned self] _, response, fileURL in
-            guard let validFileURL = fileURL else {
+            guard let fileURL else {
                 return .failure(AFError.responseValidationFailed(reason: .dataFileNil))
             }
 
             do {
-                let data = try Data(contentsOf: validFileURL)
-                return self.validate(contentType: acceptableContentTypes(), response: response, data: data)
+                let isEmpty = try (fileURL.resourceValues(forKeys: [.fileSizeKey]).fileSize ?? 0) == 0
+                return self.validate(contentType: acceptableContentTypes(), response: response, isEmpty: isEmpty)
             } catch {
-                return .failure(AFError.responseValidationFailed(reason: .dataFileReadFailed(at: validFileURL)))
+                return .failure(AFError.responseValidationFailed(reason: .dataFileReadFailed(at: fileURL)))
             }
         }
     }