From 161655ce9064a724bd99c48bd3c1bfb83aaf4c38 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 19 Aug 2026 14:50:23 -0400 Subject: [PATCH 1/3] Stop AppSecInterceptor from silently retrying failed okhttp requests chain.proceed(request) was wrapped in the same try/catch that guards the AppSec request/response hooks, so any IOException from the real network call (e.g. ConnectException) was swallowed and the request was silently retried via chain.proceed(chain.request()). This double- executes non-idempotent requests on transient network failures and surfaces the retry's own failure as an unhandled error blamed on the interceptor. Narrow the try/catch to only cover the AppSec hooks so genuine I/O failures propagate normally. Co-Authored-By: Claude Sonnet 5 --- .../okhttp2/AppSecInterceptor.java | 29 ++++++++++++------- .../okhttp3/AppSecInterceptor.java | 29 ++++++++++++------- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java index 7f55cc9a4fa..c42ffa3dbf8 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java @@ -45,23 +45,32 @@ public class AppSecInterceptor implements Interceptor { @Override public Response intercept(final Chain chain) throws IOException { + Request request = chain.request(); + final AgentSpan span = AgentTracer.activeSpan(); + final RequestContext ctx = span == null ? null : span.getRequestContext(); + if (ctx == null) { + return chain.proceed(request); + } + boolean sampled = false; try { - final AgentSpan span = AgentTracer.activeSpan(); - final RequestContext ctx = span == null ? null : span.getRequestContext(); - if (ctx == null) { - return chain.proceed(chain.request()); - } final long requestId = span.getSpanId(); - final boolean sampled = sampleRequest(ctx, requestId); + sampled = sampleRequest(ctx, requestId); final String url = span.getTag(Tags.HTTP_URL).toString(); - final Request request = onRequest(span, sampled, url, chain.request()); - final Response response = chain.proceed(request); + request = onRequest(span, sampled, url, request); + } catch (final BlockingException e) { + throw e; + } catch (final Exception e) { + LOGGER.debug("Failed to run AppSec request hooks", e); + } + // let real connection/IO failures propagate rather than swallowing and retrying the request + final Response response = chain.proceed(request); + try { return onResponse(span, sampled, response); } catch (final BlockingException e) { throw e; } catch (final Exception e) { - LOGGER.debug("Failed to intercept request", e); - return chain.proceed(chain.request()); + LOGGER.debug("Failed to run AppSec response hooks", e); + return response; } } diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java index e61a78003bb..b8bca4585b6 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java @@ -45,23 +45,32 @@ public class AppSecInterceptor implements Interceptor { @Override public Response intercept(final Chain chain) throws IOException { + Request request = chain.request(); + final AgentSpan span = AgentTracer.activeSpan(); + final RequestContext ctx = span == null ? null : span.getRequestContext(); + if (ctx == null) { + return chain.proceed(request); + } + boolean sampled = false; try { - final AgentSpan span = AgentTracer.activeSpan(); - final RequestContext ctx = span == null ? null : span.getRequestContext(); - if (ctx == null) { - return chain.proceed(chain.request()); - } final long requestId = span.getSpanId(); - final boolean sampled = sampleRequest(ctx, requestId); + sampled = sampleRequest(ctx, requestId); final String url = span.getTag(Tags.HTTP_URL).toString(); - final Request request = onRequest(span, sampled, url, chain.request()); - final Response response = chain.proceed(request); + request = onRequest(span, sampled, url, request); + } catch (final BlockingException e) { + throw e; + } catch (final Exception e) { + LOGGER.debug("Failed to run AppSec request hooks", e); + } + // let real connection/IO failures propagate rather than swallowing and retrying the request + final Response response = chain.proceed(request); + try { return onResponse(span, sampled, response); } catch (final BlockingException e) { throw e; } catch (final Exception e) { - LOGGER.debug("Failed to intercept request", e); - return chain.proceed(chain.request()); + LOGGER.debug("Failed to run AppSec response hooks", e); + return response; } } From efad53104c7c80f465fa8bc4aa83bb513c4e2750 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Fri, 21 Aug 2026 09:55:33 -0400 Subject: [PATCH 2/3] Add JUnit5 regression test for AppSecInterceptor silent-retry fix Covers both okhttp-2.2 and okhttp-3.0 AppSecInterceptor.intercept(): asserts an IOException from chain.proceed() propagates without being swallowed/retried, using Mockito + AgentTracer.forceRegister instead of Groovy/Spock. Co-Authored-By: Claude Sonnet 5 --- .../okhttp/okhttp-2.2/build.gradle | 3 + .../okhttp2/AppSecInterceptorTest.java | 68 +++++++++++++++++++ .../okhttp/okhttp-3.0/build.gradle | 3 + .../okhttp3/AppSecInterceptorTest.java | 68 +++++++++++++++++++ 4 files changed, 142 insertions(+) create mode 100644 dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/test/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptorTest.java create mode 100644 dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptorTest.java diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/build.gradle b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/build.gradle index 5f22c884db1..f9932024c1a 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/build.gradle +++ b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/build.gradle @@ -39,6 +39,9 @@ dependencies { } testImplementation group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0' + testImplementation libs.bundles.junit5 + testImplementation libs.bundles.mockito + testRuntimeOnly(project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter')) testRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-net:java-net-1.8')) diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/test/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptorTest.java b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/test/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptorTest.java new file mode 100644 index 00000000000..3319774af1f --- /dev/null +++ b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/test/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptorTest.java @@ -0,0 +1,68 @@ +package datadog.trace.instrumentation.okhttp2; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.squareup.okhttp.Interceptor; +import com.squareup.okhttp.Request; +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.io.IOException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class AppSecInterceptorTest { + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private Interceptor.Chain chain; + private Request request; + private final AppSecInterceptor interceptor = new AppSecInterceptor(); + + @BeforeEach + void setup() { + request = new Request.Builder().url("http://example.com").build(); + + final RequestContext requestContext = mock(RequestContext.class); + + final AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(requestContext); + when(span.getSpanId()).thenReturn(1L); + when(span.getTag(Tags.HTTP_URL)).thenReturn("http://example.com"); + + final AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))) + .thenReturn(CallbackProvider.CallbackProviderNoop.INSTANCE); + AgentTracer.forceRegister(tracer); + + chain = mock(Interceptor.Chain.class); + when(chain.request()).thenReturn(request); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @Test + void ioExceptionFromProceedPropagatesWithoutRetry() throws IOException { + final IOException failure = new IOException("boom"); + when(chain.proceed(request)).thenThrow(failure); + + final IOException thrown = assertThrows(IOException.class, () -> interceptor.intercept(chain)); + + assertSame(failure, thrown); + verify(chain, times(1)).proceed(request); + } +} diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle index 5ce7c997f99..3c5c051a169 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/build.gradle @@ -36,6 +36,9 @@ dependencies { latestDepTestImplementation group: 'com.squareup.okhttp3', name: 'okhttp', version: '[3.11.0, 4)' latestDepTestImplementation group: 'com.squareup.okio', name: 'okio', version: '1.+' + testImplementation libs.bundles.junit5 + testImplementation libs.bundles.mockito + testRuntimeOnly(project(':dd-java-agent:instrumentation:datadog:asm:iast-instrumenter')) testRuntimeOnly(project(':dd-java-agent:instrumentation:java:java-net:java-net-1.8')) } diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptorTest.java b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptorTest.java new file mode 100644 index 00000000000..56b230bb849 --- /dev/null +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/test/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptorTest.java @@ -0,0 +1,68 @@ +package datadog.trace.instrumentation.okhttp3; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.trace.api.gateway.CallbackProvider; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.io.IOException; +import okhttp3.Interceptor; +import okhttp3.Request; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class AppSecInterceptorTest { + + private final AgentTracer.TracerAPI originalTracer = AgentTracer.get(); + + private Interceptor.Chain chain; + private Request request; + private final AppSecInterceptor interceptor = new AppSecInterceptor(); + + @BeforeEach + void setup() { + request = new Request.Builder().url("http://example.com").build(); + + final RequestContext requestContext = mock(RequestContext.class); + + final AgentSpan span = mock(AgentSpan.class); + when(span.getRequestContext()).thenReturn(requestContext); + when(span.getSpanId()).thenReturn(1L); + when(span.getTag(Tags.HTTP_URL)).thenReturn("http://example.com"); + + final AgentTracer.TracerAPI tracer = mock(AgentTracer.TracerAPI.class); + when(tracer.activeSpan()).thenReturn(span); + when(tracer.getCallbackProvider(any(RequestContextSlot.class))) + .thenReturn(CallbackProvider.CallbackProviderNoop.INSTANCE); + AgentTracer.forceRegister(tracer); + + chain = mock(Interceptor.Chain.class); + when(chain.request()).thenReturn(request); + } + + @AfterEach + void tearDown() { + AgentTracer.forceRegister(originalTracer); + } + + @Test + void ioExceptionFromProceedPropagatesWithoutRetry() throws IOException { + final IOException failure = new IOException("boom"); + when(chain.proceed(request)).thenThrow(failure); + + final IOException thrown = assertThrows(IOException.class, () -> interceptor.intercept(chain)); + + assertSame(failure, thrown); + verify(chain, times(1)).proceed(request); + } +} From 5dc6c8801b8d4956a4d9984947eddbb114edf941 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 2 Sep 2026 19:58:52 -0400 Subject: [PATCH 3/3] Guard against NPE reading HTTP_URL tag in AppSecInterceptor span.getTag(Tags.HTTP_URL) can return null, and .toString() on it throws an NPE that silently skips the AppSec request hook for that call. Null-check instead of relying on the catch-all to swallow it. Co-Authored-By: Claude Sonnet 5 --- .../trace/instrumentation/okhttp2/AppSecInterceptor.java | 3 ++- .../trace/instrumentation/okhttp3/AppSecInterceptor.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java index c42ffa3dbf8..fbca97cd8e2 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-2.2/src/main/java/datadog/trace/instrumentation/okhttp2/AppSecInterceptor.java @@ -55,7 +55,8 @@ public Response intercept(final Chain chain) throws IOException { try { final long requestId = span.getSpanId(); sampled = sampleRequest(ctx, requestId); - final String url = span.getTag(Tags.HTTP_URL).toString(); + final Object urlTag = span.getTag(Tags.HTTP_URL); + final String url = urlTag == null ? null : urlTag.toString(); request = onRequest(span, sampled, url, request); } catch (final BlockingException e) { throw e; diff --git a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java index b8bca4585b6..abb3c7aba83 100644 --- a/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java +++ b/dd-java-agent/instrumentation/okhttp/okhttp-3.0/src/main/java/datadog/trace/instrumentation/okhttp3/AppSecInterceptor.java @@ -55,7 +55,8 @@ public Response intercept(final Chain chain) throws IOException { try { final long requestId = span.getSpanId(); sampled = sampleRequest(ctx, requestId); - final String url = span.getTag(Tags.HTTP_URL).toString(); + final Object urlTag = span.getTag(Tags.HTTP_URL); + final String url = urlTag == null ? null : urlTag.toString(); request = onRequest(span, sampled, url, request); } catch (final BlockingException e) { throw e;