fix(dynamodb): attach X-Amz-Crc32 header to JSON protocol responses#347
Merged
hectorvent merged 2 commits intofloci-io:mainfrom Apr 11, 2026
Merged
fix(dynamodb): attach X-Amz-Crc32 header to JSON protocol responses#347hectorvent merged 2 commits intofloci-io:mainfrom
hectorvent merged 2 commits intofloci-io:mainfrom
Conversation
The AWS SDK for Go v2 DynamoDB client wraps every response body in a CRC32-verifying reader and emits "failed to close HTTP response body, this may affect connection reuse" on every call when the header is missing. Real AWS DynamoDB includes this header on all responses. Add it at the JSON protocol boundary (AwsJsonController) for both DynamoDB and DynamoDBStreams, using a new DynamoDbResponses helper that serializes the entity to bytes, computes the CRC32, and rebuilds the Response with the bytes + header. Error responses are wrapped as well to match real AWS behavior. The wrapping lives in AwsJsonController rather than DynamoDbJsonHandler because the handler is also invoked from CBOR, API Gateway proxy, and Step Functions task flows — those callers must keep the original ObjectNode entity. Add unit tests for DynamoDbResponses and an integration test that verifies the header is present and matches the CRC32 of the response body bytes on both success and error responses.
There was a problem hiding this comment.
Pull request overview
This PR improves DynamoDB (and DynamoDBStreams) AWS JSON protocol compatibility by ensuring responses include the X-Amz-Crc32 header, matching real AWS DynamoDB behavior and preventing AWS SDK for Go v2 checksum-close warnings.
Changes:
- Add
DynamoDbResponses.withCrc32(...)to pre-serialize JSON responses, compute CRC32, and attachX-Amz-Crc32. - Apply the CRC32 wrapping at the JSON protocol boundary in
AwsJsonControllerfor DynamoDB and DynamoDBStreams only. - Add unit and integration test coverage to validate header presence and correctness on both success and error responses.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbResponses.java | New helper to serialize response bodies, compute CRC32, and attach X-Amz-Crc32 while preserving headers/status. |
| src/main/java/io/github/hectorvent/floci/core/common/AwsJsonController.java | Wrap DynamoDB/DynamoDBStreams JSON responses with CRC32 header at the protocol boundary. |
| src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbResponsesTest.java | New unit tests for withCrc32 behavior (entity types, headers, status preservation). |
| src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbIntegrationTest.java | New E2E verification that real HTTP responses include correct X-Amz-Crc32 on success and error. |
src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbResponsesTest.java
Show resolved
Hide resolved
Address Copilot review comment on floci-io#347: ".getBytes()" uses the platform default charset, which can make the test (and the computed CRC32) vary across environments. The string is ASCII so the bytes are the same on any sensible platform, but being explicit about UTF-8 makes the determinism obvious and matches the rest of the codebase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The AWS SDK for Go v2 DynamoDB client wraps every response body in a CRC32-verifying reader (
service/dynamodb/internal/customizations/checksum.go). When theX-Amz-Crc32header is missing, the expected value defaults to 0, the verifier'sClose()returns a checksum mismatch error, and smithy-go'scloseResponseBodymiddleware logs"failed to close HTTP response body, this may affect connection reuse"on every DynamoDB call.Real AWS DynamoDB includes this header on every response (success and error). This PR adds it for floci's JSON protocol responses so the warning goes away and clients get a real integrity check.
Where the wrapping lives
DynamoDbResponses.withCrc32()— new helper inservices/dynamodb/that pre-serializes the entity to bytes, computes the CRC32, and rebuilds theResponsewithContent-Type: application/x-amz-json-1.0and the header.AwsJsonController— calls the helper for theDynamoDBandDynamoDBStreamscases.The wrapping is applied at the JSON protocol boundary (not inside
DynamoDbJsonHandler) becauseDynamoDbJsonHandler.handle()is also invoked fromAwsJsonCborController,AwsServiceRouter(API Gateway proxy), andAslExecutor(Step Functions tasks). Those callers need to keep the originalObjectNodeentity and use a different content type.CBOR protocol CRC32 is a follow-up — it's not involved in the Go SDK warning surfaced here.
Type of change
fix:)feat:)feat!:orfix!:)AWS Compatibility
Verified against AWS SDK for Go v2 (
github.com/aws/aws-sdk-go-v2/service/dynamodb@v1.57.0) usingcompatibility-tests/sdk-test-go/tests/dynamodb_test.go. Before: every DynamoDB test emitted 1+failed to close HTTP response bodywarnings (10 per fullTestDynamoDBrun). After: 0 warnings, all sub-tests pass.Checklist
./mvnw testpasses locally (1715/1715 — includingDynamoDbCborIntegrationTest,StepFunctionsDynamoDbIntegrationTest, and the newDynamoDbResponsesTest)DynamoDbResponsesTest— 6 unit tests for the helper (ObjectNode/byte[]/null entity, status code, header preservation, null input)DynamoDbIntegrationTest.responseIncludesCorrectXAmzCrc32Header— E2E verification that success (200) and error (400) responses carry a correct CRC32 header