diff --git a/.github/workflows/build_amplify_swift_platforms.yml b/.github/workflows/build_amplify_swift_platforms.yml index 2d8c6ce5c5..1e943a73ad 100644 --- a/.github/workflows/build_amplify_swift_platforms.yml +++ b/.github/workflows/build_amplify_swift_platforms.yml @@ -27,10 +27,10 @@ on: required: true default: true type: boolean - push: - branches-ignore: - - main - - release + # push: + # branches-ignore: + # - main + # - release permissions: contents: read diff --git a/.github/workflows/fortify_scan.yml b/.github/workflows/fortify_scan.yml index 5d9e6184fb..4191e8fd3d 100644 --- a/.github/workflows/fortify_scan.yml +++ b/.github/workflows/fortify_scan.yml @@ -7,10 +7,10 @@ on: required: true type: string - push: - branches-ignore: - - main - - release + # push: + # branches-ignore: + # - main + # - release permissions: id-token: write diff --git a/.github/workflows/unit_test.yml b/.github/workflows/unit_test.yml index 008683f254..525b3b2b1c 100644 --- a/.github/workflows/unit_test.yml +++ b/.github/workflows/unit_test.yml @@ -28,10 +28,10 @@ on: required: true default: true type: boolean - push: - branches-ignore: - - main - - release + # push: + # branches-ignore: + # - main + # - release permissions: contents: read diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Dependency/UploadPartInput+presignURL.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Dependency/UploadPartInput+presignURL.swift index 11d7c02789..fa3e887fd0 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Dependency/UploadPartInput+presignURL.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Dependency/UploadPartInput+presignURL.swift @@ -51,7 +51,7 @@ extension UploadPartInput { } builder.interceptors.add(ClientRuntime.URLPathMiddleware(UploadPartInput.urlPathProvider(_:))) builder.interceptors.add(ClientRuntime.URLHostMiddleware()) - builder.deserialize(ClientRuntime.DeserializeMiddleware(UploadPartOutput.httpOutput(from:), PutObjectOutputError.httpError(from:))) + builder.deserialize(ClientRuntime.DeserializeMiddleware(UploadPartOutput.httpOutput(from:), UploadPartOutputError.httpError(from:))) builder.interceptors.add(ClientRuntime.LoggerMiddleware(clientLogMode: config.clientLogMode)) builder.retryStrategy(SmithyRetries.DefaultRetryStrategy(options: config.retryStrategyOptions)) builder.retryErrorInfoProvider(AWSClientRuntime.AWSRetryErrorInfoProvider.errorInfo(for:)) @@ -62,6 +62,7 @@ extension UploadPartInput { builder.selectAuthScheme(ClientRuntime.AuthSchemeMiddleware()) builder.interceptors.add(AWSClientRuntime.AWSS3ErrorWith200StatusXMLMiddleware()) builder.interceptors.add(AWSClientRuntime.FlexibleChecksumsRequestMiddleware(checksumAlgorithm: input.checksumAlgorithm?.rawValue)) + builder.serialize(UploadPartPresignedMiddleware()) var metricsAttributes = Smithy.Attributes() metricsAttributes.set(key: ClientRuntime.OrchestratorMetricsAttributesKeys.service, value: "S3") metricsAttributes.set(key: ClientRuntime.OrchestratorMetricsAttributesKeys.method, value: "UploadPart") @@ -76,33 +77,35 @@ extension UploadPartInput { .build() return try await op.presignRequest(input: input).endpoint.url } - - static func urlPathProvider(_ value: UploadPartInput) -> Swift.String? { - guard let key = value.key else { - return nil - } - return "/\(key.urlPercentEncoding(encodeForwardSlash: false))" - } - } -extension UploadPartInput { +struct UploadPartPresignedMiddleware: Smithy.RequestMessageSerializer { + typealias InputType = UploadPartInput + typealias RequestType = SmithyHTTPAPI.HTTPRequest + + let id: Swift.String = "UploadPartPresignedMiddleware" - static func queryItemProvider(_ value: UploadPartInput) throws -> [URIQueryItem] { - var items = [URIQueryItem]() - items.append(URIQueryItem(name: "x-id", value: "UploadPart")) - guard let partNumber = value.partNumber else { + func apply(input: InputType, builder: SmithyHTTPAPI.HTTPRequestBuilder, attributes: Smithy.Context) throws { + builder.withQueryItem(.init( + name: "x-id", + value: "UploadPart") + ) + guard let partNumber = input.partNumber else { let message = "Creating a URL Query Item failed. partNumber is required and must not be nil." - throw ClientError.unknownError(message) + throw ClientError.invalidValue(message) } - let partNumberQueryItem = URIQueryItem(name: "partNumber".urlPercentEncoding(), value: Swift.String(partNumber).urlPercentEncoding()) - items.append(partNumberQueryItem) - guard let uploadId = value.uploadId else { + builder.withQueryItem(.init( + name: "partNumber".urlPercentEncoding(), + value: Swift.String(partNumber).urlPercentEncoding()) + ) + + guard let uploadId = input.uploadId else { let message = "Creating a URL Query Item failed. uploadId is required and must not be nil." - throw ClientError.unknownError(message) + throw ClientError.invalidValue(message) } - let uploadIdQueryItem = URIQueryItem(name: "uploadId".urlPercentEncoding(), value: Swift.String(uploadId).urlPercentEncoding()) - items.append(uploadIdQueryItem) - return items + builder.withQueryItem(.init( + name: "uploadId".urlPercentEncoding(), + value: Swift.String(uploadId).urlPercentEncoding()) + ) } } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageTransferResponse.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageTransferResponse.swift index 56d42506b0..606873f984 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageTransferResponse.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StorageTransferResponse.swift @@ -72,6 +72,9 @@ class StorageTransferResponse { } else if 400 == statusCode { // 400 is a bad request result = false + } else if 403 == statusCode { + // 403 is due to lack of permissions + result = false } else if (transferTask.responseText ?? "").isEmpty { // If we didn't get any more info from the server, error is retriable result = true diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginBasicIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginBasicIntegrationTests.swift index b6fee98ea5..d8afa47f97 100644 --- a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginBasicIntegrationTests.swift +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginBasicIntegrationTests.swift @@ -63,8 +63,10 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { let key = UUID().uuidString let data = Data(key.utf8) - _ = try await Amplify.Storage.uploadData(key: key, data: data, options: nil).value - _ = try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 5) { + _ = try await Amplify.Storage.uploadData(key: key, data: data, options: nil).value + _ = try await Amplify.Storage.remove(key: key) + } // Only the remove operation results in an SDK request XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method } , [.delete]) @@ -80,8 +82,10 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { func testUploadEmptyData() async throws { let key = UUID().uuidString let data = Data("".utf8) - _ = try await Amplify.Storage.uploadData(key: key, data: data, options: nil).value - _ = try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 5) { + _ = try await Amplify.Storage.uploadData(key: key, data: data, options: nil).value + _ = try await Amplify.Storage.remove(key: key) + } XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) @@ -97,8 +101,10 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { let fileURL = URL(fileURLWithPath: filePath) FileManager.default.createFile(atPath: filePath, contents: Data(key.utf8), attributes: nil) - _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value - _ = try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 5) { + _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(key: key) + } // Only the remove operation results in an SDK request XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method} , [.delete]) @@ -117,8 +123,10 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { let fileURL = URL(fileURLWithPath: filePath) FileManager.default.createFile(atPath: filePath, contents: Data("".utf8), attributes: nil) - _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value - _ = try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 5) { + _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(key: key) + } XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) @@ -130,12 +138,13 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { func testUploadLargeData() async throws { let key = UUID().uuidString - let uploadKey = try await Amplify.Storage.uploadData(key: key, - data: AWSS3StoragePluginTestBase.largeDataObject, - options: nil).value - XCTAssertEqual(uploadKey, key) - - try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 10) { + let uploadKey = try await Amplify.Storage.uploadData(key: key, + data: AWSS3StoragePluginTestBase.largeDataObject, + options: nil).value + XCTAssertEqual(uploadKey, key) + try await Amplify.Storage.remove(key: key) + } let userAgents = requestRecorder.urlRequests.compactMap { $0.allHTTPHeaderFields?["User-Agent"] } XCTAssertGreaterThan(userAgents.count, 1) @@ -157,8 +166,10 @@ class AWSS3StoragePluginBasicIntegrationTests: AWSS3StoragePluginTestBase { contents: AWSS3StoragePluginTestBase.largeDataObject, attributes: nil) - _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value - _ = try await Amplify.Storage.remove(key: key) + await waitTasks(timeout: 10) { + _ = try await Amplify.Storage.uploadFile(key: key, local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(key: key) + } let userAgents = requestRecorder.urlRequests.compactMap { $0.allHTTPHeaderFields?["User-Agent"] } XCTAssertGreaterThan(userAgents.count, 1) diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginTestBase.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginTestBase.swift index 35e4721be2..9a2392d18e 100644 --- a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginTestBase.swift +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginTestBase.swift @@ -194,6 +194,20 @@ class AWSS3StoragePluginTestBase: XCTestCase { } } + func waitTasks(timeout: TimeInterval, closure: @escaping () async throws -> ()) async { + let expectation = expectation(description: "Operation expectation") + Task { + defer { expectation.fulfill() } + do { + try await closure() + } catch { + XCTFail("Unexpected error: \(error)") + } + } + + await fulfillment(of: [expectation], timeout: timeout) + } + private func invalidateCurrentSession() { Self.logger.debug("Invalidating URLSession") guard let plugin = try? Amplify.Storage.getPlugin(for: "awsS3StoragePlugin") as? AWSS3StoragePlugin,