AwsS3V4Signer normalizing path by default

[jattisha]

Describe the bug

Signing S3 requests that have encoded characters, when passing the path through the .encodedPath method of the SdkHttpFullRequest.builder(), blow up with a java.net.URISyntaxException. This only happens after this commit is pulled in. I believe that the commit didn't intend to normalize paths for S3, but I think it's still attempting to normalize the path which is causing this issue. I can see in my debugger that we are hitting this branch with normalizePath set to true.

Expected Behavior

We expect the request to be properly signed.

Current Behavior

Request fails with:

java.lang.IllegalArgumentException: Illegal character in path at index 17: https://test.com/ abc
	at java.base/java.net.URI.create(URI.java:906)
	at software.amazon.awssdk.http.SdkHttpRequest.getUri(SdkHttpRequest.java:157)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer$CanonicalRequest.addCanonicalizedResourcePath(AbstractAws4Signer.java:527)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer$CanonicalRequest.string(AbstractAws4Signer.java:506)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer.doSign(AbstractAws4Signer.java:111)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer.doSign(AbstractAws4Signer.java:78)
	at software.amazon.awssdk.auth.signer.internal.AbstractAwsS3V4Signer.sign(AbstractAwsS3V4Signer.java:80)
	at software.amazon.awssdk.auth.signer.internal.AbstractAwsS3V4Signer.sign(AbstractAwsS3V4Signer.java:61)
	... more
Caused by: java.net.URISyntaxException: Illegal character in path at index 17: https://test.com/ abc
	at java.base/java.net.URI$Parser.fail(URI.java:2974)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3145)
	at java.base/java.net.URI$Parser.parseHierarchical(URI.java:3227)
	at java.base/java.net.URI$Parser.parse(URI.java:3175)
	at java.base/java.net.URI.<init>(URI.java:623)
	at java.base/java.net.URI.create(URI.java:904)
	... 38 more

Reproduction Steps

public void demoTestForIssue() {
    byte[] bytes = new byte[1000];
    ThreadLocalRandom.current().nextBytes(bytes);
    ByteBuffer buffer = ByteBuffer.wrap(bytes);
    target = URI.create("https://test.com/%20abc");

    HttpHeaders httpHeadersToSign = HttpHeaders.of(Map.of("A", List.of("B")), (a, b) -> true);
    SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder()
            .contentStreamProvider(RequestBody.fromByteBuffer(buffer).contentStreamProvider())
            .method(SdkHttpMethod.fromValue("GET"))
            .uri(target)
            .headers(httpHeadersToSign.map())
            .encodedPath(target.getPath());
    ExecutionAttributes.Builder executionAttributesBuilder = ExecutionAttributes.builder()
            .put(AwsSignerExecutionAttribute.AWS_CREDENTIALS, AwsSessionCredentials.create("access", "secret", "token"))
            .put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2)
            .put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "MyService")
            .put(S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING, true);
    AwsS3V4Signer.create().sign(requestBuilder.build(), executionAttributesBuilder.build());
}

This throws

java.lang.IllegalArgumentException: Illegal character in path at index 17: https://test.com/ abc
	at java.base/java.net.URI.create(URI.java:906)
	at software.amazon.awssdk.http.SdkHttpRequest.getUri(SdkHttpRequest.java:157)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer$CanonicalRequest.addCanonicalizedResourcePath(AbstractAws4Signer.java:527)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer$CanonicalRequest.string(AbstractAws4Signer.java:506)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer.doSign(AbstractAws4Signer.java:111)
	at software.amazon.awssdk.auth.signer.internal.AbstractAws4Signer.doSign(AbstractAws4Signer.java:78)
	at software.amazon.awssdk.auth.signer.internal.AbstractAwsS3V4Signer.sign(AbstractAwsS3V4Signer.java:80)
	at software.amazon.awssdk.auth.signer.internal.AbstractAwsS3V4Signer.sign(AbstractAwsS3V4Signer.java:61)
	... more
Caused by: java.net.URISyntaxException: Illegal character in path at index 17: https://test.com/ abc
	at java.base/java.net.URI$Parser.fail(URI.java:2974)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3145)
	at java.base/java.net.URI$Parser.parseHierarchical(URI.java:3227)
	at java.base/java.net.URI$Parser.parse(URI.java:3175)
	at java.base/java.net.URI.<init>(URI.java:623)
	at java.base/java.net.URI.create(URI.java:904)
	... 38 more

Possible Solution

For S3 signers, the default behavior should be to not normalize the path. I believe this was the intention of the commit above (since the commit message specifically said "Normalize URLs during signing (except for S3)").

Additional Information/Context

Additionally, it would be great if, since we are already passing in the full URI to the SdkHttpFullRequest, we shouldn't need to set the encodedPath at all, since the request object already took the entire URI. However, we've already dealt with this in our code, so for now we just want to avoid the regression called out above.

AWS Java SDK version used

https://github.com/aws/aws-sdk-java-v2/releases/tag/2.18.33

JDK version used

17

Operating System and version

MacOS Monterey (though it shouldn't matter)

[debora-ito]

Looks like @millems worked on a fix via #3601. Will be released in the next version.

Thank you @jattisha for reporting this.

[debora-ito]

Fix released in SDK version 2.18.35.
Please check it out and let us know if you have further comments.