Remove authority from URLs sent to Sentry (#2366)

Co-authored-by: Ivan Dlugos <[email protected]>
getsentry · Jan 30, 2023 · 5fa24ec · 5fa24ec
1 parent 9ab45a7
commit 5fa24ec
Show file tree

Hide file tree

Showing 22 changed files with 575 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366))
+
 ### Dependencies
 
 - Bump Native SDK from v0.5.3 to v0.5.4 ([#2500](https://github.com/getsentry/sentry-java/pull/2500))

diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt
@@ -16,6 +16,7 @@ import io.sentry.exception.SentryHttpClientException
 import io.sentry.protocol.Mechanism
 import io.sentry.util.HttpUtils
 import io.sentry.util.PropagationTargetsUtils
+import io.sentry.util.UrlUtils
 import okhttp3.Headers
 import okhttp3.Interceptor
 import okhttp3.Request
@@ -53,11 +54,13 @@ class SentryOkHttpInterceptor(
     override fun intercept(chain: Interceptor.Chain): Response {
         var request = chain.request()
 
-        val url = request.url.toString()
+        val urlDetails = UrlUtils.parse(request.url.toString())
+        val url = urlDetails.urlOrFallback
         val method = request.method
 
         // read transaction from the bound scope
         val span = hub.span?.startChild("http.client", "$method $url")
+        urlDetails.applyToSpan(span)
 
         var response: Response? = null
 
@@ -149,20 +152,10 @@ class SentryOkHttpInterceptor(
         // 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
-        var requestUrl = request.url.toString()
-
-        val query = request.url.query
-        if (!query.isNullOrEmpty()) {
-            requestUrl = requestUrl.replace("?$query", "")
-        }
-
-        val urlFragment = request.url.fragment
-        if (!urlFragment.isNullOrEmpty()) {
-            requestUrl = requestUrl.replace("#$urlFragment", "")
-        }
+        val urlDetails = UrlUtils.parse(request.url.toString())
 
         // return if its not a target match
-        if (!PropagationTargetsUtils.contain(failedRequestTargets, requestUrl)) {
+        if (!PropagationTargetsUtils.contain(failedRequestTargets, request.url.toString())) {
             return
         }
 
@@ -180,13 +173,11 @@ class SentryOkHttpInterceptor(
         hint.set(OKHTTP_RESPONSE, response)
 
         val sentryRequest = io.sentry.protocol.Request().apply {
-            url = requestUrl
+            urlDetails.applyToRequest(this)
             // Cookie is only sent if isSendDefaultPii is enabled
             cookies = if (hub.options.isSendDefaultPii) request.headers["Cookie"] else null
             method = request.method
-            queryString = query
             headers = getHeaders(request.headers)
-            fragment = urlFragment
 
             request.body?.contentLength().ifHasValidLength {
                 bodySize = it

diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt
@@ -263,6 +263,8 @@ class SentryOkHttpInterceptorTest {
     @SuppressWarnings("SwallowedException")
     @Test
     fun `adds breadcrumb when http calls results in exception`() {
+        // to setup mocks
+        fixture.getSut()
         val interceptor = SentryOkHttpInterceptor(fixture.hub)
         val chain = mock<Interceptor.Chain>()
         whenever(chain.proceed(any())).thenThrow(IOException())
@@ -408,7 +410,8 @@ class SentryOkHttpInterceptorTest {
         val sut = fixture.getSut(
             captureFailedRequests = true,
             httpStatusCode = statusCode,
-            responseBody = "fail"
+            responseBody = "fail",
+            sendDefaultPii = true
         )
 
         val request = getRequest(url = "/hello?myQuery=myValue#myFragment")
@@ -423,16 +426,16 @@ class SentryOkHttpInterceptorTest {
                 assertEquals("GET", sentryRequest.method)
 
                 // because of isSendDefaultPii
-                assertNull(sentryRequest.headers)
+                assertNotNull(sentryRequest.headers)
                 assertNull(sentryRequest.cookies)
 
                 val sentryResponse = it.contexts.response!!
                 assertEquals(statusCode, sentryResponse.statusCode)
                 assertEquals(response.body!!.contentLength(), sentryResponse.bodySize)
 
                 // because of isSendDefaultPii
-                assertNull(sentryRequest.headers)
-                assertNull(sentryRequest.cookies)
+                assertNotNull(sentryResponse.headers)
+                assertNull(sentryResponse.cookies)
 
                 assertTrue(it.throwable is SentryHttpClientException)
             },
@@ -489,6 +492,8 @@ class SentryOkHttpInterceptorTest {
     @SuppressWarnings("SwallowedException")
     @Test
     fun `does not capture an error even if it throws`() {
+        // to setup mocks
+        fixture.getSut()
         val interceptor = SentryOkHttpInterceptor(
             fixture.hub,
             captureFailedRequests = true

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
@@ -17,6 +17,7 @@ import io.sentry.SentryLevel
 import io.sentry.SpanStatus
 import io.sentry.TypeCheckHint
 import io.sentry.util.PropagationTargetsUtils
+import io.sentry.util.UrlUtils
 
 class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null) :
     HttpInterceptor {
@@ -80,17 +81,19 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH
     }
 
     private fun startChild(request: HttpRequest, activeSpan: ISpan): ISpan {
-        val url = request.url
+        val urlDetails = UrlUtils.parse(request.url)
         val method = request.method
 
         val operationName = operationNameFromHeaders(request)
         val operation = operationName ?: "apollo.client"
         val operationType = request.valueForHeader(SENTRY_APOLLO_3_OPERATION_TYPE) ?: method
         val operationId = request.valueForHeader("X-APOLLO-OPERATION-ID")
         val variables = request.valueForHeader(SENTRY_APOLLO_3_VARIABLES)
-        val description = "$operationType ${operationName ?: url}"
+        val description = "$operationType ${operationName ?: urlDetails.urlOrFallback}"
 
         return activeSpan.startChild(operation, description).apply {
+            urlDetails.applyToSpan(this)
+
             operationId?.let {
                 setData("operationId", it)
             }
@@ -121,8 +124,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH
         }
         span.finish()
 
-        val breadcrumb =
-            Breadcrumb.http(request.url, request.method.name, statusCode)
+        val breadcrumb = Breadcrumb.http(request.url, request.method.name, statusCode)
 
         request.body?.contentLength.ifHasValidLength { contentLength ->
             breadcrumb.setData("request_body_size", contentLength)

diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java
@@ -15,6 +15,7 @@
 import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import io.sentry.util.PropagationTargetsUtils;
+import io.sentry.util.UrlUtils;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -50,13 +51,15 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O
       }
 
       ISpan span = activeSpan.startChild("http.client");
-      String url = request.url();
-      span.setDescription(request.httpMethod().name() + " " + url);
+      final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.url());
+      span.setDescription(request.httpMethod().name() + " " + urlDetails.getUrlOrFallback());
+      urlDetails.applyToSpan(span);
 
       final RequestWrapper requestWrapper = new RequestWrapper(request);
 
       if (!span.isNoOp()
-          && PropagationTargetsUtils.contain(hub.getOptions().getTracePropagationTargets(), url)) {
+          && PropagationTargetsUtils.contain(
+              hub.getOptions().getTracePropagationTargets(), request.url())) {
         final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
         final @Nullable Collection<String> requestBaggageHeader =
             request.headers().get(BaggageHeader.BAGGAGE_HEADER);

diff --git a/...rta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java b/...rta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java
@@ -6,6 +6,7 @@
 import io.sentry.protocol.Request;
 import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
+import io.sentry.util.UrlUtils;
 import jakarta.servlet.http.HttpServletRequest;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -30,8 +31,10 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http
   public @NotNull SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) {
     final Request sentryRequest = new Request();
     sentryRequest.setMethod(httpRequest.getMethod());
+    final @NotNull UrlUtils.UrlDetails urlDetails =
+        UrlUtils.parse(httpRequest.getRequestURL().toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setQueryString(httpRequest.getQueryString());
-    sentryRequest.setUrl(httpRequest.getRequestURL().toString());
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
 
     event.setRequest(sentryRequest);

diff --git a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java
@@ -6,6 +6,7 @@
 import io.sentry.protocol.Request;
 import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
+import io.sentry.util.UrlUtils;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
@@ -30,8 +31,10 @@ public SentryRequestHttpServletRequestProcessor(@NotNull HttpServletRequest http
   public @NotNull SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) {
     final Request sentryRequest = new Request();
     sentryRequest.setMethod(httpRequest.getMethod());
+    final @NotNull UrlUtils.UrlDetails urlDetails =
+        UrlUtils.parse(httpRequest.getRequestURL().toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setQueryString(httpRequest.getQueryString());
-    sentryRequest.setUrl(httpRequest.getRequestURL().toString());
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
 
     event.setRequest(sentryRequest);

diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestResolver.java
@@ -9,6 +9,8 @@
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
+
+import io.sentry.util.UrlUtils;
 import jakarta.servlet.http.HttpServletRequest;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -26,8 +28,9 @@ public SentryRequestResolver(final @NotNull IHub hub) {
   public @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) {
     final Request sentryRequest = new Request();
     sentryRequest.setMethod(httpRequest.getMethod());
+    final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(httpRequest.getRequestURL().toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setQueryString(httpRequest.getQueryString());
-    sentryRequest.setUrl(httpRequest.getRequestURL().toString());
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
 
     if (hub.getOptions().isSendDefaultPii()) {

diff --git a/...rc/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/...rc/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java
@@ -14,6 +14,8 @@
 import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import io.sentry.util.PropagationTargetsUtils;
+import io.sentry.util.UrlUtils;
+
 import java.io.IOException;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -47,7 +49,9 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
       final ISpan span = activeSpan.startChild("http.client");
       final String methodName =
           request.getMethod() != null ? request.getMethod().name() : "unknown";
-      span.setDescription(methodName + " " + request.getURI());
+      final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.getURI().toString());
+      span.setDescription(methodName + " " + urlDetails.getUrlOrFallback());
+      urlDetails.applyToSpan(span);
 
       if (!span.isNoOp() && PropagationTargetsUtils.contain(
           hub.getOptions().getTracePropagationTargets(), request.getURI())) {

diff --git a/...-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java b/...-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryRequestResolver.java
@@ -5,6 +5,9 @@
 import io.sentry.protocol.Request;
 import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
+import io.sentry.util.UrlUtils;
+
+import java.net.URI;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -28,8 +31,9 @@ public SentryRequestResolver(final @NotNull IHub hub) {
     final String methodName =
       httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown";
     sentryRequest.setMethod(methodName);
-    sentryRequest.setQueryString(httpRequest.getURI().getQuery());
-    sentryRequest.setUrl(httpRequest.getURI().toString());
+    final @NotNull URI uri = httpRequest.getURI();
+    final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(uri.toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders()));
 
     if (hub.getOptions().isSendDefaultPii()) {

diff --git a/...-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt b/...-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt
@@ -38,6 +38,7 @@ import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
+import kotlin.test.assertNull
 
 @RunWith(SpringRunner::class)
 @SpringBootTest(
@@ -63,17 +64,18 @@ class SentryWebfluxIntegrationTest {
     @Test
     fun `attaches request information to SentryEvents`() {
         testClient.get()
-            .uri("http://localhost:$port/hello?param=value")
+            .uri("http://localhost:$port/hello?param=value#top")
             .exchange()
             .expectStatus()
             .isOk
 
         verify(transport).send(
             checkEvent { event ->
                 assertNotNull(event.request) {
-                    assertEquals("http://localhost:$port/hello?param=value", it.url)
+                    assertEquals("http://localhost:$port/hello", it.url)
                     assertEquals("GET", it.method)
                     assertEquals("param=value", it.queryString)
+                    assertNull(it.fragment)
                 }
             },
             anyOrNull()

diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java
@@ -5,6 +5,7 @@
 import io.sentry.protocol.Request;
 import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
+import io.sentry.util.UrlUtils;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
@@ -26,8 +27,10 @@ public SentryRequestResolver(final @NotNull IHub hub) {
   public @NotNull Request resolveSentryRequest(final @NotNull HttpServletRequest httpRequest) {
     final Request sentryRequest = new Request();
     sentryRequest.setMethod(httpRequest.getMethod());
+    final @NotNull UrlUtils.UrlDetails urlDetails =
+        UrlUtils.parse(httpRequest.getRequestURL().toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setQueryString(httpRequest.getQueryString());
-    sentryRequest.setUrl(httpRequest.getRequestURL().toString());
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest));
 
     if (hub.getOptions().isSendDefaultPii()) {

diff --git a/...spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/...spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java
@@ -14,6 +14,7 @@
 import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import io.sentry.util.PropagationTargetsUtils;
+import io.sentry.util.UrlUtils;
 import java.io.IOException;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -47,7 +48,9 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
       final ISpan span = activeSpan.startChild("http.client");
       final String methodName =
           request.getMethod() != null ? request.getMethod().name() : "unknown";
-      span.setDescription(methodName + " " + request.getURI());
+      final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.getURI().toString());
+      urlDetails.applyToSpan(span);
+      span.setDescription(methodName + " " + urlDetails.getUrlOrFallback());
 
       if (!span.isNoOp()
           && PropagationTargetsUtils.contain(

diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java
@@ -13,6 +13,7 @@
 import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import io.sentry.util.PropagationTargetsUtils;
+import io.sentry.util.UrlUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.springframework.web.reactive.function.client.ClientRequest;
@@ -39,7 +40,9 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) {
     }
 
     final ISpan span = activeSpan.startChild("http.client");
-    span.setDescription(request.method().name() + " " + request.url());
+    final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(request.url().toString());
+    span.setDescription(request.method().name() + " " + urlDetails.getUrlOrFallback());
+    urlDetails.applyToSpan(span);
 
     final ClientRequest.Builder requestBuilder = ClientRequest.from(request);
 

diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java
@@ -5,6 +5,8 @@
 import io.sentry.protocol.Request;
 import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
+import io.sentry.util.UrlUtils;
+import java.net.URI;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -28,8 +30,9 @@ public SentryRequestResolver(final @NotNull IHub hub) {
     final String methodName =
         httpRequest.getMethod() != null ? httpRequest.getMethod().name() : "unknown";
     sentryRequest.setMethod(methodName);
-    sentryRequest.setQueryString(httpRequest.getURI().getQuery());
-    sentryRequest.setUrl(httpRequest.getURI().toString());
+    final @NotNull URI uri = httpRequest.getURI();
+    final @NotNull UrlUtils.UrlDetails urlDetails = UrlUtils.parse(uri.toString());
+    urlDetails.applyToRequest(sentryRequest);
     sentryRequest.setHeaders(resolveHeadersMap(httpRequest.getHeaders()));
 
     if (hub.getOptions().isSendDefaultPii()) {