Skip to content
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

SigV4: Add host header only when not already provided #5608

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

vsudilov
Copy link

@vsudilov vsudilov commented Sep 18, 2024

Motivation and Context

Our use case is to send SigV4 requests to a transparent proxy to S3. We need to keep the Host header in-tact, since we use that for virtual bucket routing. Without these changes, the sdk will not both send a request to a custom endpoint and sign against a custom Host header.

No existing issue for sdk-v2, however this issue was reported and closed as a wontfix in sdk-v1: aws/aws-sdk-java#2974

Modifications

Exit early from SignerUtils#addHostHeader if a Host header is present in the request.

Testing

mvn install on localhost; all tests pass except:

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.1.2:test (default-test) on project sqs

Example failure, seems completely unrelated to this PR:

-------------------------------------------------------------------------------                                                                   
Test set: software.amazon.awssdk.services.sqs.batchmanager.SqsAsyncBatchManagerTest                                                               
-------------------------------------------------------------------------------                                                                   
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.194 s <<< FAILURE! -- in software.amazon.awssdk.services.sqs.batchmanager.SqsAsy
ncBatchManagerTest                                                                                                                                
software.amazon.awssdk.services.sqs.batchmanager.SqsAsyncBatchManagerTest -- Time elapsed: 2.194 s <<< ERROR!                                     
software.amazon.awssdk.core.exception.SdkClientException: Unable to load region from any of the providers in the chain software.amazon.awssdk.regi
ons.providers.DefaultAwsRegionProviderChain@2c3bd568: [software.amazon.awssdk.regions.providers.SystemSettingsRegionProvider@6dad9ae6: Unable to l
oad region from system settings. Region must be specified either via environment variable (AWS_REGION) or  system property (aws.region)., software
.amazon.awssdk.regions.providers.AwsProfileRegionProvider@1f53e9b8: No region provided in profile: default, software.amazon.awssdk.regions.provide
rs.InstanceProfileRegionProvider@67874771: Unable to contact EC2 metadata service.] 

Screenshots (if appropriate)

N/A

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

@vsudilov vsudilov marked this pull request as ready for review September 19, 2024 14:48
@vsudilov vsudilov requested a review from a team as a code owner September 19, 2024 14:48
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.

2 participants