Allowing keys with leading slashes for S3

Author: spfink

.changes/next-release/bugfix-AmazonS3-1f528cd.json

@@ -0,0 +1,5 @@
+{
+    "category": "Amazon S3", 
+    "type": "bugfix", 
+    "description": "Allows S3 to be used with object keys that have a leading slash \"/myKey\""
+}

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

@@ -37,7 +37,7 @@ public final class SimpleTypePathMarshaller {
      * so that it preserves the path structure.
      */
     public static final XmlMarshaller<String> GREEDY_STRING =
-        new SimplePathMarshaller<>(ValueToStringConverter.FROM_STRING, PathMarshaller.GREEDY);
+        new SimplePathMarshaller<>(ValueToStringConverter.FROM_STRING, PathMarshaller.GREEDY_WITH_SLASHES);
 
     public static final XmlMarshaller<Void> NULL = (val, context, paramName, sdkField) -> {
         throw new IllegalArgumentException(String.format("Parameter '%s' must not be null", paramName));

core/protocols/protocol-core/src/main/java/software/amazon/awssdk/protocols/core/PathMarshaller.java

@@ -32,6 +32,12 @@ public abstract class PathMarshaller {
      */
     public static final PathMarshaller GREEDY = new GreedyPathMarshaller();
 
+    /**
+     * Marshaller for greedy path labels that allows leading slahes. Value is not URL encoded and
+     * replaced in the request URI.
+     */
+    public static final PathMarshaller GREEDY_WITH_SLASHES = new GreedyLeadingSlashPathMarshaller();
+
     private PathMarshaller() {
     }
 
@@ -68,4 +74,14 @@ public String marshall(String resourcePath, String paramName, String pathValue)
         }
     }
 
+    private static class GreedyLeadingSlashPathMarshaller extends PathMarshaller {
+
+        @Override
+        public String marshall(String resourcePath, String paramName, String pathValue) {
+            Validate.notEmpty(pathValue, "%s cannot be empty.", paramName);
+            return resourcePath.replace(String.format("{%s+}", paramName),
+                                        SdkHttpUtils.urlEncodeIgnoreSlashes(pathValue));
+        }
+    }
+
 }

http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java

@@ -53,13 +53,27 @@ public class ApacheHttpRequestFactory {
 
     public HttpRequestBase create(final HttpExecuteRequest request, final ApacheHttpRequestConfig requestConfig) {
         URI uri = request.httpRequest().getUri();
-        HttpRequestBase base = createApacheRequest(request, uri.toString());
+
+        HttpRequestBase base = createApacheRequest(request, sanitizeUri(uri));
         addHeadersToRequest(base, request.httpRequest());
         addRequestConfig(base, request.httpRequest(), requestConfig);
 
         return base;
     }
 
+    /**
+     * The Apache HTTP client doesn't allow consecutive slashes in the URI. For S3
+     * and other AWS services, this is allowed and required. This methods replaces
+     * any occurrence of "//" in the URI path with "/%2F".
+     *
+     * @param uri The existing URI with double slashes not sanitized for Apache.
+     * @return a new String containing the modified URI
+     */
+    private String sanitizeUri(URI uri) {
+        String newPath = uri.getPath().replace("//", "/%2F");
+        return uri.toString().replace(uri.getPath(), newPath);
+    }
+
     private void addRequestConfig(final HttpRequestBase base,
                                   final SdkHttpRequest request,
                                   final ApacheHttpRequestConfig requestConfig) {

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

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2010-2019 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.services.s3;
+
+import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
+
+import java.nio.charset.StandardCharsets;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import software.amazon.awssdk.core.sync.RequestBody;
+
+public class KeysWithLeadingSlashIntegrationTest extends S3IntegrationTestBase {
+
+    private static final String BUCKET = temporaryBucketName(KeysWithLeadingSlashIntegrationTest.class);
+    private static final String KEY = "/stupidkeywithillegalleadingslashthatsucks";
+    private static final byte[] CONTENT = "Hello".getBytes(StandardCharsets.UTF_8);
+
+    @BeforeClass
+    public static void setUp() throws Exception {
+        S3IntegrationTestBase.setUp();
+        createBucket(BUCKET);
+    }
+
+    @Test
+    public void putObject_KeyWithLeadingSlash_Succeeds() {
+        s3.putObject(r -> r.bucket(BUCKET).key(KEY), RequestBody.fromBytes(CONTENT));
+        String retrievedKey = s3.listObjects(r -> r.bucket(BUCKET)).contents().get(0).key();
+
+        assert(retrievedKey).equals(KEY);
+    }
+}

test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-core-input.json

@@ -487,24 +487,6 @@
 	  }
       }
   },
-  {
-      "description": "Input with greedy label in path, leading slash removed",
-      "given": {
-	  "input": {
-	      "NonGreedyPathParam": "pathParamValue",
-	      "GreedyPathParam": "/foo/bar/baz"
-	  }
-      },
-      "when": {
-	  "action": "marshall",
-	  "operation": "OperationWithGreedyLabel"
-      },
-      "then": {
-	  "serializedAs": {
-	      "uri": "/2016-03-11/operationWithGreedyLabel/pathParamValue/foo/bar/baz"
-	  }
-      }
-  },
   {
     "description": "Operation with null members that have the idempotent trait are autofilled.",
     "when": {

test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-json-input.json

@@ -126,6 +126,24 @@
         }
       }
     }
+  },
+  {
+    "description": "Input with greedy label in path, leading slash removed",
+    "given": {
+      "input": {
+        "NonGreedyPathParam": "pathParamValue",
+        "GreedyPathParam": "/foo/bar/baz"
+      }
+    },
+    "when": {
+      "action": "marshall",
+      "operation": "OperationWithGreedyLabel"
+    },
+    "then": {
+      "serializedAs": {
+        "uri": "/2016-03-11/operationWithGreedyLabel/pathParamValue/foo/bar/baz"
+      }
+    }
   }
   // TODO This is a post process customization for API Gateway
   //  {

test/protocol-tests-core/src/main/resources/software/amazon/awssdk/protocol/suites/cases/rest-xml-input.json

@@ -350,6 +350,24 @@
         }
       }
     }
+  },
+  {
+    "description": "Input with greedy label in path, leading slash removed",
+    "given": {
+      "input": {
+        "NonGreedyPathParam": "pathParamValue",
+        "GreedyPathParam": "/foo/bar/baz"
+      }
+    },
+    "when": {
+      "action": "marshall",
+      "operation": "OperationWithGreedyLabel"
+    },
+    "then": {
+      "serializedAs": {
+        "uri": "/2016-03-11/operationWithGreedyLabel/pathParamValue//foo/bar/baz"
+      }
+    }
   }
   //  TODO this is not possible, payloads can only be structures or blobs. Only S3 utilizes this
   //  {