Skip to content

fix(storage): Improve exception handling when attempting to overwrite a file during download #3056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.amplifyframework.storage.s3.extensions

import com.amplifyframework.storage.StorageException
import com.amplifyframework.storage.StorageFilePermissionException
import com.amplifyframework.storage.StoragePathValidationException

internal fun StoragePathValidationException.Companion.invalidStoragePathException() = StorageException(
Expand All @@ -31,3 +32,12 @@
),
"Provided StoragePath not supported by AWS S3 Storage Plugin"
)

internal fun StorageFilePermissionException.Companion.unableToOverwriteFileException() = StorageException(
"Unable to overwrite this file for download.",
StorageFilePermissionException(
"Unable to overwrite this file for download.",
"Acquire write permission for this file before attempting to overwrite it."

Check warning on line 40 in aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/extensions/StorageExceptionExtensions.kt

View check run for this annotation

Codecov / codecov/patch

aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/extensions/StorageExceptionExtensions.kt#L36-L40

Added lines #L36 - L40 were not covered by tests
),
"Acquire write permission for this file before attempting to overwrite it."

Check warning on line 42 in aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/extensions/StorageExceptionExtensions.kt

View check run for this annotation

Codecov / codecov/patch

aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/extensions/StorageExceptionExtensions.kt#L42

Added line #L42 was not covered by tests
)
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import com.amplifyframework.core.Amplify
import com.amplifyframework.core.category.CategoryType
import com.amplifyframework.storage.ObjectMetadata
import com.amplifyframework.storage.StorageFilePermissionException
import com.amplifyframework.storage.s3.AWSS3StoragePlugin
import com.amplifyframework.storage.s3.TransferOperations
import com.amplifyframework.storage.s3.extensions.unableToOverwriteFileException
import com.amplifyframework.storage.s3.transfer.worker.RouterWorker
import com.amplifyframework.storage.s3.transfer.worker.TransferWorkerFactory
import java.io.File
Expand Down Expand Up @@ -192,6 +194,9 @@
val transferRecordId: Int = uri.lastPathSegment?.toInt()
?: throw IllegalStateException("Invalid TransferRecord ID ${uri.lastPathSegment}")
if (file.isFile) {
if (!file.canWrite()) {
throw StorageFilePermissionException.unableToOverwriteFileException()

Check warning on line 198 in aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt

View check run for this annotation

Codecov / codecov/patch

aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt#L198

Added line #L198 was not covered by tests
}
logger.warn("Overwriting existing file: $file")
file.delete()
}
Expand Down
3 changes: 3 additions & 0 deletions core/api/core.api
Original file line number Diff line number Diff line change
Expand Up @@ -4212,6 +4212,9 @@ public final class com/amplifyframework/storage/StorageException : com/amplifyfr
public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/String;)V
}

public final class com/amplifyframework/storage/StorageFilePermissionException : com/amplifyframework/AmplifyException {
}

public final class com/amplifyframework/storage/StorageItem {
public fun <init> (Ljava/lang/String;JLjava/util/Date;Ljava/lang/String;Ljava/lang/Object;)V
public final fun component1 ()Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/
package com.amplifyframework.storage

import com.amplifyframework.AmplifyException
import com.amplifyframework.annotations.InternalAmplifyApi

/**
* Exception thrown when the necessary file permissions needed to handle a file have not been granted.
*/
class StorageFilePermissionException @InternalAmplifyApi constructor(
message: String,
recoverySuggestion: String
) : AmplifyException(message, recoverySuggestion) {

Check warning on line 26 in core/src/main/java/com/amplifyframework/storage/StorageFilePermissionException.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/com/amplifyframework/storage/StorageFilePermissionException.kt#L26

Added line #L26 was not covered by tests
@InternalAmplifyApi companion object
}