Skip to content

Conversation

@k5342
Copy link
Contributor

@k5342 k5342 commented Jan 23, 2025

What changes were proposed in this pull request?

S3MultipartUploadCompleteRequestWithFSO seems to throw NPE when an empty file is being overwritten by non-zero file because omBucketInfo allows null value (null value is passed when non-update needed). This patch fix this by checking omBucketInfo before use.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12131

How was this patch tested?

I've added annotation for @Nullable.
Local compilation are passed and it'll be checked by CI. Let me know if more tests are suggested.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @k5342 for reporting this bug and providing the fix.

For reference, this is where bucket info is set to null to prevent unnecessary update for empty file:

} else if (!isNamespaceUpdate) {
// If no bucket size and Namespace changed, prevent from updating
// bucket object.
omBucketInfo = null;

Let me know if more tests are suggested

I would like to suggest adding this simple robot test. I reproduced the NPE with it, and verified the fix.

diff --git hadoop-ozone/dist/src/main/smoketest/s3/MultipartUpload.robot hadoop-ozone/dist/src/main/smoketest/s3/MultipartUpload.robot
index e630fe6cda..22e26953f1 100644
--- hadoop-ozone/dist/src/main/smoketest/s3/MultipartUpload.robot
+++ hadoop-ozone/dist/src/main/smoketest/s3/MultipartUpload.robot
@@ -61,6 +61,12 @@ Test Multipart Upload With Adjusted Length
     Perform Multipart Upload    ${BUCKET}    multipart/adjusted_length_${PREFIX}    /tmp/part1    /tmp/part2
     Verify Multipart Upload     ${BUCKET}    multipart/adjusted_length_${PREFIX}    /tmp/part1    /tmp/part2
 
+Overwrite Empty File
+    Execute    touch ${TEMP_DIR}/empty
+    Execute AWSS3Cli        cp ${TEMP_DIR}/empty s3://${BUCKET}/empty_file_${PREFIX}
+    Perform Multipart Upload    ${BUCKET}    empty_file_${PREFIX}    /tmp/part1    /tmp/part2
+    Verify Multipart Upload     ${BUCKET}    empty_file_${PREFIX}    /tmp/part1    /tmp/part2
+
 Test Multipart Upload
     ${uploadID} =       Initiate MPU    ${BUCKET}    ${PREFIX}/multipartKey
     ${nextUploadID} =   Initiate MPU    ${BUCKET}    ${PREFIX}/multipartKey

@adoroszlai adoroszlai added s3 S3 Gateway om labels Jan 23, 2025
@adoroszlai adoroszlai merged commit db059c6 into apache:master Jan 24, 2025
42 checks passed
@k5342 k5342 deleted the hdds-12131 branch January 24, 2025 06:49
nandakumar131 pushed a commit to nandakumar131/ozone that referenced this pull request Feb 10, 2025
adoroszlai pushed a commit to adoroszlai/ozone that referenced this pull request Mar 21, 2025
adoroszlai pushed a commit to adoroszlai/ozone that referenced this pull request Mar 21, 2025
Cyrill pushed a commit to Cyrill/ozone that referenced this pull request Nov 10, 2025
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

om s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants