diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapEmptyBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapEmptyBenchmark.java index acc7fe3357a..c4ff8bc14a6 100644 --- a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapEmptyBenchmark.java +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapEmptyBenchmark.java @@ -15,6 +15,7 @@ import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.BenchmarkParams; import org.openjdk.jmh.infra.Blackhole; @Warmup(iterations = 2, time = 1000, timeUnit = MILLISECONDS) @@ -30,8 +31,17 @@ public class TaintedMapEmptyBenchmark { private final Object anyObject = new Object(); @Setup(Level.Iteration) - public void setup() { - map = new TaintedMap(); + public void setup(BenchmarkParams params) { + final boolean baseline = params.getBenchmark().endsWith("baseline"); + map = baseline ? TaintedMap.NoOp.INSTANCE : new TaintedMap.TaintedMapImpl(); + } + + @Benchmark + @OperationsPerInvocation(OP_COUNT) + public void baseline(final Blackhole bh) { + for (int i = 0; i < OP_COUNT; i++) { + bh.consume(map.get(anyObject)); + } } @Benchmark diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapGetsBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapGetsBenchmark.java index 4b296075cde..96416278c83 100644 --- a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapGetsBenchmark.java +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapGetsBenchmark.java @@ -18,6 +18,7 @@ import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.BenchmarkParams; import org.openjdk.jmh.infra.Blackhole; @Warmup(iterations = 1, time = 1000, timeUnit = MILLISECONDS) @@ -28,7 +29,7 @@ @State(Scope.Benchmark) public class TaintedMapGetsBenchmark { - private static final int INITIAL_OP_COUNT = TaintedMap.DEFAULT_FLAT_MODE_THRESHOLD; + private static final int INITIAL_OP_COUNT = 1 << 12; private static final int OP_COUNT = 1024; private TaintedMap map; @@ -36,27 +37,28 @@ public class TaintedMapGetsBenchmark { private List initialObjectList; @Setup(Level.Iteration) - public void setup() { - map = new TaintedMap(); + public void setup(BenchmarkParams params) { + final boolean baseline = params.getBenchmark().endsWith("baseline"); + map = baseline ? TaintedMap.NoOp.INSTANCE : new TaintedMap.TaintedMapImpl(); initialObjectList = new ArrayList<>(INITIAL_OP_COUNT); objectList = new ArrayList<>(OP_COUNT); for (int i = 0; i < INITIAL_OP_COUNT; i++) { final Object k = new Object(); initialObjectList.add(k); - map.put(new TaintedObject(k, new Range[0], map.getReferenceQueue())); + map.put(new TaintedObject(k, new Range[0])); } for (int i = 0; i < OP_COUNT; i++) { final Object k = new Object(); objectList.add(k); - map.put(new TaintedObject(k, new Range[0], map.getReferenceQueue())); + map.put(new TaintedObject(k, new Range[0])); } } @Benchmark @OperationsPerInvocation(OP_COUNT) - public void getsBaseline(final Blackhole bh) { + public void baseline(final Blackhole bh) { for (int i = 0; i < OP_COUNT; i++) { - bh.consume(objectList.get(i)); + bh.consume(map.get(objectList.get(i))); } } diff --git a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapPutsBenchmark.java b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapPutsBenchmark.java index efe35f583c6..90b92140874 100644 --- a/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapPutsBenchmark.java +++ b/dd-java-agent/agent-iast/src/jmh/java/com/datadog/iast/taint/TaintedMapPutsBenchmark.java @@ -6,7 +6,9 @@ import com.datadog.iast.model.Range; import datadog.trace.test.util.CircularBuffer; import java.util.ArrayList; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -20,10 +22,10 @@ import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.annotations.Timeout; import org.openjdk.jmh.annotations.Warmup; -import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.infra.BenchmarkParams; -@Warmup(iterations = 1, time = 1000, timeUnit = MILLISECONDS) -@Measurement(iterations = 3, time = 1000, timeUnit = MILLISECONDS) +@Warmup(iterations = 3, time = 1000, timeUnit = MILLISECONDS) +@Measurement(iterations = 5, time = 1000, timeUnit = MILLISECONDS) @Timeout(time = 10000, timeUnit = MILLISECONDS) @Fork(3) @OutputTimeUnit(NANOSECONDS) @@ -31,34 +33,36 @@ @State(Scope.Benchmark) public class TaintedMapPutsBenchmark { - private static final int INITIAL_OP_COUNT = TaintedMap.DEFAULT_FLAT_MODE_THRESHOLD; + private static final int INITIAL_OP_COUNT = 1 << 12; private static final int OP_COUNT = 1024; private static final Range[] EMPTY_RANGES = new Range[0]; private TaintedMap map; private List initialObjectList; - private CircularBuffer objectBuffer; + private GarbageCollectorHandler gcHandler; @Setup(Level.Iteration) - public void setup() { - map = new TaintedMap(); - objectBuffer = new CircularBuffer<>(OP_COUNT); + public void setup(BenchmarkParams params) { + final boolean baseline = params.getBenchmark().endsWith("baseline"); + map = baseline ? TaintedMap.NoOp.INSTANCE : new TaintedMap.TaintedMapImpl(); + gcHandler = new GarbageCollectorHandler(OP_COUNT); initialObjectList = new ArrayList<>(INITIAL_OP_COUNT); for (int i = 0; i < INITIAL_OP_COUNT; i++) { final Object k = new Object(); initialObjectList.add(k); - map.put(new TaintedObject(k, EMPTY_RANGES, map.getReferenceQueue())); + map.put(new TaintedObject(k, EMPTY_RANGES)); } } @Benchmark @OperationsPerInvocation(OP_COUNT) - public void putsBaseline(final Blackhole bh) { + public void baseline() { for (int i = 0; i < OP_COUNT; i++) { final Object k = new Object(); - objectBuffer.add(k); - bh.consume(new TaintedObject(k, EMPTY_RANGES, map.getReferenceQueue())); + final TaintedObject to = new TaintedObject(k, EMPTY_RANGES); + gcHandler.add(to); + map.put(to); } } @@ -67,8 +71,37 @@ public void putsBaseline(final Blackhole bh) { public void puts() { for (int i = 0; i < OP_COUNT; i++) { final Object k = new Object(); - objectBuffer.add(k); - map.put(new TaintedObject(k, EMPTY_RANGES, map.getReferenceQueue())); + final TaintedObject to = new TaintedObject(k, EMPTY_RANGES); + gcHandler.add(to); + map.put(to); + } + } + + /** + * Reference queue that holds a circular buffer of alive objects and enqueues to be purged when + * they are removed + */ + private static class GarbageCollectorHandler { + + private final Map map; + private final CircularBuffer alive; + + public GarbageCollectorHandler(final int aliveCount) { + map = new IdentityHashMap<>(aliveCount); + alive = new CircularBuffer<>(aliveCount); + } + + public void add(TaintedObject reference) { + if (reference == null || reference.get() == null) { + return; + } + final Object referent = reference.get(); + final Object toRemove = alive.add(referent); + if (toRemove != null) { + final TaintedObject taintedObject = map.remove(toRemove); + taintedObject.enqueue(); + } + map.put(reference.get(), reference); } } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java index 79f91c18a6a..98c32390c3e 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java @@ -1,14 +1,15 @@ package com.datadog.iast.taint; -import com.datadog.iast.util.NonBlockingSemaphore; -import java.lang.ref.Reference; -import java.lang.ref.ReferenceQueue; +import com.datadog.iast.IastSystem; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.NoSuchElementException; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Map optimized for taint tracking. @@ -16,7 +17,7 @@ *

This implementation is optimized for low concurrency scenarios, but never fails with high * concurrency. * - *

>This is intentionally a smaller API compared to {@link java.util.Map}. It is a hardcoded + *

This is intentionally a smaller API compared to {@link java.util.Map}. It is a hardcoded * interface for {@link TaintedObject} entries, which can be used themselves directly as hash table * entries and weak references. * @@ -29,294 +30,339 @@ *

  • Put operations MAY be lost under concurrent modification. * * - *

    Capacity is fixed, so there is no rehashing. Once it reaches a flat mode - * threshold, the table switches to flat mode. In this mode, every new put will be inserted at - * the head of the bucket, and any tail (colliding entries) will be discarded. Once a map switches - * to flat mode, it never goes back from it. Note that entries for garbage collected entries are - * removed before this threshold is checked. + *

    Capacity is fixed, so there is no rehashing. * *

    This implementation works reasonably well under high concurrency, but it will lose some writes * in that case. */ -public final class TaintedMap implements Iterable { +public interface TaintedMap extends Iterable { /** Default capacity. It MUST be a power of 2. */ - public static final int DEFAULT_CAPACITY = 1 << 14; - /** Default flat mode threshold. */ - public static final int DEFAULT_FLAT_MODE_THRESHOLD = 1 << 13; - /** Periodicity of table purges, as number of put operations. It MUST be a power of two. */ - static final int PURGE_COUNT = 1 << 6; - /** Bitmask for fast modulo with PURGE_COUNT. */ - static final int PURGE_MASK = PURGE_COUNT - 1; + int DEFAULT_CAPACITY = 1 << 14; + /** Bitmask to convert hashes to positive integers. */ - static final int POSITIVE_MASK = Integer.MAX_VALUE; - - private final TaintedObject[] table; - /** Bitmask for fast modulo with table length. */ - private final int lengthMask; - /** Flag to ensure we do not run multiple purges concurrently. */ - private final NonBlockingSemaphore purge = NonBlockingSemaphore.withPermitCount(1); - /** - * Estimated number of hash table entries. If the hash table switches to flat mode, it stops - * counting elements. - */ - private final AtomicInteger estimatedSize = new AtomicInteger(0); - /** Reference queue for garbage-collected entries. */ - private ReferenceQueue referenceQueue; - /** - * Whether flat mode is enabled or not. Once this is true, it is not set to false again unless - * {@link #clear()} is called. - */ - private boolean isFlat = false; - /** Number of elements in the hash table before switching to flat mode. */ - private final int flatModeThreshold; - - /** - * Default constructor. Uses {@link #DEFAULT_CAPACITY} and {@link #DEFAULT_FLAT_MODE_THRESHOLD}. - */ - public TaintedMap() { - this(DEFAULT_CAPACITY, DEFAULT_FLAT_MODE_THRESHOLD, new ReferenceQueue<>()); - } + int POSITIVE_MASK = Integer.MAX_VALUE; - /** - * Create a new hash map with the given capacity and flat mode threshold. - * - * @param capacity Capacity of the internal array. It must be a power of 2. - * @param flatModeThreshold Limit of entries before switching to flat mode. - * @param queue Reference queue. Only for tests. - */ - @SuppressWarnings("unchecked") - TaintedMap(final int capacity, final int flatModeThreshold, final ReferenceQueue queue) { - table = new TaintedObject[capacity]; - lengthMask = table.length - 1; - this.flatModeThreshold = flatModeThreshold; - this.referenceQueue = queue; + static TaintedMap build() { + final TaintedMapImpl map = new TaintedMapImpl(); + return IastSystem.DEBUG ? new Debug(map) : map; } - /** - * Returns the {@link TaintedObject} for the given input object. - * - * @param key Key object. - * @return The {@link TaintedObject} if it exists, {@code null} otherwise. - */ @Nullable - public TaintedObject get(final @Nonnull Object key) { - final int index = indexObject(key); - TaintedObject entry = table[index]; - while (entry != null) { - if (key == entry.get()) { - return entry; - } - entry = entry.next; + TaintedObject get(@Nonnull Object key); + + void put(final @Nonnull TaintedObject entry); + + int count(); + + void clear(); + + class TaintedMapImpl implements TaintedMap { + + protected final TaintedObject[] table; + + /** Bitmask for fast modulo with table length. */ + protected final int lengthMask; + + /** Default constructor. Uses {@link #DEFAULT_CAPACITY}. */ + TaintedMapImpl() { + this(DEFAULT_CAPACITY); } - return null; - } - /** - * Put a new {@link TaintedObject} in the hash table. This method will lose puts in concurrent - * scenarios. - * - * @param entry Tainted object. - */ - public void put(final @Nonnull TaintedObject entry) { - if (isFlat) { - flatPut(entry); - } else { - tailPut(entry); + /** + * Create a new hash map with the given capacity a purge queue. + * + * @param capacity Capacity of the internal array. It must be a power of 2. + */ + TaintedMapImpl(final int capacity) { + table = new TaintedObject[capacity]; + lengthMask = table.length - 1; } - } - /** - * Put operation when we are in flat mode: - Always override elements ignoring chaining. - Stop - * updating the estimated size. - */ - private void flatPut(final @Nonnull TaintedObject entry) { - final int index = index(entry.positiveHashCode); - table[index] = entry; - if ((entry.positiveHashCode & PURGE_MASK) == 0) { - purge(); + /** + * Returns the {@link TaintedObject} for the given input object. + * + * @param key Key object. + * @return The {@link TaintedObject} if it exists, {@code null} otherwise. + */ + @Nullable + @Override + public TaintedObject get(final @Nonnull Object key) { + final int index = indexObject(key); + TaintedObject entry = head(index); + while (entry != null) { + if (key == entry.get()) { + return entry; + } + entry = next(entry); + } + return null; } - } - /** - * Put an object, always to the tail of the chain. It will not insert the element if it is already - * present in the map. - */ - private void tailPut(final @Nonnull TaintedObject entry) { - final int index = index(entry.positiveHashCode); - TaintedObject cur = table[index]; - if (cur == null) { - table[index] = entry; - } else { - while (cur.next != null) { - if (cur.positiveHashCode == entry.positiveHashCode && cur.get() == entry.get()) { - // Duplicate, exit early. - return; + /** + * Put a new {@link TaintedObject} in the hash table, always to the tail of the chain. It will + * not insert the element if it is already present in the map. This method will lose puts in + * concurrent scenarios. + * + * @param entry Tainted object. + */ + @Override + public void put(final @Nonnull TaintedObject entry) { + final int index = index(entry.positiveHashCode); + TaintedObject cur = head(index); + if (cur == null) { + table[index] = entry; + } else { + TaintedObject next; + while ((next = next(cur)) != null) { + if (cur.positiveHashCode == entry.positiveHashCode && cur.get() == entry.get()) { + // Duplicate, exit early. + return; + } + cur = next; } - cur = cur.next; + cur.next = entry; } - cur.next = entry; } - if ((entry.positiveHashCode & PURGE_MASK) == 0) { - // To mitigate the cost of maintaining an atomic counter, we only update the size every - // puts. This is just an approximation, we rely on key's identity hash code as - // if it was a random number generator. - estimatedSize.addAndGet(PURGE_COUNT); - purge(); + + @Override + public void clear() { + Arrays.fill(table, null); } - } - /** - * Purge entries that have been garbage collected. Only one concurrent call to this method is - * allowed, further concurrent calls will be ignored. - */ - void purge() { - // Ensure we enter only once concurrently. - if (!purge.acquire()) { - return; + @Nonnull + @Override + public Iterator iterator() { + return iterator(0, table.length); } - try { - // Remove GC'd entries. - Reference ref; - int removedCount = 0; - while ((ref = referenceQueue.poll()) != null) { - // This would be an extremely rare bug, and maybe it should produce a health metric or - // warning. - if (!(ref instanceof TaintedObject)) { - continue; + @Override + public int count() { + int size = 0; + for (int i = 0; i < table.length; i++) { + TaintedObject entry = table[i]; + while (entry != null) { + entry = entry.next; + size++; } - final TaintedObject entry = (TaintedObject) ref; - removedCount += remove(entry); } + return size; + } - if (estimatedSize.addAndGet(-removedCount) > flatModeThreshold) { - isFlat = true; - } - } finally { - // Reset purging flag. - purge.release(); + private Iterator iterator(final int start, final int stop) { + return new Iterator() { + int currentIndex = start; + @Nullable TaintedObject currentSubPos; + + @Override + public boolean hasNext() { + if (currentSubPos != null) { + return true; + } + for (; currentIndex < stop; currentIndex++) { + if (table[currentIndex] != null) { + return true; + } + } + return false; + } + + @Override + public TaintedObject next() { + if (currentSubPos != null) { + TaintedObject toReturn = currentSubPos; + currentSubPos = toReturn.next; + return toReturn; + } + for (; currentIndex < stop; currentIndex++) { + final TaintedObject entry = table[currentIndex]; + if (entry != null) { + currentSubPos = entry.next; + currentIndex++; + return entry; + } + } + throw new NoSuchElementException(); + } + }; } - } - /** - * Removes a {@link TaintedObject} from the hash table. This method will lose puts in concurrent - * scenarios. - * - * @param entry Tainted object. - * @return Number of removed elements. - */ - private int remove(final TaintedObject entry) { - // A remove might be lost when it is concurrent to puts. If that happens, the object will not be - // removed again, (until the map goes to flat mode). When a remove is lost under concurrency, - // this method will still return 1, and it will still be subtracted from the map size estimate. - // If this is infrequent enough, this would lead to a performance degradation of get opertions. - // If this happens extremely frequently, like number of lost removals close to number of puts, - // it could prevent the map from ever going into flat mode, and its size might become - // effectively unbound. - final int index = index(entry.positiveHashCode); - TaintedObject cur = table[index]; - if (cur == entry) { - table[index] = cur.next; - return 1; + protected int indexObject(final Object obj) { + return index(positiveHashCode(System.identityHashCode(obj))); } - if (cur == null) { - return 0; + + protected int positiveHashCode(final int h) { + return h & POSITIVE_MASK; + } + + protected int index(int h) { + return h & lengthMask; + } + + @Nullable + protected TaintedObject head(final int index) { + final TaintedObject head = findAlive(table[index]); + table[index] = head; + return head; + } + + @Nullable + protected TaintedObject next(@Nonnull final TaintedObject item) { + final TaintedObject next = findAlive(item.next); + item.next = next; + return next; } - for (TaintedObject prev = cur.next; cur != null && prev != null; prev = cur, cur = cur.next) { - if (cur == entry) { - prev.next = cur.next; - return 1; + + /** Gets the first reachable reference that has not been GC'ed */ + @Nullable + protected TaintedObject findAlive(@Nullable TaintedObject item) { + while (item != null && item.get() == null) { + item = item.next; } + return item; } - // If we reach this point, the entry was already removed or put was lost. - return 0; } - public void clear() { - isFlat = false; - estimatedSize.set(0); - Arrays.fill(table, null); - referenceQueue = new ReferenceQueue<>(); - } + class Debug implements TaintedMap { - public ReferenceQueue getReferenceQueue() { - return referenceQueue; - } + static final Logger LOGGER = LoggerFactory.getLogger(TaintedMap.class); - private int indexObject(final Object obj) { - return index(positiveHashCode(System.identityHashCode(obj))); - } + /** Interval to compute statistics in debug mode * */ + static final int COMPUTE_STATISTICS_INTERVAL = 1 << 17; - private int positiveHashCode(final int h) { - return h & POSITIVE_MASK; - } + private final TaintedMapImpl delegate; - private int index(int h) { - return h & lengthMask; - } + private final AtomicLong puts = new AtomicLong(0); - private Iterator iterator(final int start, final int stop) { - return new Iterator() { - int currentIndex = start; - @Nullable TaintedObject currentSubPos; + public Debug(final TaintedMapImpl delegate) { + this.delegate = delegate; + } - @Override - public boolean hasNext() { - if (currentSubPos != null) { - return true; - } - for (; currentIndex < stop; currentIndex++) { - if (table[currentIndex] != null) { - return true; - } - } - return false; + @Override + public void put(@Nonnull final TaintedObject entry) { + delegate.put(entry); + final long putOps = puts.updateAndGet(current -> current == Long.MAX_VALUE ? 0 : current + 1); + if (putOps % COMPUTE_STATISTICS_INTERVAL == 0 && LOGGER.isDebugEnabled()) { + computeStatistics(); } + } - @Override - public TaintedObject next() { - if (currentSubPos != null) { - TaintedObject toReturn = currentSubPos; - currentSubPos = toReturn.next; - return toReturn; - } - for (; currentIndex < stop; currentIndex++) { - final TaintedObject entry = table[currentIndex]; - if (entry != null) { - currentSubPos = entry.next; - currentIndex++; - return entry; + @Nullable + @Override + public TaintedObject get(@Nonnull final Object key) { + return delegate.get(key); + } + + @Override + public int count() { + return delegate.count(); + } + + @Override + public void clear() { + delegate.clear(); + } + + @Nonnull + @Override + public Iterator iterator() { + return delegate.iterator(); + } + + protected void computeStatistics() { + final TaintedObject[] table = delegate.table; + final int[] chains = new int[table.length]; + long stale = 0; + long count = 0; + for (int bucket = 0; bucket < table.length; bucket++) { + TaintedObject cur = table[bucket]; + int chainLength = 0; + while (cur != null) { + count++; + chainLength++; + if (cur.get() == null) { + stale++; } + cur = cur.next; } - throw new NoSuchElementException(); + chains[bucket] = chainLength; } - }; - } + Arrays.sort(chains); + LOGGER.debug( + "Map [size:" + + delegate.table.length + + ", count:" + + count + + ", stale:" + + percentage(stale, count) + + "], Chains [" + + String.join( + ", ", + average(chains), + percentile(chains, 50), + percentile(chains, 75), + percentile(chains, 90), + percentile(chains, 99), + percentile(chains, 100)) + + "]"); + } - @Nonnull - @Override - public Iterator iterator() { - return iterator(0, table.length); - } + private static String percentage(final long actual, final long total) { + return String.format("%2.2f%%", total == 0 ? 0 : (actual * 100D / total)); + } - public int count() { - int size = 0; - for (int i = 0; i < table.length; i++) { - TaintedObject entry = table[i]; - while (entry != null) { - entry = entry.next; - size++; + /** Computes different percentiles, values array MUST be sorted beforehand */ + private static String percentile(final int[] values, final int percentile) { + assert percentile >= 0 && percentile <= 100; + final String prefix; + final int value; + switch (percentile) { + case 0: + prefix = "min"; + value = values[0]; + break; + case 100: + prefix = "max"; + value = values[values.length - 1]; + break; + default: + prefix = "pct" + percentile; + value = values[Math.round(values.length * percentile / 100F)]; + break; } + return String.format("%s:%s", prefix, value); } - return size; - } - public int getEstimatedSize() { - return estimatedSize.get(); + private static String average(final int[] values) { + return String.format("avg:%2.2f", Arrays.stream(values).sum() / (double) values.length); + } } - public boolean isFlat() { - return isFlat; + class NoOp implements TaintedMap { + + public static final TaintedMap INSTANCE = new NoOp(); + + @Nullable + @Override + public TaintedObject get(@Nonnull Object key) { + return null; + } + + @Override + public void put(@Nonnull TaintedObject entry) {} + + @Override + public int count() { + return 0; + } + + @Override + public void clear() {} + + @Nonnull + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java index 84d3b9c6091..6af0af837ef 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObject.java @@ -4,7 +4,6 @@ import com.datadog.iast.model.Range; import datadog.trace.api.Config; -import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -17,11 +16,8 @@ public class TaintedObject extends WeakReference { @Nullable TaintedObject next; private Range[] ranges; - public TaintedObject( - final @Nonnull Object obj, - final @Nonnull Range[] ranges, - final @Nullable ReferenceQueue queue) { - super(obj, queue); + public TaintedObject(final @Nonnull Object obj, final @Nonnull Range[] ranges) { + super(obj); this.positiveHashCode = System.identityHashCode(obj) & POSITIVE_MASK; // ensure ranges never go over the limit if (ranges.length > MAX_RANGE_COUNT) { diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObjects.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObjects.java index ab3b20e0401..9595e2d0df6 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObjects.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedObjects.java @@ -30,10 +30,6 @@ public interface TaintedObjects extends Iterable { int count(); - int getEstimatedSize(); - - boolean isFlat(); - static TaintedObjects acquire() { TaintedObjectsImpl taintedObjects = TaintedObjectsImpl.pool.poll(); if (taintedObjects == null) { @@ -52,7 +48,7 @@ class TaintedObjectsImpl implements TaintedObjects { private final TaintedMap map; public TaintedObjectsImpl() { - this(new TaintedMap()); + this(TaintedMap.build()); } private TaintedObjectsImpl(final @Nonnull TaintedMap map) { @@ -61,7 +57,7 @@ private TaintedObjectsImpl(final @Nonnull TaintedMap map) { @Override public TaintedObject taint(final @Nonnull Object obj, final @Nonnull Range[] ranges) { - final TaintedObject tainted = new TaintedObject(obj, ranges, map.getReferenceQueue()); + final TaintedObject tainted = new TaintedObject(obj, ranges); map.put(tainted); return tainted; } @@ -83,16 +79,6 @@ public int count() { return map.count(); } - @Override - public int getEstimatedSize() { - return map.getEstimatedSize(); - } - - @Override - public boolean isFlat() { - return map.isFlat(); - } - @Nonnull @Override public Iterator iterator() { @@ -147,16 +133,6 @@ public int count() { return delegated.count(); } - @Override - public int getEstimatedSize() { - return delegated.getEstimatedSize(); - } - - @Override - public boolean isFlat() { - return delegated.isFlat(); - } - @Nonnull @Override public Iterator iterator() { @@ -193,21 +169,11 @@ public TaintedObject get(@Nonnull final Object obj) { @Override public void release() {} - @Override - public boolean isFlat() { - return false; - } - @Override public int count() { return 0; } - @Override - public int getEstimatedSize() { - return 0; - } - @Override @Nonnull public Iterator iterator() { diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetry.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetry.java index 1ac059b0b54..a9c80e38e81 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetry.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetry.java @@ -2,7 +2,6 @@ import static datadog.trace.api.iast.telemetry.IastMetric.EXECUTED_TAINTED; import static datadog.trace.api.iast.telemetry.IastMetric.REQUEST_TAINTED; -import static datadog.trace.api.iast.telemetry.IastMetric.TAINTED_FLAT_MODE; import com.datadog.iast.IastRequestContext; import com.datadog.iast.model.Range; @@ -16,11 +15,6 @@ public class TaintedObjectsWithTelemetry implements TaintedObjects { - /** - * If the estimated size of the tainted objects is lower than this threshold we will count instead - */ - private static final int COUNT_THRESHOLD = 1024; - public static TaintedObjects build( final Verbosity verbosity, final TaintedObjects taintedObjects) { if (verbosity.isInformationEnabled()) { @@ -65,10 +59,7 @@ public TaintedObject get(@Nonnull Object obj) { @Override public void release() { try { - if (delegate.isFlat()) { - IastMetricCollector.add(TAINTED_FLAT_MODE, 1, ctx); - } - IastMetricCollector.add(REQUEST_TAINTED, computeSize(), ctx); + IastMetricCollector.add(REQUEST_TAINTED, count(), ctx); } finally { delegate.release(); } @@ -84,19 +75,4 @@ public Iterator iterator() { public int count() { return delegate.count(); } - - @Override - public int getEstimatedSize() { - return delegate.getEstimatedSize(); - } - - @Override - public boolean isFlat() { - return delegate.isFlat(); - } - - private int computeSize() { - int size = getEstimatedSize(); - return size > COUNT_THRESHOLD ? size : count(); - } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/TaintedObjectEncodingTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/TaintedObjectEncodingTest.groovy index dfbc0fb8275..f4d69c168cf 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/TaintedObjectEncodingTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/model/json/TaintedObjectEncodingTest.groovy @@ -8,8 +8,6 @@ import datadog.trace.api.iast.SourceTypes import datadog.trace.test.util.DDSpecification import org.skyscreamer.jsonassert.JSONAssert -import java.lang.ref.ReferenceQueue - import static datadog.trace.api.iast.VulnerabilityMarks.NOT_MARKED class TaintedObjectEncodingTest extends DDSpecification { @@ -89,7 +87,6 @@ class TaintedObjectEncodingTest extends DDSpecification { private TaintedObject taintedObject(final String value, final byte sourceType, final String sourceName, final String sourceValue) { return new TaintedObject( value, - [new Range(0, value.length(), new Source(sourceType, sourceName, sourceValue), NOT_MARKED)] as Range[], - Mock(ReferenceQueue)) + [new Range(0, value.length(), new Source(sourceType, sourceName, sourceValue), NOT_MARKED)] as Range[]) } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/PropagationModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/PropagationModuleTest.groovy index e573d5dfb2c..2cfd45aa599 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/PropagationModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/PropagationModuleTest.groovy @@ -471,7 +471,7 @@ class PropagationModuleTest extends IastModuleImplTestBase { private TaintedObject getTaintedObject(final Object target) { if (target instanceof Taintable) { final source = (target as Taintable).$$DD$getSource() as Source - return source == null ? null : new TaintedObject(target, Ranges.forObject(source), null) + return source == null ? null : new TaintedObject(target, Ranges.forObject(source)) } return ctx.getTaintedObjects().get(target) } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/ObjectGen.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/ObjectGen.groovy index deb74ce82f2..a53966661d5 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/ObjectGen.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/ObjectGen.groovy @@ -1,7 +1,6 @@ package com.datadog.iast.taint import static TaintedMap.POSITIVE_MASK -import static TaintedMap.PURGE_MASK /** * Generate objects to test {@link TaintedMap}. @@ -86,7 +85,5 @@ class ObjectGen { return bucket } - static final Closure TRIGGERS_PURGE = { i -> (i & PURGE_MASK) == 0 } - static final Closure DOES_NOT_TRIGGER_PURGE = { i -> (i & PURGE_MASK) != 0 } static final Closure TRUE = { i -> true } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedMapTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedMapTest.groovy index d5bfd5e4ca1..e87a28f401d 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedMapTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedMapTest.groovy @@ -1,22 +1,34 @@ package com.datadog.iast.taint +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger import com.datadog.iast.model.Range import datadog.trace.test.util.CircularBuffer import datadog.trace.test.util.DDSpecification -import java.lang.ref.Reference -import java.lang.ref.ReferenceQueue -import java.lang.ref.WeakReference import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors class TaintedMapTest extends DDSpecification { + + private Logger logger + private Level defaultLevel + + void setup() { + logger = TaintedMap.Debug.LOGGER as Logger + defaultLevel = logger.getLevel() + } + + void cleanup() { + logger.setLevel(defaultLevel) + } + def 'simple workflow'() { given: - def map = new TaintedMap() + final map = new TaintedMap.TaintedMapImpl() final o = new Object() - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) + final to = new TaintedObject(o, [] as Range[]) expect: map.size() == 0 @@ -43,11 +55,10 @@ class TaintedMapTest extends DDSpecification { def 'get non-existent object'() { given: - def map = new TaintedMap() + final map = new TaintedMap.TaintedMapImpl() final o = new Object() expect: - !map.isFlat() map.get(o) == null map.size() == 0 map.count() == 0 @@ -56,141 +67,23 @@ class TaintedMapTest extends DDSpecification { def 'last put always exists'() { given: int capacity = 256 - int flatModeThreshold = (int) (capacity / 2) - def map = new TaintedMap(capacity, flatModeThreshold, new ReferenceQueue<>()) + final map = new TaintedMap.TaintedMapImpl() int nTotalObjects = capacity * 10 expect: (1..nTotalObjects).each { i -> final o = new Object() - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) + final to = new TaintedObject(o, [] as Range[]) map.put(to) assert map.get(o) == to } } - def 'do not fail on extraneous reference'() { - given: - int capacity = 256 - int flatModeThreshold = (int) (capacity / 2) - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - def gen = new ObjectGen(capacity) - - when: 'extraneous reference in enqueued' - queue.free(new WeakReference(new Object())) - - and: 'purge is triggered' - gen.genObjects(1, ObjectGen.TRIGGERS_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - - then: - !map.isFlat() - map.size() == 1 - map.count() == 1 - } - - def 'do not fail on double free'() { - given: - int capacity = 256 - int flatModeThreshold = (int) (capacity / 2) - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - def gen = new ObjectGen(capacity) - - when: 'reference to non-present object in enqueued' - queue.free(new TaintedObject(new Object(), new Range[0] as Range[], queue)) - - and: 'purge is triggered' - gen.genObjects(1, ObjectGen.TRIGGERS_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - - then: - !map.isFlat() - map.size() == 1 - map.count() == 1 - } - - def 'do not fail on double free with previous data'() { - given: - int capacity = 256 - int flatModeThreshold = (int) (capacity / 2) - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - def gen = new ObjectGen(capacity) - def bucket = gen.genBucket(2, ObjectGen.TRIGGERS_PURGE) - - when: - queue.free(new TaintedObject(bucket[0], new Range[0] as Range[], queue)) - final to = new TaintedObject(bucket[1], [] as Range[], map.getReferenceQueue()) - map.put(to) - - then: - !map.isFlat() - map.size() == 1 - map.count() == 1 - } - - def 'flat mode - last put wins'() { - given: - int capacity = 256 - int flatModeThreshold = (int) (capacity / 2) - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - def gen = new ObjectGen(capacity) - - when: - // Number of purges required to switch to flat mode (in the absence of garbage collection) - final int purgesToFlatMode = (int) (flatModeThreshold / TaintedMap.PURGE_COUNT) + 1 - gen.genObjects(purgesToFlatMode, ObjectGen.TRIGGERS_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - - then: - map.isFlat() - - when: - def lastPuts = [] - def nonLastPuts = [] - gen.genBuckets(capacity, 2).each { bucket -> - bucket.each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - lastPuts.add(bucket[-1]) - nonLastPuts.addAll(bucket[0..-2]) - } - - then: - map.size() == capacity - map.count() == capacity - - and: 'last puts are present' - lastPuts.each { o -> - assert map.get(o).get() == o - } - - and: 'non-last puts are not present' - nonLastPuts.each { o -> - assert map.get(o) == null - } - } - def 'garbage-collected entries are purged'() { given: int capacity = 128 int flatModeThreshold = 64 - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) + def map = new TaintedMap.TaintedMapImpl(capacity) int iters = 16 int nObjectsPerIter = flatModeThreshold - 1 @@ -199,72 +92,26 @@ class TaintedMapTest extends DDSpecification { when: (1..iters).each { - // Insert objects that do not trigger purge - gen.genObjects(nObjectsPerIter, ObjectGen.DOES_NOT_TRIGGER_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) + final queue = gen.genObjects(nObjectsPerIter, ObjectGen.TRUE).collect { o -> + final to = new TaintedObject(o, [] as Range[]) map.put(to) + return to } // Clear previous objects + queue.each { + final referent = it.get() + it.clear() + map.get(referent) + } queue.clear() // Trigger purge - final o = gen.genObjects(1, ObjectGen.TRIGGERS_PURGE)[0] - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - objectBuffer.add(o) - map.put(to) - } - - then: - !map.isFlat() - map.size() == iters - map.count() == iters - final entries = map.toList() - entries.findAll { it.get() != null }.size() == iters - - and: 'all objects are as expected' - objectBuffer.each { o -> - final to = map.get(o) - assert to != null - assert to.get() == o - } - } - - def 'garbage-collected entries are purged in flat mode'() { - given: - int capacity = 128 - int flatModeThreshold = 64 - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - - int iters = 1 - def gen = new ObjectGen(capacity) - def objectBuffer = new CircularBuffer(iters) - - when: - // Number of purges required to switch to flat mode (in the absence of garbage collection) - final int purgesToFlatMode = (int) (flatModeThreshold / TaintedMap.PURGE_COUNT) + 1 - gen.genObjects(purgesToFlatMode, ObjectGen.TRIGGERS_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - - then: - map.isFlat() - - when: - (1..iters).each { - // Clear previous objects - queue.clear() - // Trigger purge - final o = gen.genObjects(1, ObjectGen.TRIGGERS_PURGE)[0] - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) + final o = gen.genObjects(1, ObjectGen.TRUE)[0] + final to = new TaintedObject(o, [] as Range[]) objectBuffer.add(o) map.put(to) } then: - map.isFlat() map.size() == iters map.count() == iters final entries = map.toList() @@ -278,12 +125,10 @@ class TaintedMapTest extends DDSpecification { } } - def 'multi-threaded with no collisions, no GC, non-flat mode'() { + def 'multi-threaded with no collisions, no GC'() { given: int capacity = 128 - int flatModeThreshold = 64 - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) + def map = new TaintedMap.TaintedMapImpl(capacity) and: int nThreads = 16 @@ -291,16 +136,16 @@ class TaintedMapTest extends DDSpecification { def gen = new ObjectGen(capacity) def executorService = Executors.newFixedThreadPool(nThreads) def latch = new CountDownLatch(nThreads) - def buckets = gen.genBuckets(nThreads, nObjectsPerThread, ObjectGen.DOES_NOT_TRIGGER_PURGE) + def buckets = gen.genBuckets(nThreads, nObjectsPerThread, ObjectGen.TRUE) when: 'puts from different threads to different buckets' - def futures = (0..nThreads-1).collect { thread -> + def futures = (0..nThreads - 1).collect { thread -> executorService.submit({ -> latch.countDown() latch.await() buckets[thread].each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) + final to = new TaintedObject(o, [] as Range[]) map.put(to) } } as Runnable) @@ -310,7 +155,6 @@ class TaintedMapTest extends DDSpecification { }) then: - !map.isFlat() nThreads == buckets.size() and: 'all objects are as expected' @@ -325,72 +169,10 @@ class TaintedMapTest extends DDSpecification { executorService?.shutdown() } - def 'multi-threaded with no collisions, no GC, flat mode'() { - given: - int capacity = 128 - int flatModeThreshold = 64 - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) - - and: - int nThreads = 16 - def gen = new ObjectGen(capacity) - def executorService = Executors.newFixedThreadPool(nThreads) - def latch = new CountDownLatch(nThreads) - - when: - // Number of purges required to switch to flat mode (in the absence of garbage collection) - final int purgesToFlatMode = (int) (flatModeThreshold / TaintedMap.PURGE_COUNT) + 1 - gen.genObjects(purgesToFlatMode, ObjectGen.TRIGGERS_PURGE).each { o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - map.put(to) - } - - then: - map.isFlat() - - when: 'puts from different threads to any buckets' - def futures = (0..nThreads-1).collect { thread -> - // Each thread has multiple objects for each bucket - def objects = gen.genBuckets(capacity, 10).flatten() - def taintedObjects = objects.collect {o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - return to - } - Collections.shuffle(taintedObjects) - - executorService.submit({ - -> - latch.countDown() - latch.await() - taintedObjects.each { to -> - map.put(to) - } - } as Runnable) - } - futures.collect({ - it.get() - }) - - then: - map.isFlat() - map.size() == capacity - map.count() == capacity - map.toList().findAll({ it.get() != null }).size() == capacity - map.toList().collect({ it.get() }).toSet().size() == capacity - - cleanup: - executorService?.shutdown() - } - def 'clear is thread-safe (does not throw)'() { given: int capacity = 128 - int flatModeThreshold = 64 - def queue = new MockReferenceQueue() - def map = new TaintedMap(capacity, flatModeThreshold, queue) + def map = new TaintedMap.TaintedMapImpl(capacity) and: int nThreads = 16 @@ -399,13 +181,11 @@ class TaintedMapTest extends DDSpecification { def latch = new CountDownLatch(nThreads) when: 'puts from different threads to any buckets' - def futures = (0..nThreads-1).collect { thread -> + def futures = (0..nThreads - 1).collect { thread -> // Each thread has multiple objects for each bucket def objects = gen.genBuckets(capacity, 32).flatten() - def taintedObjects = objects.collect {o -> - final to = new TaintedObject(o, [] as Range[], map.getReferenceQueue()) - queue.hold(o, to) - return to + def taintedObjects = objects.collect { o -> + return new TaintedObject(o, [] as Range[]) } Collections.shuffle(taintedObjects) @@ -429,51 +209,74 @@ class TaintedMapTest extends DDSpecification { then: map.size() == 0 map.count() == 0 - !map.isFlat() cleanup: executorService?.shutdown() } - private static class MockReferenceQueue extends ReferenceQueue { - private List> queue = new ArrayList() - private Map> objects = new HashMap<>() + void 'map purges elements on put/get'() { + given: + final capacity = 1 // single bucket + final map = new TaintedMap.TaintedMapImpl(1) + final gen = new ObjectGen(capacity) + final to = gen.genObjects(5, ObjectGen.TRUE).collect { new TaintedObject(it, [] as Range[]) } - void hold(Object referent, Reference reference) { - objects.put(referent, reference) - } + when: 'purging the head with put' + map.put(to[0]) + to[0].clear() + map.put(to[1]) - void free(Reference ref) { - def referent = ref.get() - ref.clear() - queue.push(ref) - if (referent != null) { - objects.remove(referent) - } - } + then: + map.size() == 1 + map.count() == 1 - void clear() { - objects.values().toList().each { - free(it) - } - } + when: 'purging an element in the middle with put' + map.put(to[2]) + map.put(to[3]) + to[2].clear() + map.put(to[4]) - @Override - Reference poll() { - if (queue.isEmpty()) { - return null - } - return queue.pop() - } + then: + map.size() == 3 + map.count() == 3 - @Override - Reference remove() throws InterruptedException { - throw new UnsupportedOperationException("NOT IMPLEMENTED") - } + when: 'purging the tail with get' + to[4].clear() + map.get('I am not in the map!!!') - @Override - Reference remove(long timeout) throws IllegalArgumentException, InterruptedException { - throw new UnsupportedOperationException("NOT IMPLEMENTED") - } + then: + map.size() == 2 + map.count() == 2 + } + + void 'test no op implementation'() { + setup: + final instance = TaintedMap.NoOp.INSTANCE + final toTaint = 'test' + + when: + final tainted = instance.put(new TaintedObject(toTaint, [] as Range[])) + + then: + tainted == null + instance.get(toTaint) == null + instance.count() == 0 + instance.size() == 0 + !instance.iterator().hasNext() + } + + void 'test debug instance'() { + setup: + final map = new TaintedMap.Debug(new TaintedMap.TaintedMapImpl()) + final capacity = TaintedMap.Debug.COMPUTE_STATISTICS_INTERVAL + final gen = new ObjectGen(capacity) + logger.setLevel(Level.ALL) + + when: + gen.genObjects(capacity, ObjectGen.TRUE).each { map.put(new TaintedObject(it, [] as Range[])) } + + then: + map.size() == capacity + noExceptionThrown() } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectTest.groovy index d809f5b8b69..e523bd979fd 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectTest.groovy @@ -1,14 +1,12 @@ package com.datadog.iast.taint +import com.datadog.iast.model.Range import com.datadog.iast.model.Source import datadog.trace.api.Config import spock.lang.Specification -import com.datadog.iast.model.Range - -import java.lang.ref.ReferenceQueue -import static datadog.trace.api.iast.VulnerabilityMarks.NOT_MARKED import static datadog.trace.api.iast.SourceTypes.REQUEST_HEADER_NAME +import static datadog.trace.api.iast.VulnerabilityMarks.NOT_MARKED class TaintedObjectTest extends Specification { @@ -19,7 +17,7 @@ class TaintedObjectTest extends Specification { .collect { index -> new Range(index, 1, new Source(REQUEST_HEADER_NAME, 'a', 'b'), NOT_MARKED) } when: - final tainted = new TaintedObject('test', ranges.toArray(new Range[0]), new ReferenceQueue()) + final tainted = new TaintedObject('test', ranges.toArray(new Range[0])) then: ranges.size() > max diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsLogTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsLogTest.groovy index ea01efa3989..61670133b34 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsLogTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsLogTest.groovy @@ -76,7 +76,6 @@ class TaintedObjectsLogTest extends DDSpecification { then: taintedObjects.size() == 1 taintedObjects.iterator().size() == 1 - !taintedObjects.flat } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsNoOpTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsNoOpTest.groovy index c8f1f91fe86..b364221b1bb 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsNoOpTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintedObjectsNoOpTest.groovy @@ -18,9 +18,7 @@ class TaintedObjectsNoOpTest extends Specification { tainted == null instance.get(toTaint) == null instance.count() == 0 - instance.estimatedSize == 0 instance.size() == 0 - !instance.flat !instance.iterator().hasNext() when: diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/TelemetryRequestEndedHandlerTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/TelemetryRequestEndedHandlerTest.groovy index 30a61959d98..d75b3ad01c1 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/TelemetryRequestEndedHandlerTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/TelemetryRequestEndedHandlerTest.groovy @@ -63,7 +63,7 @@ class TelemetryRequestEndedHandlerTest extends AbstractTelemetryCallbackTest { void 'test telemetry with request scoped metric'() { given: final handler = new TelemetryRequestEndedHandler(delegate) - final metric = TAINTED_FLAT_MODE + final metric = REQUEST_TAINTED when: iastCtx.metricCollector.addMetric(metric, (byte) -1, 1) @@ -79,7 +79,7 @@ class TelemetryRequestEndedHandlerTest extends AbstractTelemetryCallbackTest { then: drained.size() == 1 - drained[0].metric == TAINTED_FLAT_MODE + drained[0].metric == REQUEST_TAINTED drained[0].type == 'count' drained[0].value.longValue() == 1 } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetryTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetryTest.groovy index c20ca32dace..4029d1d00c7 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetryTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/telemetry/taint/TaintedObjectsWithTelemetryTest.groovy @@ -85,27 +85,7 @@ class TaintedObjectsWithTelemetryTest extends DDSpecification { verbosity << Verbosity.values().toList() } - void 'test tainted.flat.mode with #verbosity'() { - given: - final taintedObjects = TaintedObjectsWithTelemetry.build(verbosity, Mock(TaintedObjects) { - isFlat() >> true - }) - - when: - taintedObjects.release() - - then: - if (IastMetric.TAINTED_FLAT_MODE.isEnabled(verbosity) && taintedObjects.isFlat()) { - 1 * mockCollector.addMetric(IastMetric.TAINTED_FLAT_MODE, _, _) - } else { - 0 * mockCollector.addMetric - } - - where: - verbosity << Verbosity.values().toList() - } - private TaintedObject tainted() { - return new TaintedObject(UUID.randomUUID(), Ranges.EMPTY, null) + return new TaintedObject(UUID.randomUUID(), Ranges.EMPTY) } } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java b/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java index 2cafbec9ad3..68427d2a1d8 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/telemetry/IastMetric.java @@ -21,8 +21,7 @@ public enum IastMetric { EXECUTED_SINK( "executed.sink", true, Scope.REQUEST, Tag.VULNERABILITY_TYPE, Verbosity.INFORMATION), EXECUTED_TAINTED("executed.tainted", true, Scope.REQUEST, Verbosity.DEBUG), - REQUEST_TAINTED("request.tainted", true, Scope.REQUEST, Verbosity.INFORMATION), - TAINTED_FLAT_MODE("tainted.flat.mode", false, Scope.REQUEST, Verbosity.INFORMATION); + REQUEST_TAINTED("request.tainted", true, Scope.REQUEST, Verbosity.INFORMATION); private static final int COUNT; diff --git a/internal-api/src/test/groovy/datadog/trace/api/iast/telemetry/IastMetricCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/iast/telemetry/IastMetricCollectorTest.groovy index 65ef1690bcd..baee163fc04 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/iast/telemetry/IastMetricCollectorTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/iast/telemetry/IastMetricCollectorTest.groovy @@ -224,6 +224,5 @@ class IastMetricCollectorTest extends DDSpecification { IastMetric.EXECUTED_SOURCE | SourceTypes.REQUEST_HEADER_NAME IastMetric.EXECUTED_TAINTED | null - IastMetric.TAINTED_FLAT_MODE | null } }