Skip to content

Commit af6765d

Browse files
committed
bug: fix Duplicate HTTP headers lost in conversion
review comment - #4919 (review) ./gradlew :sentry-okhttp:test --tests="*SentryOkHttpInterceptorTest.toMap handles duplicate headers correctly*"
1 parent 4b30647 commit af6765d

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import okhttp3.Interceptor
3333
import okhttp3.Request
3434
import okhttp3.RequestBody.Companion.toRequestBody
3535
import okhttp3.Response
36+
import org.jetbrains.annotations.VisibleForTesting
3637

3738
/**
3839
* The Sentry's [SentryOkHttpInterceptor], it will automatically add a breadcrumb and start a span
@@ -263,10 +264,19 @@ public open class SentryOkHttpInterceptor(
263264
}
264265

265266
/** Extracts headers from OkHttp Headers object into a map */
266-
private fun okhttp3.Headers.toMap(): Map<String, String> {
267+
@VisibleForTesting
268+
internal fun okhttp3.Headers.toMap(): Map<String, String> {
267269
val headers = linkedMapOf<String, String>()
268270
for (i in 0 until size) {
269-
headers[name(i)] = value(i)
271+
val name = name(i)
272+
val value = value(i)
273+
val existingValue = headers[name]
274+
if (existingValue != null) {
275+
// Concatenate duplicate headers with comma separator
276+
headers[name] = "$existingValue, $value"
277+
} else {
278+
headers[name] = value
279+
}
270280
}
271281
return headers
272282
}

sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,4 +680,39 @@ class SentryOkHttpInterceptorTest {
680680
assertNotNull(recordedRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER))
681681
assertNull(recordedRequest.getHeader(W3CTraceparentHeader.TRACEPARENT_HEADER))
682682
}
683+
684+
@Test
685+
fun `toMap handles duplicate headers correctly`() {
686+
// Create a response with duplicate headers
687+
val mockResponse =
688+
MockResponse()
689+
.setResponseCode(200)
690+
.setBody("test")
691+
.addHeader("Set-Cookie", "sessionId=123")
692+
.addHeader("Set-Cookie", "userId=456")
693+
.addHeader("Set-Cookie", "theme=dark")
694+
.addHeader("Accept", "text/html")
695+
.addHeader("Accept", "application/json")
696+
.addHeader("Single-Header", "value")
697+
698+
fixture.server.enqueue(mockResponse)
699+
700+
// Execute request to get response with headers
701+
val sut = fixture.getSut()
702+
val response = sut.newCall(getRequest()).execute()
703+
val headers = response.headers
704+
705+
// Optional: verify OkHttp preserves duplicate headers
706+
assertEquals(3, headers.values("Set-Cookie").size)
707+
assertEquals(2, headers.values("Accept").size)
708+
assertEquals(1, headers.values("Single-Header").size)
709+
710+
val interceptor = SentryOkHttpInterceptor(fixture.scopes)
711+
val headerMap = with(interceptor) { headers.toMap() }
712+
713+
// Duplicate headers will be collapsed into 1 concatenated entry with ", " separator
714+
assertEquals("sessionId=123, userId=456, theme=dark", headerMap["Set-Cookie"])
715+
assertEquals("text/html, application/json", headerMap["Accept"])
716+
assertEquals("value", headerMap["Single-Header"])
717+
}
683718
}

0 commit comments

Comments
 (0)