diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-74f2d55.json b/.changes/next-release/bugfix-AWSSDKforJavav2-74f2d55.json new file mode 100644 index 000000000000..c294595239d5 --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-74f2d55.json @@ -0,0 +1,6 @@ +{ + "category": "AWS SDK for Java v2", + "contributor": "skrueger", + "type": "bugfix", + "description": "Fix bug in SigV4 canonical request creation to handle multiple header values and leading & trailing whitespace in header values correctly." +} 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..2e90d82c39be 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 @@ -304,7 +304,7 @@ private void addPreSignInformationToRequest(SdkHttpFullRequest.Builder mutableRe mutableRequest.putRawQueryParameter(SignerConstant.X_AMZ_CREDENTIAL, signingCredentials); } - private Map> canonicalizeSigningHeaders(Map> headers) { + static Map> canonicalizeSigningHeaders(Map> headers) { Map> result = new TreeMap<>(); for (Map.Entry> header : headers.entrySet()) { @@ -319,17 +319,33 @@ private Map> canonicalizeSigningHeaders(Map> canonicalizedHeaders) { + /** + * Step 4 of the AWS Signature version 4 calculation. + * Refer to + * https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + */ + static String getCanonicalizedHeaderString(Map> canonicalizedHeaders) { StringBuilder buffer = new StringBuilder(); canonicalizedHeaders.forEach((headerName, headerValues) -> { - for (String headerValue : headerValues) { + if (headerValues.size() > 0) { appendCompactedString(buffer, headerName); - buffer.append(":"); - if (headerValue != null) { - appendCompactedString(buffer, headerValue); + buffer.append(':'); + boolean headerValueAppended = false; + // Append a comma-separated list of values for that header. + // Do not sort the values in headers that have multiple values. + for (int i = 0; i < headerValues.size(); i++) { + String headerValue = headerValues.get(i); + if (headerValue != null) { + if (headerValueAppended) { + buffer.append(','); + } + appendCompactedString(buffer, headerValue); + headerValueAppended = true; + } } - buffer.append("\n"); + + buffer.append('\n'); } }); @@ -337,30 +353,39 @@ private String getCanonicalizedHeaderString(Map> canonicali } /** - * This method appends a string to a string builder and collapses contiguous - * white space is a single space. + * This method appends a modified version of the source string to the destination string builder. + * The source string's leading and trailing whitespace is removed and + * and all contiguous white space not at the beginning or end is collapsed into a single space. + * + * e.g., + * Given a source String of " a b c ", + * "a b c" is appended to the destination StringBuilder. * * This is equivalent to: - * destination.append(source.replaceAll("\\s+", " ")) + * + * destination.append(source.replaceAll("(^\\s*|\\s*$)", "").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) { + static void appendCompactedString(final StringBuilder destination, final String source) { + boolean appendedNonWhitespaceChar = false; boolean previousIsWhiteSpace = false; int length = source.length(); for (int i = 0; i < length; i++) { char ch = source.charAt(i); if (isWhiteSpace(ch)) { - if (previousIsWhiteSpace) { - continue; + if (appendedNonWhitespaceChar) { + previousIsWhiteSpace = true; } - destination.append(' '); - previousIsWhiteSpace = true; } else { + if (previousIsWhiteSpace) { + destination.append(' '); + previousIsWhiteSpace = false; + } destination.append(ch); - previousIsWhiteSpace = false; + appendedNonWhitespaceChar = true; } } } @@ -373,7 +398,7 @@ private void appendCompactedString(final StringBuilder destination, final String * @param ch the character to be tested * @return true if the character is white space, false otherwise. */ - private boolean isWhiteSpace(final char ch) { + private static boolean isWhiteSpace(final char ch) { return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\u000b' || ch == '\r' || ch == '\f'; } diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4SignerTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4SignerTest.java new file mode 100644 index 000000000000..8da4f88df020 --- /dev/null +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4SignerTest.java @@ -0,0 +1,83 @@ +/* + * 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.auth.signer.internal; + +import org.junit.Test; + +import java.util.*; + +import static org.junit.Assert.assertEquals; + +public class AbstractAws4SignerTest { + + @Test + public void testAppendCompactedString() { + StringBuilder destination = new StringBuilder(); + AbstractAws4Signer.appendCompactedString(destination, " a b c "); + String expected = "a b c"; + assertEquals(expected, destination.toString()); + } + + @Test + public void testGetCanonicalizedHeaderString_SpecExample() { + Map> headers = new LinkedHashMap<>(); + headers.put("Host", Arrays.asList("iam.amazonaws.com")); + headers.put("Content-Type", Arrays.asList("application/x-www-form-urlencoded; charset=utf-8")); + headers.put("My-header1", Arrays.asList(" a b c ")); + headers.put("X-Amz-Date", Arrays.asList("20150830T123600Z")); + headers.put("My-Header2", Arrays.asList(" \"a b c\" ")); + + Map> canonicalizeSigningHeaders = + AbstractAws4Signer.canonicalizeSigningHeaders(headers); + + String actual = AbstractAws4Signer.getCanonicalizedHeaderString(canonicalizeSigningHeaders); + + String expected = String.join("\n", + "content-type:application/x-www-form-urlencoded; charset=utf-8", + "host:iam.amazonaws.com", + "my-header1:a b c", + "my-header2:\"a b c\"", + "x-amz-date:20150830T123600Z", + ""); + + assertEquals(expected, actual); + } + + @Test + public void testGetCanonicalizedHeaderString_MultipleHeaderValuess() { + Map> headers = new LinkedHashMap<>(); + headers.put("Host", Arrays.asList("iam.amazonaws.com")); + headers.put("Content-Type", Arrays.asList("application/x-www-form-urlencoded; charset=utf-8")); + headers.put("My-header1", Arrays.asList(" a b c ")); + headers.put("X-Amz-Date", Arrays.asList("20150830T123600Z")); + headers.put("My-Header1", Arrays.asList(" \"a b c\" ")); + + Map> canonicalizeSigningHeaders = + AbstractAws4Signer.canonicalizeSigningHeaders(headers); + + String actual = AbstractAws4Signer.getCanonicalizedHeaderString(canonicalizeSigningHeaders); + + String expected = String.join("\n", + "content-type:application/x-www-form-urlencoded; charset=utf-8", + "host:iam.amazonaws.com", + "my-header1:a b c,\"a b c\"", + "x-amz-date:20150830T123600Z", + ""); + + assertEquals(expected, actual); + } + +}