From 739dc55b7809436e4364db36514a9be39b85a2f0 Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Tue, 29 Sep 2026 18:33:35 -0700 Subject: [PATCH] Replace direct servlet advice tests with instrumentation coverage --- ...tpServletRequestInstrumentationTest.groovy | 18 ++++++ .../servlet5/GetHttpSessionAdviceTest.java | 63 ------------------- .../src/test/groovy/JettyServlet3Test.groovy | 27 ++++++++ .../servlet3/GetHttpSessionAdviceTest.java | 63 ------------------- 4 files changed, 45 insertions(+), 126 deletions(-) delete mode 100644 dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/java/datadog/trace/instrumentation/servlet5/GetHttpSessionAdviceTest.java delete mode 100644 dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/java/datadog/trace/instrumentation/servlet3/GetHttpSessionAdviceTest.java diff --git a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy index f8d0d1811bf..c3db1613c4f 100644 --- a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy @@ -460,6 +460,24 @@ class JakartaHttpServletRequestInstrumentationTest extends InstrumentationSpecif suite << testSuite() } + void 'getSession tolerates a null servlet context'() { + setup: + final module = Mock(ApplicationModule) + InstrumentationBridge.registerIastModule(module) + final session = Mock(HttpSession) + final delegate = Mock(HttpServletRequest) + final request = new CustomRequest(request: delegate) + + when: + final result = request.getSession() + + then: + result.is(session) + 1 * delegate.getSession() >> session + 1 * delegate.getServletContext() >> null + 0 * module._ + } + protected E runUnderIastTrace(Closure cl) { final ddctx = new TagContext().withRequestContextDataIast(iastCtx) final span = TEST_TRACER.startSpan("test", "test-iast-span", ddctx) diff --git a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/java/datadog/trace/instrumentation/servlet5/GetHttpSessionAdviceTest.java b/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/java/datadog/trace/instrumentation/servlet5/GetHttpSessionAdviceTest.java deleted file mode 100644 index e6bd3b005ef..00000000000 --- a/dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/test/java/datadog/trace/instrumentation/servlet5/GetHttpSessionAdviceTest.java +++ /dev/null @@ -1,63 +0,0 @@ -package datadog.trace.instrumentation.servlet5; - -import static java.util.Collections.emptyList; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; - -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.sink.ApplicationModule; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.List; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -class GetHttpSessionAdviceTest { - - private final List moduleCalls = new ArrayList<>(); - private ApplicationModule previousModule; - - @BeforeEach - void registerModule() { - previousModule = InstrumentationBridge.APPLICATION; - InstrumentationBridge.APPLICATION = - proxy( - ApplicationModule.class, - (proxy, method, args) -> { - moduleCalls.add(method.getName()); - return null; - }); - } - - @AfterEach - void restoreModule() { - InstrumentationBridge.APPLICATION = previousModule; - } - - /** - * Request copies such as Wicket's {@code ServletRequestCopy} or Atmosphere's {@code NoOpsRequest} - * return a session but have no servlet context. The advice must not use that null context as a - * context store key (the weak map behind it throws on null keys). - */ - @Test - void ignoresRequestWithoutServletContext() { - HttpServletRequest request = proxy(HttpServletRequest.class, (proxy, method, args) -> null); - HttpSession session = proxy(HttpSession.class, (proxy, method, args) -> null); - - assertDoesNotThrow( - () -> - IastOptOutJakartaHttpServletRequestInstrumentation.GetHttpSessionAdvice.onExit( - request, session)); - - assertEquals(emptyList(), moduleCalls); - } - - @SuppressWarnings("unchecked") - private static T proxy(Class type, InvocationHandler handler) { - return (T) Proxy.newProxyInstance(type.getClassLoader(), new Class[] {type}, handler); - } -} diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy index 3271479c39b..d3373a40a3f 100644 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy +++ b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/groovy/JettyServlet3Test.groovy @@ -28,6 +28,7 @@ import javax.servlet.annotation.WebServlet import javax.servlet.http.HttpServlet import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import javax.servlet.http.HttpSession import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR @@ -570,6 +571,32 @@ class JettyServlet3ServeFromAsyncTimeout extends JettyServlet3Test { class IastJettyServlet3ForkedTest extends JettyServlet3TestSync { + void 'getSession tolerates a null servlet context'() { + setup: + final module = Mock(ApplicationModule) + InstrumentationBridge.registerIastModule(module) + final session = Mock(HttpSession) + final delegate = Mock(HttpServletRequest) + final request = new CustomRequest(request: delegate) + + when: + final result = request.getSession() + + then: + result.is(session) + 1 * delegate.getSession() >> session + 1 * delegate.getServletContext() >> null + 0 * module._ + + cleanup: + InstrumentationBridge.clearIastModules() + } + + private static class CustomRequest implements HttpServletRequest { + @Delegate + private HttpServletRequest request + } + @Override Class servlet() { return TestServlet3.GetSession diff --git a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/java/datadog/trace/instrumentation/servlet3/GetHttpSessionAdviceTest.java b/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/java/datadog/trace/instrumentation/servlet3/GetHttpSessionAdviceTest.java deleted file mode 100644 index 9e15950c095..00000000000 --- a/dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/test/java/datadog/trace/instrumentation/servlet3/GetHttpSessionAdviceTest.java +++ /dev/null @@ -1,63 +0,0 @@ -package datadog.trace.instrumentation.servlet3; - -import static java.util.Collections.emptyList; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; - -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.sink.ApplicationModule; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.List; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -class GetHttpSessionAdviceTest { - - private final List moduleCalls = new ArrayList<>(); - private ApplicationModule previousModule; - - @BeforeEach - void registerModule() { - previousModule = InstrumentationBridge.APPLICATION; - InstrumentationBridge.APPLICATION = - proxy( - ApplicationModule.class, - (proxy, method, args) -> { - moduleCalls.add(method.getName()); - return null; - }); - } - - @AfterEach - void restoreModule() { - InstrumentationBridge.APPLICATION = previousModule; - } - - /** - * Request copies such as Wicket's {@code ServletRequestCopy} or Atmosphere's {@code NoOpsRequest} - * return a session but have no servlet context. The advice must not use that null context as a - * context store key (the weak map behind it throws on null keys). - */ - @Test - void ignoresRequestWithoutServletContext() { - HttpServletRequest request = proxy(HttpServletRequest.class, (proxy, method, args) -> null); - HttpSession session = proxy(HttpSession.class, (proxy, method, args) -> null); - - assertDoesNotThrow( - () -> - IastOptOutHttpServletRequest3Instrumentation.GetHttpSessionAdvice.onExit( - request, session)); - - assertEquals(emptyList(), moduleCalls); - } - - @SuppressWarnings("unchecked") - private static T proxy(Class type, InvocationHandler handler) { - return (T) Proxy.newProxyInstance(type.getClassLoader(), new Class[] {type}, handler); - } -}