-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: Do not append another value to User-Agent if user already specified its value #5200
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
Conversation
WalkthroughA guard condition was added to the UserAgent plugin to prevent overwriting an existing User-Agent header. The header is now only added if Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5–10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/UserAgent.kt (1)
20-27: Consider clarifying documentation to reflect conditional behavior.The documentation states the plugin "adds a
User-Agentheader to all requests," but with this change, it only adds the header when not already present. Consider updating the documentation to reflect this behavior:/** - * A plugin that adds a `User-Agent` header to all requests. + * A plugin that adds a `User-Agent` header to requests that don't already have one. + * Headers set by the user take precedence over the plugin's default value. * * * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.client.plugins.UserAgent) * * @property agent a `User-Agent` header value. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/UserAgent.kt(1 hunks)
🔇 Additional comments (1)
ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/UserAgent.kt (1)
33-36: Guard condition is correctly implemented and will properly prevent overriding user-specified User-Agent headers.Verification confirms that user-set headers are accessible during the
onRequestphase. TheRequestHookintercepts atHttpRequestPipeline.State, which runs afterDefaultRequestatHttpRequestPipeline.Before. This means user-specified headers are populated before the UserAgent plugin's guard condition evaluates, sorequest.headers.contains(HttpHeaders.UserAgent)will correctly detect and preserve custom User-Agent headers.
e5l
left a comment
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.
Hey @zly2006, thank you for the PR. LGTM
Subsystem
Client (plugins)
Motivation
The user agent plugins is used to provide default value, not to send both values to the server, this may cause server mistakes trying to identifying the client.
Solution
This PR simply do not append to the user agent header if its value is already set. This makes the headers passed by the user to have the highest priority.
This behavior is generally applicable. python's
requestslibrary follows the same (requests.Session .headers).