Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

fix S3 PUT operations returning HTTP Body by default#9036

Merged
bentsku merged 2 commits into
masterfrom
fix-s3-put-method-return-body
Sep 1, 2023
Merged

fix S3 PUT operations returning HTTP Body by default#9036
bentsku merged 2 commits into
masterfrom
fix-s3-put-method-return-body

Conversation

@bentsku
Copy link
Copy Markdown
Contributor

@bentsku bentsku commented Aug 31, 2023

Motivation

As reported in #9032, some S3 operations (the one using the PUT http method) would return an empty XML body when the S3 docs specifies that the body should be empty. After looking at the specs, only CopyObject and CopyObjectPart would return a body. Luckily, those operations implement a field specifying that they return a body, the payload one.

Changes

Removed the pre-signed handler deleting the response body, since this behaviour should have been for every PUT request except some, and it was actually fixing a symptom instead of the cause.

Implemented a check in the serializer for not serializing a body if the conditions were not set (if it's a PUT request without the payload field).

Added 2 cases in a test checking the raw returned body without boto parsing, one for a known PUT operation which should return a body (CopyObject) and one for UploadPart, which should not return a body.

@bentsku bentsku added aws:s3 Amazon Simple Storage Service area: asf semver: patch Non-breaking changes which can be included in patch releases labels Aug 31, 2023
@bentsku bentsku self-assigned this Aug 31, 2023
@bentsku bentsku linked an issue Aug 31, 2023 that may be closed by this pull request
1 task
@bentsku bentsku changed the title fix S3 PUT operations returning Body by default fix S3 PUT operations returning HTTP Body by default Aug 31, 2023
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 80.45% (-0.001%) from 80.451% when pulling ada3209 on fix-s3-put-method-return-body into 5217b87 on master.

@github-actions
Copy link
Copy Markdown

LocalStack Community integration with Pro

       2 files         2 suites   1h 24m 30s ⏱️
2 151 tests 1 677 ✔️ 474 💤 0
2 152 runs  1 677 ✔️ 475 💤 0

Results for commit ada3209.

@bentsku bentsku marked this pull request as ready for review August 31, 2023 21:05
Copy link
Copy Markdown
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Nice! Really great to see how much indirections can be removed with just handling that one condition on a global level in the serializer! 🚀

@bentsku bentsku merged commit 22ab2ae into master Sep 1, 2023
@bentsku bentsku deleted the fix-s3-put-method-return-body branch September 1, 2023 12:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: asf aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: UploadPart & PutObject Return Non-Empty Body

3 participants