aws-sdk-java-v2 icon indicating copy to clipboard operation
aws-sdk-java-v2 copied to clipboard

Fix ApacheHttpClient's handling of request bodies on DELETE, GET, HEAD & OPTIONS requests

Open Xtansia opened this issue 11 months ago • 1 comments
trafficstars

This is a rework of https://github.com/aws/aws-sdk-java-v2/pull/5704 that was reverted.

Have modified logic to only use the entity enclosing request type on DELETE, GET, HEAD & OPTIONS when a content stream provider is present, ensuring the previous behavior of no Content-Length header being sent is retained.

Summary copied from previous PR:

Motivation and Context

Though providing request bodies on these HTTP methods is uncommon, it is not disallowed by the spec, and is used in certain Elasticsearch/OpenSearch APIs. Authenticating with Amazon OpenSearch Service can be done via SigV4 and we re-use the SDK's signing logic to provide that in https://github.com/opensearch-project/opensearch-java. We've received multiple bug reports of users having trouble making certain requests using the ApacheHttpClient (https://github.com/opensearch-project/opensearch-java/issues/712, https://github.com/opensearch-project/opensearch-java/issues/521), where the client itself doesn't complain or error but silently drops the request body resulting in a mismatched signature on the server. The Netty & CRT Sdk(Async)HttpClient implementations correctly send the request body for all methods, URL Connection does as well with the exception of GET which is hardcoded to swap to POST (and doesn't support PATCH at all). As such I think it is not harmful to bring the Apache implementation in line with the other client implementations.

Modifications

Generalize the Apache HTTP request factory implementation to attach the request body entity on all HTTP methods. This matches the original implementations of the separate method classes:

  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpDelete.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpGet.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpHead.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpOptions.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpPatch.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpPost.java
  • https://github.com/apache/httpcomponents-client/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/methods/HttpPut.java

Testing

Added unit tests

Screenshots (if appropriate)

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)

Checklist

  • [x] I have read the CONTRIBUTING document
  • [x] Local run of mvn install succeeds
  • [x] 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
  • [x] I have added tests to cover my changes
  • [x] All new and existing tests passed
  • [x] 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

  • [x] I confirm that this pull request can be released under the Apache 2 license

Xtansia avatar Dec 06 '24 02:12 Xtansia

What is the situation with this PR? @dagnir @davidh44
We are currently unable to perform requests to clear scrolls on AWS OpenSearch due to this issue (https://github.com/opensearch-project/opensearch-java/issues/712)

fire2 avatar May 12 '25 13:05 fire2