Fix stack overflow in spock making junit ignore test failures (#7674)

Fix stack overflow on TooManyInvocationsError

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java

@@ -249,9 +249,14 @@ public List<Instrumenter> typeInstrumentations() {
 
     @Override
     public boolean isApplicable(Set<TargetSystem> enabledSystems) {
-      return enabledSystems.contains(TargetSystem.IAST)
-          || (isOptOutEnabled()
-              && InstrumenterConfig.get().getAppSecActivation() == ProductActivation.FULLY_ENABLED);
+      if (enabledSystems.contains(TargetSystem.IAST)) {
+        return true;
+      }
+      final InstrumenterConfig cfg = InstrumenterConfig.get();
+      if (!isOptOutEnabled() || cfg.isIastFullyDisabled()) {
+        return false;
+      }
+      return cfg.getAppSecActivation() == ProductActivation.FULLY_ENABLED;
     }
 
     /**

dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy

@@ -57,7 +57,7 @@ class StringBuilderCallSiteTest extends AgentTestRunner {
     if (param.class == String) {
       1 * iastModule.onStringBuilderAppend(target, (String) param)
     } else {
-      1 * iastModule.onStringBuilderAppend(target, param.toString())
+      1 * iastModule.onStringBuilderAppend(target, { it -> it.toString() == param.toString() } )
     }
     _ * TEST_PROFILING_CONTEXT_INTEGRATION._
     0 * _

dd-java-agent/instrumentation/owasp-esapi-2/src/test/groovy/datadog/trace/instrumentation/owasp/esapi/EncoderCallSiteTest.groovy

@@ -25,7 +25,7 @@ class EncoderCallSiteTest extends AgentTestRunner {
     testSuite.&"$method".call(args)
 
     then:
-    1 * module.taintObjectIfTainted(_, _, false, mark)
+    1 * module.taintStringIfTainted(_, _, false, mark)
     0 * module._
 
     where:

dd-java-agent/instrumentation/tomcat-appsec-7/src/test/groovy/datadog/trace/instrumentation/tomcat7/ErrorReportValueInstrumentationTest.groovy → dd-java-agent/instrumentation/tomcat-appsec-7/src/test/groovy/datadog/trace/instrumentation/tomcat7/ErrorReportValueInstrumentationForkedTest.groovy

@@ -7,7 +7,7 @@ import org.apache.catalina.connector.Request
 import org.apache.catalina.connector.Response
 import org.apache.catalina.valves.ErrorReportValve
 
-class ErrorReportValueInstrumentationTest extends AgentTestRunner {
+class ErrorReportValueInstrumentationForkedTest extends AgentTestRunner {
 
   void 'test vulnerability detection'() {
     given:
@@ -37,7 +37,7 @@ class ErrorReportValueInstrumentationTest extends AgentTestRunner {
   }
 }
 
-class AppSecErrorReportValueInstrumentationTest extends ErrorReportValueInstrumentationTest {
+class AppSecErrorReportValueInstrumentationForkedTest extends ErrorReportValueInstrumentationForkedTest {
 
   @Override
   protected void configurePreAgent() {
@@ -51,7 +51,7 @@ class AppSecErrorReportValueInstrumentationTest extends ErrorReportValueInstrume
   }
 }
 
-class IastErrorReportValueInstrumentationTest extends ErrorReportValueInstrumentationTest {
+class IastErrorReportValueInstrumentationForkedTest extends ErrorReportValueInstrumentationForkedTest {
 
   @Override
   protected void configurePreAgent() {
@@ -65,7 +65,7 @@ class IastErrorReportValueInstrumentationTest extends ErrorReportValueInstrument
   }
 }
 
-class IastDisabledErrorReportValueInstrumentationTest extends ErrorReportValueInstrumentationTest {
+class IastDisabledErrorReportValueInstrumentationForkedTest extends ErrorReportValueInstrumentationForkedTest {
 
   @Override
   protected void configurePreAgent() {

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/SpockRunner.java

@@ -9,13 +9,18 @@
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.jar.JarFile;
 import net.bytebuddy.agent.ByteBuddyAgent;
 import net.bytebuddy.dynamic.ClassFileLocator;
 import org.junit.platform.runner.JUnitPlatform;
+import org.junit.runner.notification.Failure;
+import org.junit.runner.notification.RunListener;
 import org.junit.runner.notification.RunNotifier;
+import org.spockframework.mock.IMockInvocation;
+import org.spockframework.mock.TooManyInvocationsError;
 
 /**
  * Runs a spock test in an agent-friendly way.
@@ -129,10 +134,13 @@ private static Class<?> shadowTestClass(final Class<?> clazz) {
   @Override
   public void run(final RunNotifier notifier) {
     final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader();
+    final RunListener listener = new TooManyInvocationsErrorListener();
     try {
       Thread.currentThread().setContextClassLoader(customLoader);
+      notifier.addFirstListener(listener);
       super.run(notifier);
     } finally {
+      notifier.removeListener(listener);
       Thread.currentThread().setContextClassLoader(contextLoader);
     }
   }
@@ -222,4 +230,51 @@ protected Class<?> loadClass(final String name, final boolean resolve)
       }
     }
   }
+
+  /**
+   * This class tries to fix {@link TooManyInvocationsError} exceptions when the assertion error is
+   * caught by a mock triggering a stack overflow while composing the failure message.
+   */
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  @RunListener.ThreadSafe
+  private static class TooManyInvocationsErrorListener extends RunListener {
+
+    @Override
+    public void testFailure(final Failure failure) throws Exception {
+      if (failure.getException() instanceof TooManyInvocationsError) {
+        final TooManyInvocationsError assertion = (TooManyInvocationsError) failure.getException();
+        try {
+          // try to trigger an error (e.g. stack overflow)
+          assertion.getMessage();
+        } catch (final Throwable e) {
+          fixTooManyInvocationsError(assertion);
+        }
+      }
+    }
+
+    private void fixTooManyInvocationsError(final TooManyInvocationsError error) {
+      final List<IMockInvocation> accepted = error.getAcceptedInvocations();
+      for (final IMockInvocation invocation : accepted) {
+        try {
+          invocation.toString();
+        } catch (final Throwable t) {
+          final List<Object> arguments = invocation.getArguments();
+          for (int i = 0; i < arguments.size(); i++) {
+            final Object arg = arguments.get(i);
+            if (arg instanceof AssertionError) {
+              final AssertionError updatedAssertion =
+                  new AssertionError(
+                      "'"
+                          + arg.getClass().getName()
+                          + "' hidden due to '"
+                          + t.getClass().getName()
+                          + "'",
+                      t);
+              invocation.getArguments().set(i, updatedAssertion);
+            }
+          }
+        }
+      }
+    }
+  }
 }

dd-java-agent/testing/src/test/groovy/TooManyInvocationsErrorListenerTest.groovy

@@ -0,0 +1,38 @@
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.SpockRunner
+import org.junit.runner.Description
+import org.junit.runner.notification.Failure
+import org.spockframework.mock.IMockInteraction
+import org.spockframework.mock.IMockMethod
+import org.spockframework.mock.IMockObject
+import org.spockframework.mock.IResponseGenerator
+import org.spockframework.mock.TooManyInvocationsError
+import org.spockframework.mock.runtime.MockInvocation
+
+class TooManyInvocationsErrorListenerTest extends AgentTestRunner {
+
+  @SuppressWarnings('GroovyAccessibility')
+  void 'test that listener modifies failure'() {
+    setup:
+    final error = new TooManyInvocationsError(Stub(IMockInteraction), [])
+    error.acceptedInvocations.add(new MockInvocation(Stub(IMockObject),
+      Stub(IMockMethod),
+      [error],
+      Stub(IResponseGenerator)))
+    final failure = new Failure(new Description(TooManyInvocationsErrorListenerTest, 'test'), error)
+
+    when:
+    failure.getMessage()
+
+    then:
+    thrown(StackOverflowError)
+
+    when:
+    final listener = new SpockRunner.TooManyInvocationsErrorListener()
+    listener.testFailure(failure)
+    failure.getMessage()
+
+    then:
+    noExceptionThrown()
+  }
+}

internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java

@@ -108,6 +108,7 @@ public class InstrumenterConfig {
   private final boolean ciVisibilityEnabled;
   private final ProductActivation appSecActivation;
   private final ProductActivation iastActivation;
+  private final boolean iastFullyDisabled;
   private final boolean usmEnabled;
   private final boolean telemetryEnabled;
 
@@ -194,13 +195,16 @@ private InstrumenterConfig() {
       iastActivation =
           ProductActivation.fromString(
               configProvider.getStringNotEmpty(IAST_ENABLED, DEFAULT_IAST_ENABLED));
+      final Boolean iastEnabled = configProvider.getBoolean(IAST_ENABLED);
+      iastFullyDisabled = iastEnabled != null && !iastEnabled;
       usmEnabled = configProvider.getBoolean(USM_ENABLED, DEFAULT_USM_ENABLED);
       telemetryEnabled = configProvider.getBoolean(TELEMETRY_ENABLED, DEFAULT_TELEMETRY_ENABLED);
     } else {
       // disable these features in native-image
       ciVisibilityEnabled = false;
       appSecActivation = ProductActivation.FULLY_DISABLED;
       iastActivation = ProductActivation.FULLY_DISABLED;
+      iastFullyDisabled = true;
       telemetryEnabled = false;
       usmEnabled = false;
     }
@@ -321,6 +325,10 @@ public ProductActivation getIastActivation() {
     return iastActivation;
   }
 
+  public boolean isIastFullyDisabled() {
+    return iastFullyDisabled;
+  }
+
   public boolean isUsmEnabled() {
     return usmEnabled;
   }