Use separate XML writer for S3 operations

.changes/next-release/bugfix-AmazonS3-8749050.json

@@ -0,0 +1,6 @@
+{
+    "category": "Amazon S3", 
+    "contributor": "", 
+    "type": "bugfix", 
+    "description": "Fixes a bug in DeleteObjects where specifying a key that contains characters that need to be escaped in XML and that is encoded, would delete the object with the decoded key instead."
+}

core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/AwsXmlProtocolFactory.java

@@ -74,6 +74,8 @@ public class AwsXmlProtocolFactory {
 
     private static final XmlProtocolUnmarshaller XML_PROTOCOL_UNMARSHALLER = XmlProtocolUnmarshaller.create();
 
+    private static final String S3_NAMESPACE_PREFIX = "http://s3.amazonaws.com";
+
     private final List<ExceptionMetadata> modeledExceptions;
     private final Supplier<SdkPojo> defaultServiceExceptionSupplier;
     private final HttpResponseHandler<AwsServiceException> errorUnmarshaller;
@@ -163,10 +165,15 @@ Optional<XmlElement> getErrorRoot(XmlElement document) {
 
     private XmlGenerator createGenerator(OperationInfo operationInfo) {
         return operationInfo.hasPayloadMembers() ?
-               XmlGenerator.create(operationInfo.addtionalMetadata(XML_NAMESPACE_ATTRIBUTE)) :
+               XmlGenerator.create(operationInfo.addtionalMetadata(XML_NAMESPACE_ATTRIBUTE), isS3(operationInfo)) :
                null;
     }
 
+    private boolean isS3(OperationInfo operationInfo) {
+        String namespace = operationInfo.addtionalMetadata(XML_NAMESPACE_ATTRIBUTE);
+        return namespace != null && namespace.startsWith(S3_NAMESPACE_PREFIX);
+    }
+
     public static Builder builder() {
         return new Builder();
     }

core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/S3XmlWriter.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright 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 software.amazon.awssdk.protocols.xml.internal.marshall;
+
+import java.io.Writer;
+import software.amazon.awssdk.annotations.SdkInternalApi;
+import software.amazon.awssdk.utils.StringUtils;
+
+/**
+ * Utility for creating easily creating XML documents, one element at a time, for S3 operations.
+ */
+@SdkInternalApi
+public final class S3XmlWriter extends XmlWriter {
+
+    /**
+     * Creates a new S3XmlWriter, ready to write an XML document to the specified writer. The root element in the XML
+     * document will specify an xmlns attribute with the specified namespace parameter.
+     *
+     * @param w     The writer this S3XmlWriter will write to.
+     * @param xmlns The XML namespace to include in the xmlns attribute of the root element.
+     */
+    S3XmlWriter(Writer w, String xmlns) {
+        super(w, xmlns);
+    }
+
+    @Override
+    protected String escapeXmlEntities(String s) {
+        return StringUtils.replaceEach(s, ESCAPE_SEARCHES, ESCAPE_REPLACEMENTS);
+    }
+}

core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlGenerator.java

@@ -33,9 +33,10 @@ private XmlGenerator(StringWriter stringWriter, XmlWriter xmlWriter) {
         this.xmlWriter = xmlWriter;
     }
 
-    public static XmlGenerator create(String xmlns) {
+    public static XmlGenerator create(String xmlns, boolean isS3) {
         StringWriter stringWriter = new StringWriter();
-        return new XmlGenerator(stringWriter, new XmlWriter(stringWriter, xmlns));
+        XmlWriter xmlWriter = isS3 ? new S3XmlWriter(stringWriter, xmlns) : new XmlWriter(stringWriter, xmlns);
+        return new XmlGenerator(stringWriter, xmlWriter);
     }
 
     public XmlWriter xmlWriter() {

core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/marshall/XmlWriter.java

@@ -31,7 +31,16 @@
  * Utility for creating easily creating XML documents, one element at a time.
  */
 @SdkInternalApi
-final class XmlWriter {
+public class XmlWriter {
+
+    static final String[] ESCAPE_SEARCHES = {
+        // Ampersands should always be the first to escape
+        "&", "\"", "'", "<", ">", "\r", "\n"
+    };
+
+    static final String[] ESCAPE_REPLACEMENTS = {
+        "&amp;", "&quot;", "&apos;", "&lt;", "&gt;", "&#x0D;", "&#x0A;"
+    };
 
     /** Standard XML prolog to add to the beginning of each XML document. */
     private static final String PROLOG = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
@@ -45,15 +54,6 @@ final class XmlWriter {
         "\"", "'", "<", ">", "\r", "\n", "&"
     };
 
-    private static final String[] ESCAPE_SEARCHES = {
-        // Ampersands should always be the first to escape
-        "&", "\"", "'", "<", ">", "\r", "\n"
-    };
-
-    private static final String[] ESCAPE_REPLACEMENTS = {
-        "&amp;", "&quot;", "&apos;", "&lt;", "&gt;", "&#x0D;", "&#x0A;"
-    };
-
     /** The writer to which the XML document created by this writer will be written. */
     private final Writer writer;
 
@@ -205,7 +205,7 @@ private void append(String s) {
         }
     }
 
-    private String escapeXmlEntities(String s) {
+    protected String escapeXmlEntities(String s) {
         // Unescape any escaped characters.
         if (s.contains("&")) {
             s = StringUtils.replaceEach(s, UNESCAPE_SEARCHES, UNESCAPE_REPLACEMENTS);

services/s3/src/it/java/software/amazon/awssdk/services/s3/DeleteObjectsIntegrationTest.java

@@ -0,0 +1,71 @@
+package software.amazon.awssdk.services.s3;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.services.s3.model.Delete;
+import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest;
+import software.amazon.awssdk.services.s3.model.DeleteObjectsResponse;
+import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+
+@RunWith(Parameterized.class)
+public class DeleteObjectsIntegrationTest extends S3IntegrationTestBase {
+
+    private static final String BUCKET = temporaryBucketName(DeleteObjectsIntegrationTest.class);
+    private static final String XML_KEY = "<Key>objectId</Key>";
+    private static final String ENCODED_KEY = "&lt;Key&gt;objectId&lt;/Key&gt;";
+    private static final String MIXED_KEY = "&lt;<";
+
+    @BeforeClass
+    public static void init() {
+        createBucket(BUCKET);
+    }
+
+    @AfterClass
+    public static void tearDownFixture() {
+        deleteBucketAndAllContents(BUCKET);
+    }
+
+    @Parameterized.Parameters
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][] {
+            { XML_KEY },
+            { ENCODED_KEY },
+            { MIXED_KEY }
+        });
+    }
+
+    private final String key;
+
+    public DeleteObjectsIntegrationTest(String key) {
+        this.key = key;
+    }
+
+    @Test
+    public void deleteObjects_shouldCorrectlyDeleteObjectsWithSpecifiedKeys() {
+        s3.putObject(PutObjectRequest.builder()
+                                     .bucket(BUCKET)
+                                     .key(key)
+                                     .build(), RequestBody.fromString("contents"));
+
+        // Does not throw exception
+        s3.headObject(r -> r.bucket(BUCKET).key(key));
+
+        Delete delete = Delete.builder().objects(o -> o.key(key)).build();
+        DeleteObjectsRequest request = DeleteObjectsRequest.builder().bucket(BUCKET).delete(delete).build();
+        DeleteObjectsResponse response = s3.deleteObjects(request);
+
+        assertThat(response.deleted().get(0).key()).isEqualTo(key);
+        assertThrows(NoSuchKeyException.class, () -> s3.headObject(r -> r.bucket(BUCKET).key(key)));
+    }
+}