Skip to content

Honor content-length set on request body #3123

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.

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

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Honor content-length set on request body #3123

merged 2 commits into from
Mar 29, 2022

Conversation

dagnir
Copy link
Contributor

@dagnir dagnir commented Mar 24, 2022

Motivation and Context

Bug fix.

Modifications

This commit ensures that when a RequestBody is created with an InputStream, the
content-length also set on the RequestBody is honored, regardless of how much
(extra) data is avaialble in the stream.

Fixes #2908

Testing

  • Unit tests for stream impl, sync client, trailing checksum
  • Running Integ tests

Screenshots (if appropriate)

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

@dagnir dagnir force-pushed the gh2908 branch 5 times, most recently from b0f50cc to f2f927d Compare March 25, 2022 16:40
@dagnir dagnir marked this pull request as ready for review March 25, 2022 16:46
@dagnir dagnir requested a review from a team as a code owner March 25, 2022 16:46
This commit ensures that when a RequestBody is created with an InputStream, the
content-length also set on the RequestBody is honored, regardless of how much
extra data is available in the stream. Note that if less data is actually
available in the wrapped stream, this implmenetation will return EOF as well.

Fixes aws#2908
@dagnir
Copy link
Contributor Author

dagnir commented Mar 29, 2022

The two bugs identified by SonarCloud are the same bug:

Make this method "synchronized" to match the parent class implementation.

  1. https://sonarcloud.io/project/issues?pullRequest=3123&issues=AX--g0BcvbEZitqyKCv6&open=AX--g0BcvbEZitqyKCv6&id=aws_aws-sdk-java-v2
  2. https://sonarcloud.io/project/issues?pullRequest=3123&issues=AX--g0BcvbEZitqyKCv7&open=AX--g0BcvbEZitqyKCv7&id=aws_aws-sdk-java-v2

I've resolved them as won't fix because this class isn't meant to be threadsafe.

@dagnir dagnir merged commit 170bc73 into aws:master Mar 29, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

98.2% 98.2% Coverage
0.0% 0.0% Duplication

aws-sdk-java-automation added a commit that referenced this pull request Jul 29, 2024
…c7772a7cf

Pull request: release <- staging/986b8bd4-c7b6-4cba-b7ef-af9c7772a7cf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SDK ignores "Content-Length" header of a S3 PutObject request with the body backed by an InputStream
2 participants