Stop repeated NoSuchFieldError in Jackson 2.16 interner lookup

JsonParser216Helper reads package-private Jackson fields. On classpaths where
they are missing (e.g. a mixed or repackaged Jackson) every getCurrentName()
call threw and was reported. Rethrow the first NoSuchFieldError per class
loader so it is still reported once, then assume interned field names.

Assuming "interned" skips tainting the field name, trading a possible false
negative for avoiding false positives from tainting shared interned strings.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Author: dougqh

dd-java-agent/instrumentation/jackson-core/jackson-core-2.16/src/main/java/com/fasterxml/jackson/core/json/JsonParser216Helper.java

@@ -2,10 +2,44 @@
 
 import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer216Helper;
 
+/**
+ * Reads whether a {@link UTF8StreamJsonParser} interns its field names.
+ *
+ * <p>This reads package-private Jackson fields ({@code _symbols}, {@code _interner}). Stock
+ * jackson-core 2.16+ always has them, but a classpath that mixes Jackson builds can lack them,
+ * which surfaces as a {@link NoSuchFieldError}.
+ *
+ * <p><b>IAST design note:</b> when the fields are missing we cannot tell whether names are
+ * interned, and we answer {@code true} ("interned"). The caller then records the current field name
+ * but does <em>not</em> taint the name string. This is a deliberate trade-off:
+ *
+ * <ul>
+ *   <li>Jackson interns field names by default, so "interned" is the likely answer.
+ *   <li>Tainting an interned {@code String} taints the one shared instance, so every occurrence of
+ *       that name (across requests) would look tainted. That is a false-positive risk.
+ *   <li>The cost is a possible false negative: on such classpaths, an attacker-controlled JSON
+ *       <em>key</em> reaching a sink is not detected. Values are still attributed to their field
+ *       name.
+ * </ul>
+ *
+ * <p>The first failure per class loader is rethrown so the instrumentation exception handler still
+ * reports it once. After that the failure is remembered and calls return {@code true} without
+ * throwing, so a broken classpath does not cost an exception per parsed field name.
+ */
 public final class JsonParser216Helper {
+  private static volatile boolean fieldsUnavailable;
+
   private JsonParser216Helper() {}
 
   public static boolean fetchInterner(UTF8StreamJsonParser jsonParser) {
-    return ByteQuadsCanonicalizer216Helper.fetchInterner(jsonParser._symbols);
+    if (fieldsUnavailable) {
+      return true;
+    }
+    try {
+      return ByteQuadsCanonicalizer216Helper.fetchInterner(jsonParser._symbols);
+    } catch (NoSuchFieldError e) {
+      fieldsUnavailable = true;
+      throw e;
+    }
   }
 }

dd-java-agent/instrumentation/jackson-core/jackson-core-2.16/src/test/java/datadog/trace/instrumentation/jackson_2_16/core/JsonParser216HelperTest.java

@@ -0,0 +1,131 @@
+package datadog.trace.instrumentation.jackson_2_16.core;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.json.JsonParser216Helper;
+import com.fasterxml.jackson.core.json.UTF8StreamJsonParser;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import net.bytebuddy.jar.asm.ClassReader;
+import net.bytebuddy.jar.asm.ClassWriter;
+import net.bytebuddy.jar.asm.commons.ClassRemapper;
+import net.bytebuddy.jar.asm.commons.SimpleRemapper;
+import org.junit.jupiter.api.Test;
+
+class JsonParser216HelperTest {
+
+  private static final String JACKSON_CORE_PREFIX = "com.fasterxml.jackson.core.";
+  private static final String CANONICALIZER =
+      "com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer";
+
+  @Test
+  void reportsInternedFieldNames() throws Exception {
+    JsonFactory factory = new JsonFactory().enable(JsonFactory.Feature.INTERN_FIELD_NAMES);
+    UTF8StreamJsonParser parser = (UTF8StreamJsonParser) factory.createParser(json());
+
+    assertTrue(JsonParser216Helper.fetchInterner(parser));
+  }
+
+  @Test
+  void reportsNonInternedFieldNames() throws Exception {
+    JsonFactory factory = new JsonFactory().disable(JsonFactory.Feature.INTERN_FIELD_NAMES);
+    UTF8StreamJsonParser parser = (UTF8StreamJsonParser) factory.createParser(json());
+
+    assertFalse(JsonParser216Helper.fetchInterner(parser));
+  }
+
+  /**
+   * Simulates a classpath whose {@code ByteQuadsCanonicalizer} has no {@code _interner} field. The
+   * first failure is rethrown so it is reported once; later calls assume interned names.
+   */
+  @Test
+  void rethrowsFirstMissingFieldThenAssumesInterned() throws Exception {
+    ClassLoader loader = new MissingInternerClassLoader();
+    Object factory = loader.loadClass(JsonFactory.class.getName()).getConstructor().newInstance();
+    Object parser =
+        factory.getClass().getMethod("createParser", byte[].class).invoke(factory, (Object) json());
+    Method fetchInterner =
+        loader
+            .loadClass(JsonParser216Helper.class.getName())
+            .getMethod("fetchInterner", loader.loadClass(UTF8StreamJsonParser.class.getName()));
+
+    InvocationTargetException first =
+        assertThrows(InvocationTargetException.class, () -> fetchInterner.invoke(null, parser));
+    assertInstanceOf(NoSuchFieldError.class, first.getCause());
+
+    assertTrue((boolean) fetchInterner.invoke(null, parser));
+    assertTrue((boolean) fetchInterner.invoke(null, parser));
+  }
+
+  private static byte[] json() {
+    return "{\"name\":\"value\"}".getBytes(UTF_8);
+  }
+
+  /**
+   * Loads jackson-core child-first, renaming {@code ByteQuadsCanonicalizer._interner} in the
+   * bytecode. The class stays self-consistent, but a lookup of the original field name fails with
+   * {@link NoSuchFieldError}, like a mixed or repackaged Jackson on the classpath.
+   */
+  private static final class MissingInternerClassLoader extends ClassLoader {
+    MissingInternerClassLoader() {
+      super(JsonParser216HelperTest.class.getClassLoader());
+    }
+
+    @Override
+    protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+      if (!name.startsWith(JACKSON_CORE_PREFIX)) {
+        return super.loadClass(name, resolve);
+      }
+      synchronized (getClassLoadingLock(name)) {
+        Class<?> clazz = findLoadedClass(name);
+        if (clazz == null) {
+          clazz = define(name);
+        }
+        if (resolve) {
+          resolveClass(clazz);
+        }
+        return clazz;
+      }
+    }
+
+    private Class<?> define(String name) throws ClassNotFoundException {
+      try (InputStream in = getParent().getResourceAsStream(name.replace('.', '/') + ".class")) {
+        if (in == null) {
+          throw new ClassNotFoundException(name);
+        }
+        byte[] bytes = readAll(in);
+        if (name.equals(CANONICALIZER)) {
+          bytes = renameInterner(bytes);
+        }
+        return defineClass(name, bytes, 0, bytes.length);
+      } catch (IOException e) {
+        throw new ClassNotFoundException(name, e);
+      }
+    }
+
+    private static byte[] renameInterner(byte[] bytes) {
+      ClassWriter writer = new ClassWriter(0);
+      SimpleRemapper remapper =
+          new SimpleRemapper(CANONICALIZER.replace('.', '/') + "._interner", "_interner_renamed");
+      new ClassReader(bytes).accept(new ClassRemapper(writer, remapper), 0);
+      return writer.toByteArray();
+    }
+
+    private static byte[] readAll(InputStream in) throws IOException {
+      ByteArrayOutputStream out = new ByteArrayOutputStream();
+      byte[] buffer = new byte[8192];
+      for (int read = in.read(buffer); read != -1; read = in.read(buffer)) {
+        out.write(buffer, 0, read);
+      }
+      return out.toByteArray();
+    }
+  }
+}