peterxcli commented on code in PR #8506:
URL: https://github.com/apache/ozone/pull/8506#discussion_r2105860626
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestMultipartUploadComplete.java:
##########
@@ -266,4 +270,96 @@ public void testMultipartInvalidPartError() throws
Exception {
() -> completeMultipartUpload(key, completeMultipartUploadRequest,
uploadID));
assertEquals(ex.getCode(), S3ErrorTable.INVALID_PART.getCode());
}
+
+ @Test
+ public void testMultipartCompleteWithEtagMismatchShouldAbortAndThrow()
throws Exception {
+ // Arrange
+ String key = UUID.randomUUID().toString();
+ String uploadID = initiateMultipartUpload(key);
+ CompleteMultipartUploadRequest completeMultipartUploadRequest =
+ buildCompleteMultipartUploadRequest(key, uploadID);
+
+ // Mock headers to return a mismatched etag
+ HttpHeaders etagHeaders = mock(HttpHeaders.class);
+ String fakeMd5Hex = "deadbeefdeadbeefdeadbeefdeadbeef";
+ byte[] fakeMd5Bytes = Hex.decodeHex(fakeMd5Hex);
+ String encodedEtag = Base64.getEncoder().encodeToString(fakeMd5Bytes);
+ when(etagHeaders.getHeaderString(CHECKSUM_HEADER)).thenReturn(
+ encodedEtag);
+ rest.setHeaders(etagHeaders);
+
+ // Act & Assert
+ OS3Exception os3Exception = assertThrows(OS3Exception.class,
+ () -> rest.completeMultipartUpload(OzoneConsts.S3_BUCKET, key,
uploadID, completeMultipartUploadRequest));
+ assertEquals(S3ErrorTable.BAD_DIGEST.getCode(), os3Exception.getCode());
+ assertEquals(HTTP_BAD_REQUEST, os3Exception.getHttpCode());
+ }
+
+ @Test
+ public void testMultipartCompleteWithEtagMatchShouldSucceed() throws
Exception {
+ // Arrange
+ String key = UUID.randomUUID().toString();
+ String uploadID = initiateMultipartUpload(key);
+ CompleteMultipartUploadRequest completeMultipartUploadRequest =
+ buildCompleteMultipartUploadRequest(key, uploadID);
+
+ // calculate correct etag
+ Response tempResp =
+ rest.completeMultipartUpload(OzoneConsts.S3_BUCKET, key, uploadID,
completeMultipartUploadRequest);
Review Comment:
Is the duplicate(line 308, 321) completeMultipartUpload request ok? Wouldn't
the second one be ignored or skipped?
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java:
##########
@@ -758,4 +768,89 @@ public void testPutEmptyObject() throws IOException,
OS3Exception {
OzoneKeyDetails keyDetails =
clientStub.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_NAME);
assertEquals(0, keyDetails.getDataSize());
}
+
+ @Test
+ void testPutObjectWithEtagMismatchShouldCleanupAndThrow() throws
IOException, OS3Exception, DecoderException {
+ // Arrange
+ String content = "test-content";
+ ByteArrayInputStream body = new
ByteArrayInputStream(content.getBytes(UTF_8));
+
bucket.setReplicationConfig(RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE));
+
+ // Mock headers to return a mismatched checksum
+ HttpHeaders etagHeaders = mock(HttpHeaders.class);
+ String fakeMd5Hex = "deadbeefdeadbeefdeadbeefdeadbeef";
+ byte[] fakeMd5Bytes = Hex.decodeHex(fakeMd5Hex);
+ String encodedEtag = Base64.getEncoder().encodeToString(fakeMd5Bytes);
+ when(etagHeaders.getHeaderString(CHECKSUM_HEADER)).thenReturn(encodedEtag);
+
when(etagHeaders.getHeaderString(X_AMZ_CONTENT_SHA256)).thenReturn(UNSIGNED_PAYLOAD);
+ objectEndpoint.setHeaders(etagHeaders);
+
+ // Spy on objectEndpoint.delete to verify cleanup is called
+ ObjectEndpoint spyEndpoint = spy(objectEndpoint);
+ doReturn(Response.ok().build()).when(spyEndpoint).delete(any(), any(),
any(), any());
+
+ // Act
+ OS3Exception os3Exception = assertThrows(OS3Exception.class, () ->
+ spyEndpoint.put(BUCKET_NAME, KEY_NAME, content.length(), 1,
+ null, null, null, body));
+
+ // Assert
+ verify(spyEndpoint, times(1)).delete(any(), any(), any(), any());
+ assertEquals(BAD_DIGEST.getCode(), os3Exception.getCode());
+ assertEquals(HTTP_BAD_REQUEST, os3Exception.getHttpCode());
+ }
+
+ @Test
+ void testPutObjectWithEtagMatchShouldNotCleanupOrThrow() throws IOException,
OS3Exception, DecoderException {
+ // Arrange
+ String content = "test-content-match";
+ ByteArrayInputStream body = new
ByteArrayInputStream(content.getBytes(UTF_8));
+
bucket.setReplicationConfig(RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE));
+
+ // get server etag
+ ObjectEndpoint spyEndpoint = spy(objectEndpoint);
+ doReturn(Response.ok().build()).when(spyEndpoint).delete(any(), any(),
any(), any());
+ Response tempResp = spyEndpoint.put(BUCKET_NAME, KEY_NAME,
content.length(), 1, null,
Review Comment:
> Is the duplicate(line 308, 321) completeMultipartUpload request ok?
Wouldn't the second one be ignored or skipped?
Same question as I left in TestMultipartUploadComplete. Please correct me if
I'm wrong.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestMultipartUploadComplete.java:
##########
@@ -266,4 +270,96 @@ public void testMultipartInvalidPartError() throws
Exception {
() -> completeMultipartUpload(key, completeMultipartUploadRequest,
uploadID));
assertEquals(ex.getCode(), S3ErrorTable.INVALID_PART.getCode());
}
+
+ @Test
+ public void testMultipartCompleteWithEtagMismatchShouldAbortAndThrow()
throws Exception {
+ // Arrange
+ String key = UUID.randomUUID().toString();
+ String uploadID = initiateMultipartUpload(key);
+ CompleteMultipartUploadRequest completeMultipartUploadRequest =
+ buildCompleteMultipartUploadRequest(key, uploadID);
+
+ // Mock headers to return a mismatched etag
+ HttpHeaders etagHeaders = mock(HttpHeaders.class);
+ String fakeMd5Hex = "deadbeefdeadbeefdeadbeefdeadbeef";
+ byte[] fakeMd5Bytes = Hex.decodeHex(fakeMd5Hex);
+ String encodedEtag = Base64.getEncoder().encodeToString(fakeMd5Bytes);
+ when(etagHeaders.getHeaderString(CHECKSUM_HEADER)).thenReturn(
+ encodedEtag);
+ rest.setHeaders(etagHeaders);
+
+ // Act & Assert
+ OS3Exception os3Exception = assertThrows(OS3Exception.class,
+ () -> rest.completeMultipartUpload(OzoneConsts.S3_BUCKET, key,
uploadID, completeMultipartUploadRequest));
Review Comment:
Should also check the multipart upload has been aborted and cleaned up.
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java:
##########
@@ -758,4 +768,89 @@ public void testPutEmptyObject() throws IOException,
OS3Exception {
OzoneKeyDetails keyDetails =
clientStub.getObjectStore().getS3Bucket(BUCKET_NAME).getKey(KEY_NAME);
assertEquals(0, keyDetails.getDataSize());
}
+
+ @Test
+ void testPutObjectWithEtagMismatchShouldCleanupAndThrow() throws
IOException, OS3Exception, DecoderException {
+ // Arrange
+ String content = "test-content";
+ ByteArrayInputStream body = new
ByteArrayInputStream(content.getBytes(UTF_8));
Review Comment:
```suggestion
ByteArrayInputStream body = new
ByteArrayInputStream(CONTENT.getBytes(UTF_8));
```
same as other test cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
