diff --git a/CHANGELOG.md b/CHANGELOG.md index 90ddcf1f06..568a848546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Keep UI profiling running when `startProfiler` is called before a requested stop takes effect at the end of the current chunk ([#6291](https://github.com/getsentry/sentry-java/pull/6291)) - Before, the start call was ignored and the profiler still stopped at the end of the chunk, even in `TRACE` mode while sampled spans were active. On API 34 and lower, every later profile also stopped after one chunk. - Refuse to send data to servers whose certificate was issued by one of the leaked eMudhra intermediates (CVE-2026-111943) ([#6336](https://github.com/getsentry/sentry-java/pull/6336)) +- Do not inspect the response body of streaming Apollo 3 and 4 responses (`multipart/mixed`, `text/event-stream`) for failed requests, which blocked `@defer` and `@stream` ([#6432](https://github.com/getsentry/sentry-java/pull/6432)) ### Dependencies diff --git a/sentry-apollo-3/build.gradle.kts b/sentry-apollo-3/build.gradle.kts index 2750eec5fd..7d259c1bec 100644 --- a/sentry-apollo-3/build.gradle.kts +++ b/sentry-apollo-3/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(projects.sentryTestSupport) testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.apollo3.kotlin) + testImplementation(libs.google.truth) testImplementation(libs.kotlin.test.junit) testImplementation(libs.google.truth) testImplementation(libs.kotlinx.coroutines) diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 53e4865739..8c796cec40 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -281,6 +281,13 @@ constructor( private fun getHeader(key: String, headers: List): String? = headers.firstOrNull { it.name.equals(key, true) }?.value + private fun isStreamingResponse(response: HttpResponse): Boolean { + val contentType = getHeader("Content-Type", response.headers) ?: return false + // strip parameters such as boundary or charset, e.g. "multipart/mixed; boundary=xyz" + val mediaType = contentType.substringBefore(';').trim() + return STREAMING_MEDIA_TYPES.any { it.equals(mediaType, ignoreCase = true) } + } + private fun getRequestHeaders(headers: List): MutableMap? { if (scopes.options.dataCollectionResolver.isDataCollectionConfigured) { val requestHeaders = mutableMapOf() @@ -347,6 +354,25 @@ constructor( // wrap everything up in a try catch block so every exception is swallowed and degraded // gracefully try { + // not possible to get a parameterized url, but we remove at least the + // query string and the fragment. + // url example: https://api.github.com/users/getsentry/repos/#fragment?query=query + // url will be: https://api.github.com/users/getsentry/repos/ + // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ + // but that's not possible + val urlDetails = UrlUtils.parse(request.url, scopes.options.dataCollectionResolver) + + // return before reading the response body if it's not a target match + if (!PropagationTargetsUtils.contain(failedRequestTargets, urlDetails.urlOrFallback)) { + return + } + + // streaming responses (e.g. multipart incremental delivery, server-sent events) may never + // reach EOF or be unbounded, so peeking at them would block or buffer indefinitely + if (isStreamingResponse(response)) { + return + } + // we pay the price to read the response in the memory to check if there's any errors // GraphQL does not throw status code 400+ for every type of error val body = @@ -363,19 +389,6 @@ constructor( return } - // not possible to get a parameterized url, but we remove at least the - // query string and the fragment. - // url example: https://api.github.com/users/getsentry/repos/#fragment?query=query - // url will be: https://api.github.com/users/getsentry/repos/ - // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ - // but that's not possible - val urlDetails = UrlUtils.parse(request.url, scopes.options.dataCollectionResolver) - - // return if its not a target match - if (!PropagationTargetsUtils.contain(failedRequestTargets, urlDetails.urlOrFallback)) { - return - } - val mechanism = Mechanism().apply { type = "SentryApollo3Interceptor" } val fingerprints = mutableListOf() @@ -471,6 +484,8 @@ constructor( const val SENTRY_APOLLO_3_OPERATION_TYPE = "SENTRY-APOLLO-3-OPERATION-TYPE" const val DEFAULT_CAPTURE_FAILED_REQUESTS = true + private val STREAMING_MEDIA_TYPES = setOf("multipart/mixed", "text/event-stream") + init { SentryIntegrationPackageStorage.getInstance() .addPackage("maven:io.sentry:sentry-apollo-3", BuildConfig.VERSION_NAME) diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt index 1ca41b566d..e53da365d5 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt @@ -1,6 +1,7 @@ package io.sentry.apollo3 import com.apollographql.apollo3.ApolloClient +import com.apollographql.apollo3.api.http.HttpMethod import com.apollographql.apollo3.api.http.HttpRequest import com.apollographql.apollo3.api.http.HttpResponse import com.apollographql.apollo3.exception.ApolloException @@ -11,6 +12,7 @@ import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.DataCategory +import io.sentry.Hint import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction @@ -46,12 +48,16 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import okio.Buffer +import okio.ForwardingSource +import okio.buffer import org.junit.Before import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -275,6 +281,83 @@ class SentryApollo3InterceptorTest { assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) } + @Test + fun `does not read response body if failed request target does not match`(): Unit = runBlocking { + val bodyRead = + interceptWithContentType( + contentType = null, + url = "https://excluded.example/graphql", + ) + + assertThat(bodyRead).isFalse() + } + + @Test + fun `does not read multipart mixed response body`(): Unit = runBlocking { + val bodyRead = interceptWithContentType("Multipart/Mixed; boundary=graphql") + + assertThat(bodyRead).isFalse() + verify(fixture.scopes, never()).captureEvent(any(), any()) + } + + @Test + fun `does not read event stream response body`(): Unit = runBlocking { + val bodyRead = interceptWithContentType(" text/event-stream ; charset=utf-8") + + assertThat(bodyRead).isFalse() + verify(fixture.scopes, never()).captureEvent(any(), any()) + } + + @Test + fun `reads json response body for failed request detection`(): Unit = runBlocking { + val bodyRead = interceptWithContentType("application/json; charset=utf-8") + + assertThat(bodyRead).isTrue() + verify(fixture.scopes).captureEvent(any(), any()) + } + + @Test + fun `reads response body without content type for failed request detection`(): Unit = + runBlocking { + val bodyRead = interceptWithContentType(null) + + assertThat(bodyRead).isTrue() + verify(fixture.scopes).captureEvent(any(), any()) + } + + /** Runs the interceptor against a GraphQL error body and returns whether the body was read. */ + private suspend fun interceptWithContentType( + contentType: String?, + url: String = "https://target.example/graphql", + ): Boolean { + var bodyRead = false + val body = + object : ForwardingSource(Buffer().writeUtf8("{\"errors\":[{\"message\":\"error\"}]}")) { + override fun read(sink: Buffer, byteCount: Long): Long { + bodyRead = true + return super.read(sink, byteCount) + } + } + .buffer() + val request = HttpRequest.Builder(HttpMethod.Post, url).build() + val responseBuilder = HttpResponse.Builder(200).body(body) + contentType?.let { responseBuilder.addHeader("content-type", it) } + val response = responseBuilder.build() + val chain = + object : HttpInterceptorChain { + override suspend fun proceed(request: HttpRequest): HttpResponse = response + } + val interceptor = + SentryApollo3HttpInterceptor( + fixture.scopes, + captureFailedRequests = true, + failedRequestTargets = listOf("target.example"), + ) + + interceptor.intercept(request, chain) + return bodyRead + } + @Test fun `customizer modifies span`() { executeQuery( diff --git a/sentry-apollo-4/build.gradle.kts b/sentry-apollo-4/build.gradle.kts index 04cb6b12f1..9d33736c91 100644 --- a/sentry-apollo-4/build.gradle.kts +++ b/sentry-apollo-4/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(projects.sentryTestSupport) testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.apollo4.kotlin) + testImplementation(libs.google.truth) testImplementation(libs.kotlin.test.junit) testImplementation(libs.google.truth) testImplementation(libs.kotlinx.coroutines) diff --git a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt index aea5d6691c..99f96b2207 100644 --- a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt +++ b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt @@ -280,6 +280,13 @@ constructor( private fun getHeader(key: String, headers: List): String? = headers.firstOrNull { it.name.equals(key, true) }?.value + private fun isStreamingResponse(response: HttpResponse): Boolean { + val contentType = getHeader("Content-Type", response.headers) ?: return false + // strip parameters such as boundary or charset, e.g. "multipart/mixed; boundary=xyz" + val mediaType = contentType.substringBefore(';').trim().lowercase(Locale.ROOT) + return mediaType in STREAMING_MEDIA_TYPES + } + private fun getRequestHeaders(headers: List): MutableMap? { if (scopes.options.dataCollectionResolver.isDataCollectionConfigured) { val requestHeaders = mutableMapOf() @@ -346,6 +353,25 @@ constructor( // wrap everything up in a try catch block so every exception is swallowed and degraded // gracefully try { + // not possible to get a parameterized url, but we remove at least the + // query string and the fragment. + // url example: https://api.github.com/users/getsentry/repos/#fragment?query=query + // url will be: https://api.github.com/users/getsentry/repos/ + // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ + // but that's not possible + val urlDetails = UrlUtils.parse(request.url, scopes.options.dataCollectionResolver) + + // return before reading the response body if it's not a target match + if (!PropagationTargetsUtils.contain(failedRequestTargets, urlDetails.urlOrFallback)) { + return + } + + // streaming responses (e.g. multipart incremental delivery, server-sent events) may never + // reach EOF or be unbounded, so peeking at them would block or buffer indefinitely + if (isStreamingResponse(response)) { + return + } + // we pay the price to read the response in the memory to check if there's any errors // GraphQL does not throw status code 400+ for every type of error val body = @@ -362,19 +388,6 @@ constructor( return } - // not possible to get a parameterized url, but we remove at least the - // query string and the fragment. - // url example: https://api.github.com/users/getsentry/repos/#fragment?query=query - // url will be: https://api.github.com/users/getsentry/repos/ - // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/ - // but that's not possible - val urlDetails = UrlUtils.parse(request.url, scopes.options.dataCollectionResolver) - - // return if it's not a target match - if (!PropagationTargetsUtils.contain(failedRequestTargets, urlDetails.urlOrFallback)) { - return - } - val mechanism = Mechanism().apply { type = "SentryApollo4Interceptor" } val fingerprints = mutableListOf() @@ -468,6 +481,8 @@ constructor( companion object { const val DEFAULT_CAPTURE_FAILED_REQUESTS = true + private val STREAMING_MEDIA_TYPES = setOf("multipart/mixed", "text/event-stream") + init { SentryIntegrationPackageStorage.getInstance() .addPackage("maven:io.sentry:sentry-apollo-4", BuildConfig.VERSION_NAME) diff --git a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt index 0418e55fb3..756f0d9c26 100644 --- a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt +++ b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt @@ -4,6 +4,7 @@ import com.apollographql.apollo.ApolloCall import com.apollographql.apollo.ApolloClient import com.apollographql.apollo.api.ApolloResponse import com.apollographql.apollo.api.Operation +import com.apollographql.apollo.api.http.HttpMethod import com.apollographql.apollo.api.http.HttpRequest import com.apollographql.apollo.api.http.HttpResponse import com.apollographql.apollo.exception.ApolloException @@ -14,6 +15,7 @@ import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.DataCategory +import io.sentry.Hint import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction @@ -51,12 +53,16 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import okio.Buffer +import okio.ForwardingSource +import okio.buffer import org.junit.Before import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -289,6 +295,83 @@ abstract class SentryApollo4HttpInterceptorTest( assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) } + @Test + fun `does not read response body if failed request target does not match`(): Unit = runBlocking { + val bodyRead = + interceptWithContentType( + contentType = null, + url = "https://excluded.example/graphql", + ) + + assertThat(bodyRead).isFalse() + } + + @Test + fun `does not read multipart mixed response body`(): Unit = runBlocking { + val bodyRead = interceptWithContentType("Multipart/Mixed; boundary=graphql") + + assertThat(bodyRead).isFalse() + verify(fixture.scopes, never()).captureEvent(any(), any()) + } + + @Test + fun `does not read event stream response body`(): Unit = runBlocking { + val bodyRead = interceptWithContentType(" text/event-stream ; charset=utf-8") + + assertThat(bodyRead).isFalse() + verify(fixture.scopes, never()).captureEvent(any(), any()) + } + + @Test + fun `reads json response body for failed request detection`(): Unit = runBlocking { + val bodyRead = interceptWithContentType("application/json; charset=utf-8") + + assertThat(bodyRead).isTrue() + verify(fixture.scopes).captureEvent(any(), any()) + } + + @Test + fun `reads response body without content type for failed request detection`(): Unit = + runBlocking { + val bodyRead = interceptWithContentType(null) + + assertThat(bodyRead).isTrue() + verify(fixture.scopes).captureEvent(any(), any()) + } + + /** Runs the interceptor against a GraphQL error body and returns whether the body was read. */ + private suspend fun interceptWithContentType( + contentType: String?, + url: String = "https://target.example/graphql", + ): Boolean { + var bodyRead = false + val body = + object : ForwardingSource(Buffer().writeUtf8("{\"errors\":[{\"message\":\"error\"}]}")) { + override fun read(sink: Buffer, byteCount: Long): Long { + bodyRead = true + return super.read(sink, byteCount) + } + } + .buffer() + val request = HttpRequest.Builder(HttpMethod.Post, url).build() + val responseBuilder = HttpResponse.Builder(200).body(body) + contentType?.let { responseBuilder.addHeader("content-type", it) } + val response = responseBuilder.build() + val chain = + object : HttpInterceptorChain { + override suspend fun proceed(request: HttpRequest): HttpResponse = response + } + val interceptor = + SentryApollo4HttpInterceptor( + fixture.scopes, + captureFailedRequests = true, + failedRequestTargets = listOf("target.example"), + ) + + interceptor.intercept(request, chain) + return bodyRead + } + @Test fun `customizer modifies span`() { executeQuery(