Bug Fix: Preserve trailing slashes in HTTP paths.

Fixes: CloudTrail integration test failures.
Details: S3 allows defining resources with trailing slashes in the name, and requires specifying the trailing slashes in the URL to be able to delete these resources. Eg. a Bucket "foo" can have a key "/bar/", and the request must be initiated as DELETE /bar/.

Author: millems

core/src/main/java/software/amazon/awssdk/core/auth/AbstractAwsSigner.java

@@ -278,7 +278,7 @@ private InputStream getContentUnwrapped(InputStream is) {
     }
 
     protected String getCanonicalizedResourcePath(String resourcePath, boolean urlEncode) {
-        if (resourcePath == null || resourcePath.isEmpty()) {
+        if (StringUtils.isEmpty(resourcePath)) {
             return "/";
         } else {
             String value = urlEncode ? SdkHttpUtils.urlEncodeIgnoreSlashes(resourcePath) : resourcePath;

core/src/test/java/software/amazon/awssdk/core/http/SdkHttpFullRequestAdapterTest.java

@@ -96,7 +96,7 @@ public void adapt_ResourcePathPreserved() {
 
         SdkHttpFullRequest adapted = SdkHttpFullRequestAdapter.toHttpFullRequest(request);
 
-        assertThat(adapted.encodedPath(), equalTo("/foo/bar"));
+        assertThat(adapted.encodedPath(), equalTo("/foo/bar/"));
     }
 
     @Test

http-client-spi/src/main/java/software/amazon/awssdk/http/DefaultSdkHttpFullRequest.java

@@ -69,7 +69,7 @@ private String standardizeProtocol(String protocol) {
     }
 
     private String standardizePath(String path) {
-        if (StringUtils.isEmpty(path) || path.equals("/")) {
+        if (StringUtils.isEmpty(path)) {
             return "";
         }
 
@@ -82,11 +82,6 @@ private String standardizePath(String path) {
 
         standardizedPath.append(path);
 
-        // Path must never end with '/', unless they're an empty path.
-        if (standardizedPath.length() > 1 && path.endsWith("/")) {
-            standardizedPath.setLength(standardizedPath.length() - 1);
-        }
-
         return standardizedPath.toString();
     }
 

http-client-spi/src/main/java/software/amazon/awssdk/http/SdkHttpRequest.java

@@ -64,8 +64,9 @@ public interface SdkHttpRequest extends SdkHttpHeaders {
     /**
      * Returns the URL-encoded path that should be used in the HTTP request.
      *
-     * <p>If a path is configured, the path will always start with '/' and never end with '/' (eg. "/my/path").
-     * If a path is not configured, this will always return empty-string (ie. "").</p>
+     * <p>If a path is configured, the path will always start with '/' and may or may not end with '/', depending on what the
+     * service might expect. If a path is not configured, this will always return empty-string (ie. ""). Note that '/' is also a
+     * valid path.</p>
      *
      * @return The path to the resource being requested.
      */

http-client-spi/src/test/java/software/amazon/awssdk/http/SdkHttpRequestResponseTest.java

@@ -49,12 +49,20 @@ public void uriConversionIsCorrect() {
         assertThat(normalizedUri(b -> b.protocol("https").host("localhost").port(443))).isEqualTo("https://localhost");
         assertThat(normalizedUri(b -> b.protocol("https").host("localhost").port(8443))).isEqualTo("https://localhost:8443");
         assertThat(normalizedUri(b -> b.protocol("https").host("localhost").port(80))).isEqualTo("https://localhost:80");
+        assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("foo")))
+                .isEqualTo("http://localhost/foo");
         assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("/foo")))
                 .isEqualTo("http://localhost/foo");
+        assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("foo/")))
+                .isEqualTo("http://localhost/foo/");
+        assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("/foo/")))
+                .isEqualTo("http://localhost/foo/");
         assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).rawQueryParameter("foo", "bar ")))
                 .isEqualTo("http://localhost?foo=bar%20");
         assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("/foo").rawQueryParameter("foo", "bar")))
                 .isEqualTo("http://localhost/foo?foo=bar");
+        assertThat(normalizedUri(b -> b.protocol("http").host("localhost").port(80).encodedPath("foo/").rawQueryParameter("foo", "bar")))
+                .isEqualTo("http://localhost/foo/?foo=bar");
     }
 
     private String normalizedUri(Consumer<SdkHttpFullRequest.Builder> builderMutator) {
@@ -104,17 +112,17 @@ private int normalizedPort(String protocol, Integer port) {
     @Test
     public void requestPathNormalizationIsCorrect() {
         assertThat(normalizedPath(null)).isEqualTo("");
-        assertThat(normalizedPath("/")).isEqualTo("");
+        assertThat(normalizedPath("/")).isEqualTo("/");
         assertThat(normalizedPath(" ")).isEqualTo("/ ");
-        assertThat(normalizedPath(" /")).isEqualTo("/ ");
+        assertThat(normalizedPath(" /")).isEqualTo("/ /");
         assertThat(normalizedPath("/ ")).isEqualTo("/ ");
-        assertThat(normalizedPath("/ /")).isEqualTo("/ ");
+        assertThat(normalizedPath("/ /")).isEqualTo("/ /");
         assertThat(normalizedPath(" / ")).isEqualTo("/ / ");
-        assertThat(normalizedPath("/Foo/")).isEqualTo("/Foo");
-        assertThat(normalizedPath("Foo/")).isEqualTo("/Foo");
+        assertThat(normalizedPath("/Foo/")).isEqualTo("/Foo/");
+        assertThat(normalizedPath("Foo/")).isEqualTo("/Foo/");
         assertThat(normalizedPath("Foo")).isEqualTo("/Foo");
-        assertThat(normalizedPath("/Foo/bar/")).isEqualTo("/Foo/bar");
-        assertThat(normalizedPath("Foo/bar/")).isEqualTo("/Foo/bar");
+        assertThat(normalizedPath("/Foo/bar/")).isEqualTo("/Foo/bar/");
+        assertThat(normalizedPath("Foo/bar/")).isEqualTo("/Foo/bar/");
         assertThat(normalizedPath("/Foo/bar")).isEqualTo("/Foo/bar");
         assertThat(normalizedPath("Foo/bar")).isEqualTo("/Foo/bar");
         assertThat(normalizedPath("%2F")).isEqualTo("/%2F");
@@ -328,7 +336,7 @@ private SdkHttpFullRequest validRequest() {
     }
 
     private SdkHttpFullRequest.Builder validRequestBuilder() {
-        return SdkHttpFullRequest.builder().protocol("http").host("localhost").encodedPath("/").method(SdkHttpMethod.GET);
+        return SdkHttpFullRequest.builder().protocol("http").host("localhost").method(SdkHttpMethod.GET);
     }
 
     private SdkHttpFullResponse validResponse() {

services/s3/src/test/java/software/amazon/awssdk/services/s3/handlers/S3EndpointResolutionTest.java

@@ -67,11 +67,11 @@ public void serviceLevelOperation_UsesStandardEndpoint() throws Exception {
 
         assertThat(mockHttpClient.getLastRequest().getUri())
                 .as("Uses regional S3 endpoint without bucket")
-                .isEqualTo(URI.create(ENDPOINT_WITHOUT_BUCKET));
+                .isEqualTo(URI.create(ENDPOINT_WITHOUT_BUCKET + "/"));
 
         assertThat(mockHttpClient.getLastRequest().encodedPath())
                 .as("Bucket is not in resource path")
-                .isEqualTo("");
+                .isEqualTo("/");
     }
 
     /**
@@ -86,7 +86,7 @@ public void serviceLevelOperation_WithDualstackEnabled_UsesDualstackEndpoint() t
 
         assertThat(mockHttpClient.getLastRequest().getUri())
                 .as("Uses regional S3 endpoint without bucket")
-                .isEqualTo(URI.create("https://s3.dualstack.ap-south-1.amazonaws.com"));
+                .isEqualTo(URI.create("https://s3.dualstack.ap-south-1.amazonaws.com/"));
     }
 
     /**
@@ -103,7 +103,7 @@ public void customEndpointProvided_UsesCustomEndpoint() throws Exception {
 
         assertThat(mockHttpClient.getLastRequest().getUri())
                 .as("Uses custom endpoint")
-                .isEqualTo(customEndpoint);
+                .isEqualTo(URI.create(customEndpoint + "/"));
     }
 
     /**
@@ -272,7 +272,7 @@ public void unsupportedAccelerateOption_UsesStandardEndpoint() throws Exception
 
         assertThat(mockHttpClient.getLastRequest().getUri())
                 .as("Uses regional S3 endpoint")
-                .isEqualTo(URI.create("https://s3.ap-south-1.amazonaws.com"));
+                .isEqualTo(URI.create("https://s3.ap-south-1.amazonaws.com/"));
     }
 
     /**

utils/src/main/java/software/amazon/awssdk/utils/http/SdkHttpUtils.java

@@ -230,22 +230,19 @@ public static int standardPort(String protocol) {
     }
 
     /**
-     * Append the given path to the given baseUri, separating them with a slash, if required. The result is always guaranteed to
-     * have a trailing slash.
+     * Append the given path to the given baseUri, separating them with a slash, if required. The result will preserve the
+     * trailing slash of the provided path.
      */
     public static String appendUri(String baseUri, String path) {
+        Validate.paramNotNull(baseUri, "baseUri");
         StringBuilder resultUri = new StringBuilder(baseUri);
 
-        if (!baseUri.endsWith("/")) {
-            resultUri.append('/');
-        }
+        if (!StringUtils.isEmpty(path)) {
+            if (!baseUri.endsWith("/")) {
+                resultUri.append("/");
+            }
 
-        if (path != null && path.length() > 0) {
             resultUri.append(path.startsWith("/") ? path.substring(1) : path);
-
-            if (resultUri.charAt(resultUri.length() - 1) != '/') {
-                resultUri.append('/');
-            }
         }
 
         return resultUri.toString();

utils/src/test/java/software/amazon/awssdk/utils/SdkHttpUtilsTest.java

@@ -95,14 +95,19 @@ public void encodeFlattenBehavesCorrectly() {
     public void urisAppendCorrectly() {
         assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> SdkHttpUtils.appendUri(null, ""));
         assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> SdkHttpUtils.appendUri(null, null));
-        assertThat(SdkHttpUtils.appendUri("", null)).isEqualTo("/");
-        assertThat(SdkHttpUtils.appendUri("", "")).isEqualTo("/");
-        assertThat(SdkHttpUtils.appendUri("foo.com", "")).isEqualTo("foo.com/");
+        assertThat(SdkHttpUtils.appendUri("", null)).isEqualTo("");
+        assertThat(SdkHttpUtils.appendUri("", "")).isEqualTo("");
+        assertThat(SdkHttpUtils.appendUri("", "bar")).isEqualTo("/bar");
+        assertThat(SdkHttpUtils.appendUri("", "/bar")).isEqualTo("/bar");
+        assertThat(SdkHttpUtils.appendUri("", "bar/")).isEqualTo("/bar/");
+        assertThat(SdkHttpUtils.appendUri("", "/bar/")).isEqualTo("/bar/");
+        assertThat(SdkHttpUtils.appendUri("foo.com", null)).isEqualTo("foo.com");
+        assertThat(SdkHttpUtils.appendUri("foo.com", "")).isEqualTo("foo.com");
         assertThat(SdkHttpUtils.appendUri("foo.com/", "")).isEqualTo("foo.com/");
-        assertThat(SdkHttpUtils.appendUri("foo.com", "bar")).isEqualTo("foo.com/bar/");
-        assertThat(SdkHttpUtils.appendUri("foo.com/", "bar")).isEqualTo("foo.com/bar/");
-        assertThat(SdkHttpUtils.appendUri("foo.com", "/bar")).isEqualTo("foo.com/bar/");
-        assertThat(SdkHttpUtils.appendUri("foo.com/", "/bar")).isEqualTo("foo.com/bar/");
+        assertThat(SdkHttpUtils.appendUri("foo.com", "bar")).isEqualTo("foo.com/bar");
+        assertThat(SdkHttpUtils.appendUri("foo.com/", "bar")).isEqualTo("foo.com/bar");
+        assertThat(SdkHttpUtils.appendUri("foo.com", "/bar")).isEqualTo("foo.com/bar");
+        assertThat(SdkHttpUtils.appendUri("foo.com/", "/bar")).isEqualTo("foo.com/bar");
         assertThat(SdkHttpUtils.appendUri("foo.com/", "/bar/")).isEqualTo("foo.com/bar/");
         assertThat(SdkHttpUtils.appendUri("foo.com/", "//bar/")).isEqualTo("foo.com//bar/");
     }