diff --git a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java index 53cb48e99e9..9de17faf659 100644 --- a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java +++ b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java @@ -140,6 +140,7 @@ import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; import software.amazon.awssdk.services.s3.model.ListPartsRequest; +import software.amazon.awssdk.services.s3.model.MetadataDirective; import software.amazon.awssdk.services.s3.model.NoSuchKeyException; import software.amazon.awssdk.services.s3.model.ObjectIdentifier; import software.amazon.awssdk.services.s3.model.PutBucketAclRequest; @@ -151,6 +152,7 @@ import software.amazon.awssdk.services.s3.model.S3Object; import software.amazon.awssdk.services.s3.model.Tag; import software.amazon.awssdk.services.s3.model.Tagging; +import software.amazon.awssdk.services.s3.model.TaggingDirective; import software.amazon.awssdk.services.s3.model.UploadPartCopyRequest; import software.amazon.awssdk.services.s3.model.UploadPartRequest; import software.amazon.awssdk.services.s3.model.UploadPartResponse; @@ -1204,6 +1206,69 @@ public void testCopyObject() { assertEquals("\"37b51d194a7513e45b56f6524f2d51f2\"", copyObjectResponse.copyObjectResult().eTag()); } + @Test + public void testCopyObjectToSelfWithMetadataReplace() { + final String bucketName = getBucketName(); + final String key = getKeyName(); + final String content = "bar"; + s3Client.createBucket(b -> b.bucket(bucketName)); + s3Client.putObject(b -> b.bucket(bucketName).key(key).metadata(Collections.singletonMap("meta1", "v1")), + RequestBody.fromString(content)); + + // Copying an object onto itself is allowed when the metadata is replaced. + CopyObjectRequest copyReq = CopyObjectRequest.builder() + .sourceBucket(bucketName) + .sourceKey(key) + .destinationBucket(bucketName) + .destinationKey(key) + .metadataDirective(MetadataDirective.REPLACE) + .metadata(Collections.singletonMap("meta2", "v2")) + .build(); + + CopyObjectResponse copyObjectResponse = assertDoesNotThrow(() -> s3Client.copyObject(copyReq)); + assertNotNull(copyObjectResponse.copyObjectResult().eTag()); + + // The metadata was replaced in place: the new entry is present and the old one is gone. + HeadObjectResponse head = s3Client.headObject(b -> b.bucket(bucketName).key(key)); + assertThat(head.metadata()) + .containsEntry("meta2", "v2") + .doesNotContainKey("meta1"); + + // The object is still readable with its original content after the in-place copy. + ResponseBytes objectBytes = s3Client.getObjectAsBytes( + b -> b.bucket(bucketName).key(key)); + assertEquals(content, objectBytes.asUtf8String()); + } + + @Test + public void testCopyObjectToSelfWithTaggingReplace() { + final String bucketName = getBucketName(); + final String key = getKeyName(); + final String content = "bar"; + s3Client.createBucket(b -> b.bucket(bucketName)); + s3Client.putObject(b -> b.bucket(bucketName).key(key).tagging("tag1=value1"), + RequestBody.fromString(content)); + + // Copying an object onto itself is allowed when the tag set is replaced. + CopyObjectRequest copyReq = CopyObjectRequest.builder() + .sourceBucket(bucketName) + .sourceKey(key) + .destinationBucket(bucketName) + .destinationKey(key) + .taggingDirective(TaggingDirective.REPLACE) + .tagging("tag2=value2") + .build(); + + CopyObjectResponse copyObjectResponse = assertDoesNotThrow(() -> s3Client.copyObject(copyReq)); + assertNotNull(copyObjectResponse.copyObjectResult().eTag()); + + // The tag set was replaced in place. + GetObjectTaggingResponse tagging = s3Client.getObjectTagging(b -> b.bucket(bucketName).key(key)); + assertEquals(1, tagging.tagSet().size()); + assertEquals("tag2", tagging.tagSet().get(0).key()); + assertEquals("value2", tagging.tagSet().get(0).value()); + } + @Test public void testCopyObjectWithSourceIfMatch() { final String sourceBucketName = getBucketName("source"); diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java index 04e0765a9b2..d751dd01dfb 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java @@ -1069,13 +1069,19 @@ private CopyObjectResponse copyObject(OzoneVolume volume, try { OzoneKeyDetails sourceKeyDetails = getClientProtocol().getKeyDetails( volume.getName(), sourceBucket, sourceKey); + // The tagging and metadata directives are read up front because a + // self-copy is only legal when at least one attribute is being changed. + String tagCopyDirective = getHeaders().getHeaderString(TAG_DIRECTIVE_HEADER); + String metadataCopyDirective = getHeaders().getHeaderString(CUSTOM_METADATA_COPY_DIRECTIVE_HEADER); + boolean replacingTags = CopyDirective.REPLACE.name().equals(tagCopyDirective); + boolean replacingMetadata = CopyDirective.REPLACE.name().equals(metadataCopyDirective); + // Checking whether we trying to copying to it self. - if (sourceBucket.equals(destBucket) && sourceKey - .equals(destkey)) { - // When copying to same storage type when storage type is provided, - // we should not throw exception, as aws cli checks if any of the - // options like storage type are provided or not when source and - // dest are given same + if (sourceBucket.equals(destBucket) && sourceKey.equals(destkey) + && !replacingTags && !replacingMetadata) { + // Self-copy without a metadata or tag replacement. AWS still allows it + // when a storage class is provided (aws cli passes storage type), so + // only the default-storage-type case is rejected. if (storageTypeDefault) { OS3Exception ex = newError(S3ErrorTable.INVALID_REQUEST, copyHeader); ex.setErrorMessage("This copy request is illegal because it is " + @@ -1106,7 +1112,6 @@ private CopyObjectResponse copyObject(OzoneVolume volume, // Object tagging in copyObject with tagging directive Map tags; - String tagCopyDirective = getHeaders().getHeaderString(TAG_DIRECTIVE_HEADER); if (StringUtils.isEmpty(tagCopyDirective) || tagCopyDirective.equals(CopyDirective.COPY.name())) { // Tag-set will be copied from the source directly tags = sourceKeyDetails.getTags(); @@ -1123,7 +1128,6 @@ private CopyObjectResponse copyObject(OzoneVolume volume, // Custom metadata in copyObject with metadata directive Map customMetadata; - String metadataCopyDirective = getHeaders().getHeaderString(CUSTOM_METADATA_COPY_DIRECTIVE_HEADER); if (StringUtils.isEmpty(metadataCopyDirective) || metadataCopyDirective.equals(CopyDirective.COPY.name())) { // The custom metadata will be copied from the source key customMetadata = sourceKeyDetails.getMetadata(); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java index 2af6cbcfdcc..5c4f900d588 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java @@ -382,6 +382,57 @@ void testCopyObject() throws Exception { () -> put(objectEndpoint, "nonexistent", KEY_NAME, CONTENT)); } + @Test + void testCopyObjectToSelfWithMetadataReplace() throws Exception { + // Put the source object with some custom metadata. + Map sourceMetadata = ImmutableMap.of( + "custom-key-1", "custom-value-1", + "custom-key-2", "custom-value-2"); + MultivaluedMap metadataHeaders = new MultivaluedHashMap<>(); + sourceMetadata.forEach((k, v) -> metadataHeaders.putSingle(CUSTOM_METADATA_HEADER_PREFIX + k, v)); + when(headers.getRequestHeaders()).thenReturn(metadataHeaders); + when(headers.getHeaderString(CUSTOM_METADATA_COPY_DIRECTIVE_HEADER)).thenReturn("COPY"); + assertSucceeds(() -> putObject(CONTENT)); + assertThat(bucket.getKey(KEY_NAME).getMetadata()).containsAllEntriesOf(sourceMetadata); + + // Copy the object onto itself with x-amz-metadata-directive: REPLACE and a + // new metadata set. AWS allows this as an in-place metadata update. + when(headers.getHeaderString(COPY_SOURCE_HEADER)).thenReturn( + BUCKET_NAME + "/" + urlEncode(KEY_NAME)); + when(headers.getHeaderString(CUSTOM_METADATA_COPY_DIRECTIVE_HEADER)).thenReturn("REPLACE"); + metadataHeaders.clear(); + metadataHeaders.putSingle(CUSTOM_METADATA_HEADER_PREFIX + "custom-key-3", "custom-value-3"); + + assertSucceeds(() -> putObject(CONTENT)); + + OzoneKeyDetails keyDetails = assertKeyContent(bucket, KEY_NAME, CONTENT); + assertThat(keyDetails.getMetadata()) + .containsEntry("custom-key-3", "custom-value-3") + .doesNotContainKeys("custom-key-1", "custom-key-2"); + } + + @Test + void testCopyObjectToSelfWithTaggingReplace() throws Exception { + // Put the source object with some tags. + when(headers.getHeaderString(TAG_HEADER)).thenReturn("tag1=value1&tag2=value2"); + assertSucceeds(() -> putObject(CONTENT)); + assertThat(bucket.getKey(KEY_NAME).getTags()).hasSize(2); + + // Copy the object onto itself with x-amz-tagging-directive: REPLACE and a + // new tag set. AWS allows this as an in-place tag update. + when(headers.getHeaderString(COPY_SOURCE_HEADER)).thenReturn( + BUCKET_NAME + "/" + urlEncode(KEY_NAME)); + when(headers.getHeaderString(TAG_DIRECTIVE_HEADER)).thenReturn("REPLACE"); + when(headers.getHeaderString(TAG_HEADER)).thenReturn("tag3=value3"); + + assertSucceeds(() -> putObject(CONTENT)); + + Map tags = assertKeyContent(bucket, KEY_NAME, CONTENT).getTags(); + assertThat(tags) + .containsEntry("tag3", "value3") + .doesNotContainKeys("tag1", "tag2"); + } + @Test void testContentTypeStoredAndCopied() throws Exception { // PUT with an explicit Content-Type (preserved by HeaderPreprocessor).