Fix checksum encoding, content-encoding header #4507

haydenbaker · 2023-09-29T00:31:05Z

Motivation and Context

Flexible checksums are supposed to be base64-encoded, not hex

Modifications

Updates encoding of checksums to be configurable
Updates FlexibleChecksummer to be more configurable through options

Testing

Ran with a few s3 integ tests, and chunk-encoded payloads now return 200's

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

I have read the CONTRIBUTING document
Local run of mvn install succeeds
My code follows the code style of this project
My change requires a change to the Javadoc documentation
I have updated the Javadoc documentation accordingly
I have added tests to cover my changes
All new and existing tests passed
I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

I confirm that this pull request can be released under the Apache 2 license

haydenbaker · 2023-09-29T00:34:14Z

...in/java/software/amazon/awssdk/http/auth/aws/internal/signer/checksums/ConstantChecksum.java

-
-    @Override
-    public String getChecksum() {
-        return value;
-    }


No longer needed (i added this default method), string repr is now configurable

haydenbaker · 2023-09-29T00:34:28Z

...rc/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/checksums/SdkChecksum.java

-
-    /**
-     * Return an encoded-string representation of the checksum. By default, this returns a String that is the lowercase, base16
-     * (hex) representation of the checksum.
-     */
-    default String getChecksum() {
-        return toHex(getChecksumBytes());
-    }


No longer needed (i added this default method), string repr is now configurable

haydenbaker · 2023-09-29T00:35:06Z

...are/amazon/awssdk/http/auth/aws/internal/signer/chunkedencoding/ChecksumTrailerProvider.java

@@ -41,7 +42,7 @@ public void reset() {
    public Pair<String, List<String>> get() {
        return Pair.of(
            checksumName,
-            Collections.singletonList(checksum.getChecksum())
+            Collections.singletonList(BinaryUtils.toBase64(checksum.getChecksumBytes()))


flexible checksum as trailer MUST be base64

cenedhryn · 2023-09-29T23:17:09Z

...re/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/SigningStage.java

-        if (request instanceof SdkHttpFullRequest) {
-            return (SdkHttpFullRequest) request;
-        }


Is this old code and what's the story behind the removal?

Yes, sorry for not clarifying. This code is problematic, because in cases where the request IS an SdkHttpFullRequest (such as in the integ tests) and the payload is transformed in signing (chunk-encoding), the underlying payload never gets re-set to the transformed payload from signing.