Skip to content

Do not store "aws-chunked" encoding#2345

Merged
afranken merged 2 commits intomainfrom
2218-do-not-store-aws-chunked-encoding
Apr 22, 2025
Merged

Do not store "aws-chunked" encoding#2345
afranken merged 2 commits intomainfrom
2218-do-not-store-aws-chunked-encoding

Conversation

@afranken
Copy link
Member

Description

AWS docs specify that this encoding is not persisted or returned if it's the only value sent in the "Content-Encoding" header.

Related Issue

Fixes #2218

Tasks

  • I have signed the CLA.
  • I have written tests and verified that they fail without my change.

@afranken afranken self-assigned this Apr 22, 2025
@afranken afranken requested a review from Copilot April 22, 2025 08:12
@afranken afranken added the bug label Apr 22, 2025

This comment was marked as outdated.

@afranken afranken force-pushed the 2218-do-not-store-aws-chunked-encoding branch 2 times, most recently from acf17a8 to 8290974 Compare April 22, 2025 08:25
@afranken afranken requested a review from Copilot April 22, 2025 08:28

This comment was marked as outdated.

AWS docs specify that this encoding is not persisted or returned if it's
the only value sent in the "Content-Encoding" header.

Fixes #2218
@afranken afranken force-pushed the 2218-do-not-store-aws-chunked-encoding branch from 8290974 to 8cfb422 Compare April 22, 2025 09:00
@afranken afranken requested a review from Copilot April 22, 2025 09:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the logic for storing HTTP headers so that the "aws-chunked" content-encoding is not persisted when it is the only value and also improves method naming and tests to support the change.

  • Updated HeaderUtil.java to conditionally exclude "aws-chunked" if it is the sole encoding header.
  • Renamed methods and updated ServiceBase.java to use the new logic.
  • Adjusted tests in AwsChunkedEncodingIT.kt and modified the cleanup logic in S3TestBase.kt.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
server/src/main/java/com/adobe/testing/s3mock/util/HeaderUtil.java Updates header storage logic and adds helper method to detect sole "aws-chunked" encoding.
server/src/main/java/com/adobe/testing/s3mock/service/ServiceBase.java Refactors usage of header-checking methods to align with updated HeaderUtil.java.
integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt Removes condition that previously skipped cleanup of a specific test bucket.
integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/AwsChunkedEncodingIT.kt Fixes a typo in the test class name and updates tests to assert the absence of "aws-chunked" encoding.
CHANGELOG.md Updates version information and changelog content to reflect the new changes.
Comments suppressed due to low confidence (1)

integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt:229

  • The removal of the condition that skips the 'testputandgetretention-545488000' bucket in cleanupStores might lead to unintended deletion of this special test bucket. Consider reintroducing the condition or adding a comment to clarify the intended change.
if(bucket.name() == "testputandgetretention-545488000") {return}

@afranken afranken merged commit 2173361 into main Apr 22, 2025
6 checks passed
@afranken afranken deleted the 2218-do-not-store-aws-chunked-encoding branch April 22, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Content-Encoding: aws-chunked should not be stored

2 participants