Skip to content

S3: fix Expires header behavior #12894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 24, 2025
Merged

S3: fix Expires header behavior #12894

merged 1 commit into from
Jul 24, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jul 22, 2025 •
edited
Loading

Motivation

As reported by #12890, we had the wrong behavior with the Expires parameter. This was ported back from our legacy implementation, which wrong assumptions: the Expires header is only about cache invalidation, and has nothing to do with Object Expiration, linked with lifecycle rules.

Expires: The date and time at which the object is no longer cacheable

This PR improves the AWS validated test, and remove the old behavior raising an Exception.

Changes

  • remove old behavior raising an exception based on Expires value
  • improve existing test

@bentsku bentsku added this to the 4.7 milestone Jul 22, 2025
@bentsku bentsku self-assigned this Jul 22, 2025
@bentsku bentsku requested a review from k-a-il as a code owner July 22, 2025 15:17
@bentsku bentsku added aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases labels Jul 22, 2025
@github-actions GitHub Actions
Copy link

S3 Image Test Results (AMD64 / ARM64)

    2 files    2 suites   8m 33s ⏱️
  515 tests 465 ✅  50 💤 0 ❌
1 030 runs  930 ✅ 100 💤 0 ❌

Results for commit 979a714.

@github-actions GitHub Actions
Copy link

Test Results - Preflight, Unit

21 983 tests  ±0   20 249 ✅ ±0   6m 10s ⏱️ -10s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 979a714. ± Comparison against base commit 75858d8.

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 10s ⏱️ -1s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 979a714. ± Comparison against base commit 75858d8.

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   1h 26m 1s ⏱️
2 285 tests 1 669 ✅ 616 💤 0 ❌
2 291 runs  1 669 ✅ 622 💤 0 ❌

Results for commit 979a714.

@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 4m 56s ⏱️ - 39m 23s
2 261 tests  - 2 669  1 642 ✅  - 2 511  619 💤  - 158  0 ❌ ±0 
2 263 runs   - 2 669  1 642 ✅  - 2 511  621 💤  - 158  0 ❌ ±0 

Results for commit 979a714. ± Comparison against base commit 75858d8.

This pull request removes 2670 and adds 1 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.s3.test_s3.TestS3 ‑ test_s3_object_expires

k-a-il
k-a-il approved these changes Jul 24, 2025
View reviewed changes
Copy link
Contributor

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

LGTM! Kudos to the reporter for spotting this issue

@bentsku bentsku merged commit d23f369 into main Jul 24, 2025
55 checks passed
@bentsku bentsku deleted the fix-s3-expires branch July 24, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k-a-il k-a-il k-a-il approved these changes

Assignees

@bentsku bentsku

Labels
aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.7
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsku @k-a-il