From 022ccb8566e1a7cf8aff8ab26c81034bf54f1d69 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Thu, 28 Mar 2024 14:55:10 -0500 Subject: [PATCH] chore(storage): update storage path validation to include empty/white spaces (#3587) --- .../Internal/StoragePath+Extensions.swift | 10 +++- ...SS3StorageDownloadFileOperationTests.swift | 32 +++++++++++++ .../AWSS3StorageGetDataOperationTests.swift | 30 ++++++++++++ .../AWSS3StoragePutDataOperationTests.swift | 31 +++++++++++++ ...AWSS3StorageUploadFileOperationTests.swift | 46 +++++++++++++++++++ .../Tasks/AWSS3StorageGetURLTaskTests.swift | 32 +++++++++++++ .../AWSS3StorageListObjectsTaskTests.swift | 26 +++++++++++ .../Tasks/AWSS3StorageRemoveTaskTests.swift | 26 +++++++++++ 8 files changed, 231 insertions(+), 2 deletions(-) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift index bdc2ebece4..482190be32 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -20,7 +20,7 @@ extension StoragePath { nil ) } - let path = resolve(identityId) + let path = resolve(identityId).trimmingCharacters(in: .whitespaces) try validate(path) return path } else if self is StringStoragePath { @@ -30,7 +30,7 @@ extension StoragePath { nil ) } - let path = resolve(input) + let path = resolve(input).trimmingCharacters(in: .whitespaces) try validate(path) return path } else { @@ -41,6 +41,12 @@ extension StoragePath { } func validate(_ path: String) throws { + guard !path.isEmpty else { + let errorDescription = "Invalid StoragePath specified." + let recoverySuggestion = "Please specify a valid StoragePath" + throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) + } + if path.hasPrefix("/") { let errorDescription = "Invalid StoragePath specified." let recoverySuggestion = "Please specify a valid StoragePath that does not contain the prefix / " diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift index 98b15f5954..9b1b6b6d65 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift @@ -214,6 +214,38 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Download File Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testDownloadFileOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Download File Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift index 23a28ee935..6a7c7a5485 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift @@ -204,6 +204,36 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Download Data Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testDownloadDataOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Download Data Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift index faeb9fc862..5934e419d3 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift @@ -250,6 +250,37 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Upload Data Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testUploadDataOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let failedInvoked = expectation(description: "failed was invoked on operation") + let options = StorageUploadDataRequest.Options(accessLevel: .protected) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Upload Data Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift index 87183415e6..12374173bb 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift @@ -301,6 +301,52 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Upload File Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testUploadFileOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Upload File Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift index ccd5212bc8..71f5ea6ea1 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift @@ -103,4 +103,36 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { } } + /// - Given: A configured Storage GetURL Task with invalid path + /// - When: AWSS3StorageGetURLTask value is invoked + /// - Then: A storage validation error should be returned + func testGetURLTaskWithInvalidEmptyPath() async throws { + let emptyPath = " " + let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) + + let serviceMock = MockAWSS3StorageService() + serviceMock.getPreSignedURLHandler = { path, _, _ in + XCTAssertEqual(emptyPath, path) + return tempURL + } + + let request = StorageGetURLRequest( + path: StringStoragePath.fromString(emptyPath), options: .init()) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift index 4ba7cff47c..251111abfa 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift @@ -103,4 +103,30 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { } } + /// - Given: A configured Storage ListObjects Task with invalid path + /// - When: AWSS3StorageListObjectsTask value is invoked + /// - Then: A storage validation error should be returned + func testListObjectsTaskWithInvalidEmptyPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageListRequest( + path: StringStoragePath.fromString(" "), options: .init()) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift index 047f130036..06f4ecf809 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift @@ -94,4 +94,30 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { } } + /// - Given: A configured Storage Remove Task with invalid path + /// - When: AWSS3StorageRemoveTask value is invoked + /// - Then: A storage validation error should be returned + func testRemoveTaskWithInvalidEmptyPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageRemoveRequest( + path: StringStoragePath.fromString(" "), options: .init()) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } }