Fix uploading empty files with multipart (#659)

soto-project · Jan 31, 2023 · 37d8d29 · 37d8d29
1 parent 898a859
commit 37d8d29
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 3 deletions.
diff --git a/Sources/Soto/Extensions/S3/S3+multipart+async.swift b/Sources/Soto/Extensions/S3/S3+multipart+async.swift
@@ -637,7 +637,10 @@ extension S3 {
                 }
 
                 let payload = try await inputStream(eventLoop)
-                guard let size = payload.size, size > 0 else {
+                // if no data returned then break out of loop. If this is the first part
+                // and it is empty then that means the entire file is empty. In that
+                // case, we do still "upload" this first empty part.
+                guard let size = payload.size, size > 0 || partNumber == 1 else {
                     break
                 }
 

diff --git a/Sources/Soto/Extensions/S3/S3+multipart.swift b/Sources/Soto/Extensions/S3/S3+multipart.swift
@@ -694,8 +694,10 @@ extension S3 {
             } else {
                 // supply payload data
                 inputStream(eventLoop).flatMap { payload -> EventLoopFuture<Bool> in
-                    // if no data returned then return success
-                    guard let size = payload.size, size > 0 else {
+                    // if no data returned then return success. If this is the first part
+                    // and it is empty then that means the entire file is empty. In that
+                    // case, we do still "upload" this first empty part.
+                    guard let size = payload.size, size > 0 || partNumber == 1 else {
                         return eventLoop.makeSucceededFuture(true)
                     }
                     // upload payload

diff --git a/Tests/SotoTests/Services/S3/S3ExtensionTests.swift b/Tests/SotoTests/Services/S3/S3ExtensionTests.swift
@@ -236,6 +236,46 @@ class S3ExtensionTests: XCTestCase {
         XCTAssertNoThrow(try response.wait())
     }
 
+    func testMultiPartUploadEmpty() {
+        let s3 = Self.s3.with(timeout: .minutes(2))
+        let data = Data() // Empty
+        let name = TestEnvironment.generateResourceName()
+        let filenameUpload = "S3MultipartUploadTestEmpty"
+        let filenameDownload = "S3MultipartUploadTestEmpty-Downloaded"
+
+        XCTAssertNoThrow(try data.write(to: URL(fileURLWithPath: filenameUpload)))
+        defer {
+            XCTAssertNoThrow(try FileManager.default.removeItem(atPath: filenameUpload))
+        }
+
+        let response = S3Tests.createBucket(name: name, s3: s3)
+            .flatMap { _ -> EventLoopFuture<S3.CompleteMultipartUploadOutput> in
+                let request = S3.CreateMultipartUploadRequest(
+                    bucket: name,
+                    key: name
+                )
+                return s3.multipartUpload(request, partSize: 5 * 1024 * 1024, filename: filenameUpload, logger: TestEnvironment.logger) { print("Progress \($0 * 100)%") }
+            }
+            .flatMap { _ -> EventLoopFuture<Int64> in
+                // Download the empty file
+                let request = S3.GetObjectRequest(bucket: name, key: name)
+                return s3.multipartDownload(request, partSize: 1024 * 1024, filename: filenameDownload, logger: TestEnvironment.logger) { print("Progress \($0 * 100)%") }
+            }
+            .flatMapErrorThrowing { error in
+                print("\(error)")
+                throw error
+            }
+            .flatMapThrowing { size in
+                XCTAssertEqual(size, 0) // Empty
+                XCTAssert(FileManager.default.fileExists(atPath: filenameDownload))
+                try FileManager.default.removeItem(atPath: filenameDownload)
+            }
+            .flatAlways { _ in
+                return S3Tests.deleteBucket(name: name, s3: s3)
+            }
+        XCTAssertNoThrow(try response.wait())
+    }
+
     func testMultiPartUploadFailure() {
         let data = S3Tests.createRandomBuffer(size: 10 * 1024 * 1024)
         let name = TestEnvironment.generateResourceName()

diff --git a/Tests/SotoTests/Services/S3/S3Tests+async.swift b/Tests/SotoTests/Services/S3/S3Tests+async.swift
@@ -578,6 +578,29 @@ class S3AsyncTests: XCTestCase {
         }
     }
 
+    func testMultiPartEmptyUploadAsync() async throws {
+        let s3 = Self.s3.with(timeout: .minutes(2))
+        let data = Data() // Empty
+        let name = TestEnvironment.generateResourceName()
+        let filename = "testMultiPartEmptyUploadAsync"
+
+        XCTAssertNoThrow(try data.write(to: URL(fileURLWithPath: filename)))
+        defer {
+            XCTAssertNoThrow(try FileManager.default.removeItem(atPath: filename))
+        }
+
+        try await self.s3Test(bucket: name) {
+            let request = S3.CreateMultipartUploadRequest(
+                bucket: name,
+                key: name
+            )
+            _ = try await s3.multipartUpload(request, partSize: 5 * 1024 * 1024, filename: filename, logger: TestEnvironment.logger) { print("Progress \($0 * 100)%") }
+
+            let download = try await s3.getObject(.init(bucket: name, key: name), logger: TestEnvironment.logger)
+            XCTAssert(download.body?.isEmpty != false)
+        }
+    }
+
     func testResumeMultiPartUploadAsync() async throws {
         struct CancelError: Error {}
         let s3 = Self.s3.with(timeout: .minutes(2))