-
Notifications
You must be signed in to change notification settings - Fork 964
Skip modifying User-Agent in ApplyUserAgentStage if the user has already passed custom User-Agent in the request #6614
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
base: master
Are you sure you want to change the base?
Conversation
65f6dc4 to
05fbba0
Compare
…ady passed custom User-Agent in the request
05fbba0 to
e8b1e16
Compare
| "type": "bugfix", | ||
| "category": "AWS SDK for Java v2", | ||
| "contributor": "", | ||
| "description": "Skip User-Agent header modification in ApplyUserAgentStage when custom User-Agent is already provided." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is heavy on internal details. Can we simplify so we just say we don't overwrite the custom User-Agent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks the same, forgot to commit the new file perhaps? 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad
...-test/src/test/java/software/amazon/awssdk/services/useragent/CustomUserAgentHeaderTest.java
Show resolved
Hide resolved
| public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request, | ||
| RequestExecutionContext context) throws Exception { | ||
|
|
||
| if (hasUserAgentInAdditionalHeaders() || hasUserAgentInRequestHeaders(context)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: hasUserAgentInRequestHeaders "inRequestConfig", otherwise it sounds like the header is in the marshalled SdkHttpRequest
|



Motivation and Context
PR #6513 moved the
ApplyUserAgentStageto execute afterMergeCustomHeadersStage. This caused user-supplied User-Agent headers to be overwritten by the SDK's default User-Agent.Previous behavior: User-supplied User-Agent headers would overwrite the SDK's User-Agent because
ApplyUserAgentStageran beforeMergeCustomHeadersStage.After PR #6513: The SDK's User-Agent now overwrites user-supplied User-Agent headers because
ApplyUserAgentStageruns afterMergeCustomHeadersStage.This PR fixes the issue by skipping User-Agent modification in
ApplyUserAgentStagewhen a custom User-Agent is already provided viaADDITIONAL_HTTP_HEADERS.Modifications
ApplyUserAgentStageto detect if User-Agent header exists inADDITIONAL_HTTP_HEADERSconfigurationTesting
ApplyUserAgentStageTestcovering: - Custom User-Agent preservation - Empty/null User-Agent handling - User-Agent presence in additional headersCustomUserAgentHeaderTestcovering: - Single and multiple custom User-Agent values - API name handling with custom User-Agent - Edge cases (empty, null, list values)mvn installsucceedsTypes of changes
License