From 37d8d296c71f4738600afbf549bb74c28ea192a6 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Tue, 31 Jan 2023 18:32:54 +0100 Subject: [PATCH] Fix uploading empty files with multipart (#659) --- .../Extensions/S3/S3+multipart+async.swift | 5 ++- Sources/Soto/Extensions/S3/S3+multipart.swift | 6 ++- .../Services/S3/S3ExtensionTests.swift | 40 +++++++++++++++++++ .../SotoTests/Services/S3/S3Tests+async.swift | 23 +++++++++++ 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/Sources/Soto/Extensions/S3/S3+multipart+async.swift b/Sources/Soto/Extensions/S3/S3+multipart+async.swift index 546d1e66667..9882b2ee284 100644 --- 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 index cf548068bd5..44561c22389 100644 --- 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 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 index db4895f7b67..9e9c5d88e7a 100644 --- 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 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 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 index 8ac67ce0078..0d457f46583 100644 --- 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))