Uh oh!
There was an error while loading. Please reload this page.
feat(api): support custom request headers - #729
Conversation
Warning Review limit reached
More reviews will be available in 5 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/api/src/main/kotlin/com/clerk/api/network/ClerkApi.kt (1)
110-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear
_magicLinkduring reset to avoid stale API usage.Line [110]-[122] resets most Retrofit services, but
_magicLinkis not nulled. That leaves one endpoint callable against stale configuration afterreset().Proposed fix
fun reset() { _client = null _environment = null _session = null _signIn = null _signUp = null _user = null _deviceAttestation = null _organization = null + _magicLink = null configuredBaseUrl = null configuredUrlWithVersion = null configuredCustomHeaders = emptyMap() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/api/src/main/kotlin/com/clerk/api/network/ClerkApi.kt` around lines 110 - 122, The reset() function in the ClerkApi class clears most Retrofit service properties but does not null the _magicLink property, leaving it callable with stale configuration. Add _magicLink = null to the reset() function alongside the other property resets to ensure all API endpoints are properly cleared during reset.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@source/api/src/main/kotlin/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddleware.kt`:
- Line 55: The customHeaders forEach loop at line 55 in
VersioningUserAgentMiddleware uses addHeader to apply all custom headers without
filtering, which allows reserved Clerk authentication headers (Authorization,
x-clerk-client-id, x-clerk-device-id, etc.) to be duplicated via
case-insensitive HTTP semantics rather than replaced. Filter the customHeaders
before the forEach loop to exclude any keys matching reserved headers (perform
case-insensitive comparison), then only add the filtered headers using
addHeader. Additionally, update the test at lines 77-86 to validate that custom
headers matching reserved header names are properly excluded and do not create
duplicate header entries in the request.
---
Outside diff comments:
In `@source/api/src/main/kotlin/com/clerk/api/network/ClerkApi.kt`:
- Around line 110-122: The reset() function in the ClerkApi class clears most
Retrofit service properties but does not null the _magicLink property, leaving
it callable with stale configuration. Add _magicLink = null to the reset()
function alongside the other property resets to ensure all API endpoints are
properly cleared during reset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d12540e-e8da-425b-ae6f-238657d05961
📒 Files selected for processing (6)
source/api/src/main/kotlin/com/clerk/api/Clerk.ktsource/api/src/main/kotlin/com/clerk/api/configuration/ConfigurationManager.ktsource/api/src/main/kotlin/com/clerk/api/network/ClerkApi.ktsource/api/src/main/kotlin/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddleware.ktsource/api/src/test/java/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddlewareTest.ktsource/api/src/test/java/com/clerk/api/sdk/ProxyUrlConfigurationTest.kt
| newRequestBuilder.removeHeader("Content-Type") | ||
| } | ||
| customHeaders.forEach { (name, value) -> newRequestBuilder.addHeader(name, value) } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In OkHttp Request.Builder, what is the exact behavior difference between addHeader(name, value) and header(name, value) when the header already exists?
💡 Result:
In the OkHttp Request.Builder, the difference between header(name, value) and addHeader(name, value) lies in how they handle existing headers with the same name: 1. header(name, value): This method sets the specified header to the given value [1][2]. If any headers with that name already exist, they are all removed and replaced by this single new value [1][2]. Use this method when you want to ensure the header has exactly one value [3]. 2. addHeader(name, value): This method appends a new header with the specified name and value to the request [1][2]. If headers with that name already exist, they are preserved, and the new value is added alongside them [4]. This method is preferred for headers that support multiple values, such as "Cookie" [1][2]. Additionally, note that for certain headers, such as Content-Length and Content-Encoding, OkHttp may override the provided value with a header derived from the request body, regardless of which method is used [1][2].
Citations:
- 1: https://square.github.io/okhttp/3.x/okhttp/okhttp3/Request.Builder.html
- 2: https://github.com/square/okhttp/blob/master/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Request.kt
- 3: https://square.github.io/okhttp/recipes/
- 4: https://stackoverflow.com/questions/57872635/okhttp-authenticator-addheader-vs-header
🏁 Script executed:
# Find and examine the VersioningUserAgentMiddleware file
find . -name "VersioningUserAgentMiddleware.kt" -type fRepository: clerk/clerk-android
Length of output: 167
🏁 Script executed:
# Examine the middleware implementation
cat -n source/api/src/main/kotlin/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddleware.ktRepository: clerk/clerk-android
Length of output: 3073
🏁 Script executed:
# Check for OutgoingHeaders enum definition
find . -name "OutgoingHeaders.kt" -o -name "*OutgoingHeaders*"| head -5Repository: clerk/clerk-android
Length of output: 45
🏁 Script executed:
# Search for tests of VersioningUserAgentMiddleware
find . -path "*/test*" -name "*VersioningUserAgent*" -type fRepository: clerk/clerk-android
Length of output: 169
🏁 Script executed:
cat -n ./source/api/src/test/java/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddlewareTest.ktRepository: clerk/clerk-android
Length of output: 6898
🏁 Script executed:
# Search for where VersioningUserAgentMiddleware is instantiated with customHeaders
rg "VersioningUserAgentMiddleware" -A 2 -B 2 --type kotlinRepository: clerk/clerk-android
Length of output: 3451
🏁 Script executed:
# Check case sensitivity of OutgoingHeaders - check if header names use exact case
rg "addHeader|header\(" source/api/src/main/kotlin/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddleware.kt -A 1Repository: clerk/clerk-android
Length of output: 819
Prevent custom headers from overriding reserved Clerk authentication and metadata headers.
Line 55 uses addHeader for arbitrary custom headers without filtering. If customHeaders contains keys matching reserved headers (case-insensitive HTTP semantics: Authorization, x-clerk-client-id, x-clerk-device-id, etc.), OkHttp will create duplicate header entries instead of replacing them. This can cause conflicting authentication tokens or client identifiers in outgoing requests.
Additionally, the test at line 77–86 only validates non-reserved custom headers and does not cover the collision scenario.
Proposed fix
internal class VersioningUserAgentMiddleware(customHeaders: Map<String, String> = emptyMap()) :
Interceptor {
private val customHeaders = customHeaders.toMap()
+ private val reservedHeaders =+ setOf(+ OutgoingHeaders.CLERK_API_VERSION.header,+ OutgoingHeaders.X_ANDROID_SDK_VERSION.header,+ OutgoingHeaders.X_MOBILE.header,+ OutgoingHeaders.AUTHORIZATION.header,+ OutgoingHeaders.X_CLERK_CLIENT_ID.header,+ OutgoingHeaders.X_CLERK_DEVICE_ID.header,+ ).map { it.lowercase() }.toSet()
override fun intercept(chain: Interceptor.Chain): Response {
@@
- customHeaders.forEach { (name, value) -> newRequestBuilder.addHeader(name, value) }+ customHeaders.forEach { (name, value) ->+ if (name.lowercase() !in reservedHeaders) {+ newRequestBuilder.addHeader(name, value)+ }+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@source/api/src/main/kotlin/com/clerk/api/network/middleware/outgoing/VersioningUserAgentMiddleware.kt`
at line 55, The customHeaders forEach loop at line 55 in
VersioningUserAgentMiddleware uses addHeader to apply all custom headers without
filtering, which allows reserved Clerk authentication headers (Authorization,
x-clerk-client-id, x-clerk-device-id, etc.) to be duplicated via
case-insensitive HTTP semantics rather than replaced. Filter the customHeaders
before the forEach loop to exclude any keys matching reserved headers (perform
case-insensitive comparison), then only add the filtered headers using
addHeader. Additionally, update the test at lines 77-86 to validate that custom
headers matching reserved header names are properly excluded and do not create
duplicate header entries in the request.
@swolfand if you think there's a better way to do this, feel free to do that. Just need the ability to add custom headers to outgoing requests. iOS lets you inject custom middleware. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
ConfigurationManagerandClerkApiinto the outgoing request middlewareWhy
@clerk/exponeeds to identify itself when it configures the native Android SDK, in addition to the existing Android SDK headers. The motivating headers are:x-clerk-host-sdk: expox-clerk-host-sdk-version: <expo package version>This mirrors the direction available on iOS, where request customization lives in
Clerk.Optionsmiddleware.Approach
This keeps the new capability part of
ClerkConfigurationOptions, but adds it as an additive method:The configured headers are snapshotted with
toMap()and appended byVersioningUserAgentMiddlewareafter the built-in Clerk headers. Existing headers likex-android-sdk-version,x-mobile, auth, client id, and device id are left intact.Compatibility and tradeoffs
I intentionally did not add
customHeadersas a new primary-constructor property onClerkConfigurationOptions. Even with a default value, changing a Kotlin data-class primary constructor can be source-compatible while still being binary-risky for already-compiled consumers because constructor/default-arg/copy signatures change.The tradeoff is that
customHeadersis not part of the data class generatedcopy(),equals(), orhashCode()semantics in this non-major version. That keeps the change additive and minor-safe while still making headers option-owned. In a future major, we can movecustomHeadersinto the primary constructor for the cleaner Kotlin data-class shape.Testing
./gradlew :source:api:spotlessCheck./gradlew :source:api:compileDebugKotlin :source:api:compileDebugUnitTestKotlinFocused Robolectric test execution was attempted, but this local machine only has Java 17 installed and Robolectric selected Android SDK 36, which requires Java 21. The changed source and test Kotlin compile successfully.