From 9071851aea7c13e3d82e286f7bf2866e60d1ec83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Vandon?= Date: Thu, 18 Jun 2026 15:07:59 +0200 Subject: [PATCH 1/2] fix for APMSP-3274 --- .../ApacheHttpAsyncClientModule.java | 3 +- ...acheHttpClientRedirectInstrumentation.java | 27 +++--- .../apachehttpasyncclient/RedirectHelper.java | 72 ++++++++++++++++ ...HttpClientRedirectInstrumentationTest.java | 85 +++++++++++++++++++ 4 files changed, 169 insertions(+), 18 deletions(-) create mode 100644 dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/RedirectHelper.java create mode 100644 dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientModule.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientModule.java index 15a0a582b40..8dcec31c16f 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientModule.java +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientModule.java @@ -22,7 +22,8 @@ public String[] helperClassNames() { packageName + ".DelegatingRequestProducer", packageName + ".TraceContinuedFutureCallback", packageName + ".ApacheHttpAsyncClientDecorator", - packageName + ".HostAndRequestAsHttpUriRequest" + packageName + ".HostAndRequestAsHttpUriRequest", + packageName + ".RedirectHelper" }; } diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java index 15890fb4571..8e0e7955416 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java @@ -46,7 +46,7 @@ public void methodAdvice(MethodTransformer transformer) { public static class ClientRedirectAdvice { @Advice.OnMethodExit(suppress = Throwable.class) - private static void onAfterExecute( + static void onAfterExecute( @Advice.Argument(value = 2) final HttpContext context, @Advice.Return(typing = Assigner.Typing.DYNAMIC) final HttpRequest redirect) { if (redirect == null) { @@ -58,22 +58,15 @@ private static void onAfterExecute( } HttpRequest original = (HttpRequest) originalRequest; - // Apache HttpClient 4.0.1+ copies headers from original to redirect only - // if redirect headers are empty. Because we add headers - // "x-datadog-" and "x-b3-" to redirect: it means redirect headers never - // will be empty. So in case if not-instrumented redirect had no headers, - // we just copy all not set headers from original to redirect (doing same - // thing as apache httpclient does). - if (!redirect.headerIterator().hasNext()) { - // redirect didn't have other headers besides tracing, so we need to do copy - // (same work as Apache HttpClient 4.0.1+ does w/o instrumentation) - if (original instanceof HttpRequestWrapper) { - // We should use the initial request because the wrapped one might contain more headers - // (i.e. Host) we do not want to copy - // if we cannot access the original request we cannot safely copy. - // At this point we break the propagation not to corrupt the customer request - redirect.setHeaders(((HttpRequestWrapper) original).getOriginal().getAllHeaders()); - } + // Apache HttpClient 4.0.1+ copies headers from the original request to the redirect only when + // the redirect request has no headers. Because tracing injects propagation headers before + // redirect handling completes, an otherwise empty redirect request may no longer look empty + // to HttpClient. Preserve that header-copy behavior only for same-origin redirects; + // cross-origin redirects must not receive application headers such as Authorization or + // Cookie. + if (!redirect.headerIterator().hasNext() + && RedirectHelper.isSameOrigin(context, original, redirect)) { + redirect.setHeaders(((HttpRequestWrapper) original).getOriginal().getAllHeaders()); } else { for (final Header header : original.getAllHeaders()) { if (PropagationUtils.KNOWN_PROPAGATION_HEADERS.contains( diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/RedirectHelper.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/RedirectHelper.java new file mode 100644 index 00000000000..abda2997bbe --- /dev/null +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/RedirectHelper.java @@ -0,0 +1,72 @@ +package datadog.trace.instrumentation.apachehttpasyncclient; + +import java.net.URI; +import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; +import org.apache.http.client.methods.HttpRequestWrapper; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.protocol.HttpContext; + +public final class RedirectHelper { + private RedirectHelper() {} + + public static boolean isSameOrigin( + final HttpContext context, final HttpRequest original, final HttpRequest redirect) { + if (!(original instanceof HttpRequestWrapper) || !(redirect instanceof HttpUriRequest)) { + return false; + } + HttpRequest unwrappedOriginal = ((HttpRequestWrapper) original).getOriginal(); + if (!(unwrappedOriginal instanceof HttpUriRequest)) { + return false; + } + URI originalUri = ((HttpUriRequest) unwrappedOriginal).getURI(); + URI redirectUri = ((HttpUriRequest) redirect).getURI(); + if (originalUri == null || redirectUri == null) { + return false; + } + + originalUri = resolveOriginalUri(context, originalUri); + if (!redirectUri.isAbsolute()) { + redirectUri = originalUri.resolve(redirectUri); + } + if (originalUri.getScheme() == null || redirectUri.getScheme() == null) { + return false; + } + + String originalHost = originalUri.getHost(); + String redirectHost = redirectUri.getHost(); + if (originalHost == null || redirectHost == null) { + return false; + } + + return originalUri.getScheme().equalsIgnoreCase(redirectUri.getScheme()) + && originalHost.equalsIgnoreCase(redirectHost) + && effectivePort(originalUri) == effectivePort(redirectUri); + } + + private static URI resolveOriginalUri(final HttpContext context, final URI originalUri) { + if (originalUri.isAbsolute()) { + return originalUri; + } + Object targetHost = context.getAttribute("http.target_host"); + if (!(targetHost instanceof HttpHost)) { + return originalUri; + } + HttpHost host = (HttpHost) targetHost; + return URI.create(host.toURI()).resolve(originalUri); + } + + private static int effectivePort(final URI uri) { + if (uri.getPort() != -1) { + return uri.getPort(); + } + String scheme = uri.getScheme(); + if ("http".equalsIgnoreCase(scheme)) { + return 80; + } + if ("https".equalsIgnoreCase(scheme)) { + return 443; + } + return -1; + } +} diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java new file mode 100644 index 00000000000..f24876bd72e --- /dev/null +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java @@ -0,0 +1,85 @@ +package datadog.trace.instrumentation.apachehttpasyncclient; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import org.apache.http.HttpHost; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpRequestWrapper; +import org.apache.http.protocol.BasicHttpContext; +import org.apache.http.protocol.HttpContext; +import org.junit.jupiter.api.Test; + +class ApacheHttpClientRedirectInstrumentationTest { + + @Test + void doesNotCopyApplicationHeadersToCrossOriginRedirects() throws Exception { + HttpGet original = originalRequest("http://example.com/request"); + original.addHeader("Authorization", "Bearer secret"); + original.addHeader("Cookie", "session=secret"); + original.addHeader("X-Api-Key", "secret"); + original.addHeader("x-datadog-trace-id", "123"); + + HttpGet redirect = new HttpGet("http://attacker.example/redirect"); + + ApacheHttpClientRedirectInstrumentation.ClientRedirectAdvice.onAfterExecute( + contextWith(original), redirect); + + assertFalse(redirect.containsHeader("Authorization")); + assertFalse(redirect.containsHeader("Cookie")); + assertFalse(redirect.containsHeader("X-Api-Key")); + assertEquals("123", redirect.getFirstHeader("x-datadog-trace-id").getValue()); + } + + @Test + void copiesApplicationHeadersToSameOriginRedirects() throws Exception { + HttpGet original = originalRequest("https://example.com/request"); + original.addHeader("Authorization", "Bearer secret"); + original.addHeader("x-datadog-trace-id", "123"); + + HttpGet redirect = new HttpGet("https://example.com/redirect"); + + ApacheHttpClientRedirectInstrumentation.ClientRedirectAdvice.onAfterExecute( + contextWith(original), redirect); + + assertEquals("Bearer secret", redirect.getFirstHeader("Authorization").getValue()); + assertEquals("123", redirect.getFirstHeader("x-datadog-trace-id").getValue()); + } + + @Test + void treatsDefaultPortsAsSameOrigin() throws Exception { + HttpGet original = originalRequest("https://example.com:443/request"); + original.addHeader("Authorization", "Bearer secret"); + + HttpGet redirect = new HttpGet("https://example.com/redirect"); + + ApacheHttpClientRedirectInstrumentation.ClientRedirectAdvice.onAfterExecute( + contextWith(original), redirect); + + assertEquals("Bearer secret", redirect.getFirstHeader("Authorization").getValue()); + } + + @Test + void resolvesHostRequestOriginFromContext() throws Exception { + HttpGet original = originalRequest("/request"); + original.addHeader("Authorization", "Bearer secret"); + + HttpGet redirect = new HttpGet("http://example.com/redirect"); + HttpContext context = contextWith(original); + context.setAttribute("http.target_host", new HttpHost("example.com", 80, "http")); + + ApacheHttpClientRedirectInstrumentation.ClientRedirectAdvice.onAfterExecute(context, redirect); + + assertEquals("Bearer secret", redirect.getFirstHeader("Authorization").getValue()); + } + + private static HttpGet originalRequest(final String uri) { + return new HttpGet(uri); + } + + private static HttpContext contextWith(final HttpGet request) throws Exception { + HttpContext context = new BasicHttpContext(); + context.setAttribute("http.request", HttpRequestWrapper.wrap(request)); + return context; + } +} From f7025272a0d11ad1373625577c94c2e5d297fe66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Vandon?= Date: Thu, 18 Jun 2026 17:02:46 +0200 Subject: [PATCH 2/2] add empty marker header to prevent future blind copy --- .../ApacheHttpClientRedirectInstrumentation.java | 11 +++++++++-- ...cheHttpClientRedirectInstrumentationTest.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java index 8e0e7955416..cd6473b1285 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentation.java @@ -64,18 +64,25 @@ static void onAfterExecute( // to HttpClient. Preserve that header-copy behavior only for same-origin redirects; // cross-origin redirects must not receive application headers such as Authorization or // Cookie. - if (!redirect.headerIterator().hasNext() - && RedirectHelper.isSameOrigin(context, original, redirect)) { + boolean emptyRedirect = !redirect.headerIterator().hasNext(); + if (emptyRedirect && RedirectHelper.isSameOrigin(context, original, redirect)) { redirect.setHeaders(((HttpRequestWrapper) original).getOriginal().getAllHeaders()); } else { + boolean copiedPropagationHeader = false; for (final Header header : original.getAllHeaders()) { if (PropagationUtils.KNOWN_PROPAGATION_HEADERS.contains( header.getName().toLowerCase(Locale.ROOT))) { if (!redirect.containsHeader(header.getName())) { redirect.setHeader(header.getName(), header.getValue()); + copiedPropagationHeader = true; } } } + if (emptyRedirect && !copiedPropagationHeader) { + // When there are no propagation headers to copy, add a harmless header to keep HttpClient + // from treating the redirect as empty and copying application headers later. + redirect.setHeader("x-datadog-redirect", "true"); + } } } } diff --git a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java index f24876bd72e..83892ef07a5 100644 --- a/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java +++ b/dd-java-agent/instrumentation/apache-httpclient/apache-httpasyncclient-4.0/src/test/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpClientRedirectInstrumentationTest.java @@ -31,6 +31,22 @@ void doesNotCopyApplicationHeadersToCrossOriginRedirects() throws Exception { assertEquals("123", redirect.getFirstHeader("x-datadog-trace-id").getValue()); } + @Test + void preventsApacheHeaderCopyWhenCrossOriginRedirectHasNoPropagationHeaders() throws Exception { + HttpGet original = originalRequest("http://example.com/request"); + original.addHeader("Authorization", "Bearer secret"); + original.addHeader("Cookie", "session=secret"); + + HttpGet redirect = new HttpGet("http://attacker.example/redirect"); + + ApacheHttpClientRedirectInstrumentation.ClientRedirectAdvice.onAfterExecute( + contextWith(original), redirect); + + assertFalse(redirect.containsHeader("Authorization")); + assertFalse(redirect.containsHeader("Cookie")); + assertEquals("true", redirect.getFirstHeader("x-datadog-redirect").getValue()); + } + @Test void copiesApplicationHeadersToSameOriginRedirects() throws Exception { HttpGet original = originalRequest("https://example.com/request");