diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-764e546.json b/.changes/next-release/bugfix-AWSSDKforJavav2-764e546.json new file mode 100644 index 000000000000..2349e3ed0919 --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-764e546.json @@ -0,0 +1,6 @@ +{ + "category": "AWS SDK for Java v2", + "contributor": "", + "type": "bugfix", + "description": "Correctly handle multi-value headers in Aws4Signer" +} diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java index 310b60db61ac..46bf5f3a2c49 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.stream.Collectors; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; @@ -323,46 +324,51 @@ private String getCanonicalizedHeaderString(Map> canonicali StringBuilder buffer = new StringBuilder(); canonicalizedHeaders.forEach((headerName, headerValues) -> { - for (String headerValue : headerValues) { - appendCompactedString(buffer, headerName); - buffer.append(":"); - if (headerValue != null) { - appendCompactedString(buffer, headerValue); - } - buffer.append("\n"); - } + buffer.append(headerName); + buffer.append(":"); + buffer.append(String.join(",", trimAll(headerValues))); + buffer.append("\n"); }); return buffer.toString(); } /** - * This method appends a string to a string builder and collapses contiguous - * white space is a single space. - * - * This is equivalent to: - * destination.append(source.replaceAll("\\s+", " ")) + * "The Trimall function removes excess white space before and after values, + * and converts sequential spaces to a single space." + *

+ * https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + *

+ * The collapse-whitespace logic is equivalent to: + *

+     *     value.replaceAll("\\s+", " ")
+     * 
* but does not create a Pattern object that needs to compile the match * string; it also prevents us from having to make a Matcher object as well. - * */ - private void appendCompactedString(final StringBuilder destination, final String source) { + private String trimAll(String value) { boolean previousIsWhiteSpace = false; - int length = source.length(); + StringBuilder sb = new StringBuilder(value.length()); - for (int i = 0; i < length; i++) { - char ch = source.charAt(i); + for (int i = 0; i < value.length(); i++) { + char ch = value.charAt(i); if (isWhiteSpace(ch)) { if (previousIsWhiteSpace) { continue; } - destination.append(' '); + sb.append(' '); previousIsWhiteSpace = true; } else { - destination.append(ch); + sb.append(ch); previousIsWhiteSpace = false; } } + + return sb.toString().trim(); + } + + private List trimAll(List values) { + return values.stream().map(this::trimAll).collect(Collectors.toList()); } /** diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/signer/Aws4SignerTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/signer/Aws4SignerTest.java index ef2ee593a995..185ae8e55bae 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/signer/Aws4SignerTest.java +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/signer/Aws4SignerTest.java @@ -156,6 +156,48 @@ public void xAmznTraceId_NotSigned() throws Exception { "Signature=581d0042389009a28d461124138f1fe8eeb8daed87611d2a2b47fd3d68d81d73"); } + /** + * Multi-value headers should be comma separated. + */ + @Test + public void canonicalizedHeaderString_multiValueHeaders_areCommaSeparated() throws Exception { + AwsBasicCredentials credentials = AwsBasicCredentials.create("akid", "skid"); + SdkHttpFullRequest.Builder request = generateBasicRequest(); + request.appendHeader("foo","bar"); + request.appendHeader("foo","baz"); + + SdkHttpFullRequest actual = SignerTestUtils.signRequest(signer, request.build(), credentials, "demo", signingOverrideClock, "us-east-1"); + + // We cannot easily test the canonical header string value, but the below signature asserts that it contains: + // foo:bar,baz + assertThat(actual.firstMatchingHeader("Authorization")) + .hasValue("AWS4-HMAC-SHA256 Credential=akid/19810216/us-east-1/demo/aws4_request, " + + "SignedHeaders=foo;host;x-amz-archive-description;x-amz-date, " + + "Signature=1253bc1751048ea299e688cbe07a2224292e5cc606a079cb40459ad987793c19"); + } + + /** + * Canonical headers should remove excess white space before and after values, and convert sequential spaces to a single + * space. + */ + @Test + public void canonicalizedHeaderString_valuesWithExtraWhitespace_areTrimmed() throws Exception { + AwsBasicCredentials credentials = AwsBasicCredentials.create("akid", "skid"); + SdkHttpFullRequest.Builder request = generateBasicRequest(); + request.putHeader("My-header1"," a b c "); + request.putHeader("My-Header2"," \"a b c\" "); + + SdkHttpFullRequest actual = SignerTestUtils.signRequest(signer, request.build(), credentials, "demo", signingOverrideClock, "us-east-1"); + + // We cannot easily test the canonical header string value, but the below signature asserts that it contains: + // my-header1:a b c + // my-header2:"a b c" + assertThat(actual.firstMatchingHeader("Authorization")) + .hasValue("AWS4-HMAC-SHA256 Credential=akid/19810216/us-east-1/demo/aws4_request, " + + "SignedHeaders=host;my-header1;my-header2;x-amz-archive-description;x-amz-date, " + + "Signature=6d3520e3397e7aba593d8ebd8361fc4405e90aed71bc4c7a09dcacb6f72460b9"); + } + private SdkHttpFullRequest.Builder generateBasicRequest() { return SdkHttpFullRequest.builder() .contentStreamProvider(() -> new ByteArrayInputStream("{\"TableName\": \"foo\"}".getBytes()))