chore(storage): update storage path validation to include empty/white spaces (#3587)

phantumcode · web-flow · commit 8c85096ccea8 · 2024-03-28T14:55:10.000-05:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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`")
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ extension StoragePath {`
`20`	`20`	`nil`
`21`	`21`	`)`
`22`	`22`	`}`
`23`		`- let path = resolve(identityId)`
	`23`	`+ let path = resolve(identityId).trimmingCharacters(in: .whitespaces)`
`24`	`24`	`try validate(path)`
`25`	`25`	`return path`
`26`	`26`	`} else if self is StringStoragePath {`
`@@ -30,7 +30,7 @@ extension StoragePath {`
`30`	`30`	`nil`
`31`	`31`	`)`
`32`	`32`	`}`
`33`		`- let path = resolve(input)`
	`33`	`+ let path = resolve(input).trimmingCharacters(in: .whitespaces)`
`34`	`34`	`try validate(path)`
`35`	`35`	`return path`
`36`	`36`	`} else {`
`@@ -41,6 +41,12 @@ extension StoragePath {`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`func validate(_ path: String) throws {`
	`44`	`+ guard !path.isEmpty else {`
	`45`	`+ let errorDescription = "Invalid StoragePath specified."`
	`46`	`+ let recoverySuggestion = "Please specify a valid StoragePath"`
	`47`	`+ throw StorageError.validation("path", errorDescription, recoverySuggestion, nil)`
	`48`	`+ }`
	`49`	`+`
`44`	`50`	`if path.hasPrefix("/") {`
`45`	`51`	`let errorDescription = "Invalid StoragePath specified."`
`46`	`52`	`let recoverySuggestion = "Please specify a valid StoragePath that does not contain the prefix / "`