Skip to content

feat(java): add cycle detection and depth limit to JSON serializer #383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
buongarzoni wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(java): add cycle detection and depth limit to JSON serializer #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
893c5dd
feat(java): add cycle detection and depth limit to JSON serializer
buongarzoni Oct 5, 2026
f4846c9
fix(java): stop scrubber passing unscrubbed data past its depth cap
buongarzoni Oct 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 48 additions & 20 deletions rollbar-java/src/main/java/com/rollbar/notifier/scrubbing/ScrubDataTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -51,7 +52,10 @@
*
* <p>Nested data is walked recursively: maps reachable through other maps, through
* {@link Collection}s and through object arrays are all scrubbed, up to 8 levels of nesting. The
* surrounding shape is preserved, so a list stays a list and an array stays an array.
* surrounding shape is preserved, so a list stays a list and an array stays an array. A container
* nested deeper than that is replaced by {@value #MAX_DEPTH_VALUE}, and one that contains itself is
* replaced by {@value #CIRCULAR_REFERENCE_VALUE} where it refers back, so no value this
* transformer has not walked is ever passed on.
*
* <p>The built-in header deny-list above applies to {@code Request.headers} only; every other
* slot matches on the field keys alone.
Expand Down Expand Up @@ -97,9 +101,13 @@ public final class ScrubDataTransformer implements Transformer {
);

// Recursion cap for nested containers in custom data, request payloads and Frame.locals. Every
// map, collection or array counts as one level; this also terminates cyclic structures.
// map, collection or array counts as one level.
private static final int MAX_SCRUB_DEPTH = 8;

static final String MAX_DEPTH_VALUE = "<max depth exceeded>";

static final String CIRCULAR_REFERENCE_VALUE = "<circular reference>";

private final KeyMatcher fieldKeys;
private final StringUrlSanitizer urlSanitizer;

Expand Down Expand Up @@ -399,40 +407,58 @@ private Map<String, Object> scrubObjectMap(Map<String, Object> map, KeyMatcher k
if (map == null || keys.isEmpty()) {
return map;
}
// Identity based: the values being tracked are the ones that may contain themselves, and the
// equals() and hashCode() of such a map or collection recurse forever as well.
Set<Object> ancestors = Collections.newSetFromMap(new IdentityHashMap<>());
ancestors.add(map);
// scrubMap only ever copies keys across, so a Map<String, Object> in stays one on the way out.
return (Map<String, Object>) scrubMap(map, keys, depth);
return (Map<String, Object>) scrubMap(map, keys, depth, ancestors);
}

/**
* Recursively scrubs a nested value. Maps are scrubbed by key; collections and object arrays are
* traversed so that the maps they contain are scrubbed too, preserving the surrounding shape.
* Every container counts as one level against {@code MAX_SCRUB_DEPTH}, which also terminates
* cyclic structures. Anything else is returned untouched.
* Anything else is returned untouched.
*
* <p>A container is never returned as is without having been walked, since whatever it holds
* would then reach the payload unscrubbed. Every container counts as one level against
* {@code MAX_SCRUB_DEPTH}, and one past it is replaced by {@link #MAX_DEPTH_VALUE}.
* {@code ancestors} holds the containers enclosing this value, so finding it there means it
* contains itself, and it is replaced by {@link #CIRCULAR_REFERENCE_VALUE}.
*/
private Object scrubNested(Object value, KeyMatcher keys, int depth) {
if (depth >= MAX_SCRUB_DEPTH) {
private Object scrubNested(Object value, KeyMatcher keys, int depth, Set<Object> ancestors) {
if (!(value instanceof Map || value instanceof Collection || value instanceof Object[])) {
return value;
}
if (value instanceof Map) {
return scrubMap((Map<?, ?>) value, keys, depth + 1);
if (ancestors.contains(value)) {
return CIRCULAR_REFERENCE_VALUE;
}
if (value instanceof Collection) {
return scrubCollection((Collection<?>) value, keys, depth + 1);
if (depth >= MAX_SCRUB_DEPTH) {
return MAX_DEPTH_VALUE;
}
if (value instanceof Object[]) {
return scrubArray((Object[]) value, keys, depth + 1);

ancestors.add(value);
try {
if (value instanceof Map) {
return scrubMap((Map<?, ?>) value, keys, depth + 1, ancestors);
}
if (value instanceof Collection) {
return scrubCollection((Collection<?>) value, keys, depth + 1, ancestors);
}
return scrubArray((Object[]) value, keys, depth + 1, ancestors);
} finally {
ancestors.remove(value);
}
return value;
}

private Object scrubMap(Map<?, ?> map, KeyMatcher keys, int depth) {
private Object scrubMap(Map<?, ?> map, KeyMatcher keys, int depth, Set<Object> ancestors) {
Map<Object, Object> result = null;
for (Map.Entry<?, ?> entry : map.entrySet()) {
Object key = entry.getKey();
Object value = entry.getValue();
// A non-String key cannot match a redactedKeys pattern, but its value is still traversed.
boolean keyMatches = key instanceof String && keys.matches((String) key);
Object scrubbed = keyMatches ? SCRUBBED_VALUE : scrubNested(value, keys, depth);
Object scrubbed = keyMatches ? SCRUBBED_VALUE : scrubNested(value, keys, depth, ancestors);
if (keyMatches || scrubbed != value) {
if (result == null) {
result = new LinkedHashMap<>(map);
Expand All @@ -443,14 +469,15 @@ private Object scrubMap(Map<?, ?> map, KeyMatcher keys, int depth) {
return result != null ? result : map;
}

private Object scrubCollection(Collection<?> collection, KeyMatcher keys, int depth) {
private Object scrubCollection(Collection<?> collection, KeyMatcher keys, int depth,
Set<Object> ancestors) {
if (collection.isEmpty()) {
return collection;
}
List<Object> scrubbed = new ArrayList<>(collection.size());
boolean changed = false;
for (Object element : collection) {
Object scrubbedElement = scrubNested(element, keys, depth);
Object scrubbedElement = scrubNested(element, keys, depth, ancestors);
scrubbed.add(scrubbedElement);
if (scrubbedElement != element) {
changed = true;
Expand All @@ -464,10 +491,11 @@ private Object scrubCollection(Collection<?> collection, KeyMatcher keys, int de
return collection instanceof Set ? new LinkedHashSet<>(scrubbed) : scrubbed;
}

private Object scrubArray(Object[] array, KeyMatcher keys, int depth) {
private Object scrubArray(Object[] array, KeyMatcher keys, int depth,
Set<Object> ancestors) {
Object[] result = null;
for (int i = 0; i < array.length; i++) {
Object scrubbedElement = scrubNested(array[i], keys, depth);
Object scrubbedElement = scrubNested(array[i], keys, depth, ancestors);
if (scrubbedElement != array[i]) {
if (result == null) {
// Object[] rather than array.clone(): a rebuilt value may not fit the original component
Expand Down
111 changes: 96 additions & 15 deletions rollbar-java/src/main/java/com/rollbar/notifier/sender/json/JsonSerializerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,24 @@
import com.rollbar.api.payload.Payload;
import com.rollbar.notifier.sender.result.Result;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* Implementation of the {@link JsonSerializer json serializer}.
*
* <p>
* Payloads carry arbitrary user data, such as custom maps and captured local variables, so the
* serializer guards against object graphs that would otherwise make it recurse without bound. A
* map, collection, array or {@link JsonSerializable} that refers back to one of its enclosing
* values is replaced by {@value #CIRCULAR_REFERENCE_PLACEHOLDER}, and one nested deeper than
* {@code maxDepth} levels is replaced by {@value #MAX_DEPTH_PLACEHOLDER}.
* </p>
*/
public class JsonSerializerImpl implements JsonSerializer {

Expand All @@ -38,8 +49,27 @@ public class JsonSerializerImpl implements JsonSerializer {
REPLACEMENT_CHARS['\f'] = "\\f";
}

/**
* The default maximum nesting depth, see {@link #JsonSerializerImpl(boolean, int)}.
*/
public static final int DEFAULT_MAX_DEPTH = 100;

/**
* The lowest maximum nesting depth accepted, see {@link #JsonSerializerImpl(boolean, int)}.
* Depth counts from the root of the payload, whose own structure takes up the first levels:
* custom data starts at level 3 and the captured local variables of a stack trace at level 7,
* so a lower limit would cut the stack trace itself and leave a payload the API cannot use.
*/
public static final int MIN_MAX_DEPTH = 10;

static final String CIRCULAR_REFERENCE_PLACEHOLDER = "<circular reference>";

static final String MAX_DEPTH_PLACEHOLDER = "<max depth exceeded>";

private final boolean prettyPrint;

private final int maxDepth;

/**
* Construct a JsonSerializerImpl that does <b>not</b> pretty print the Payload.
*/
Expand All @@ -52,7 +82,23 @@ public JsonSerializerImpl() {
* @param prettyPrint whether or not to pretty print the payload.
*/
public JsonSerializerImpl(boolean prettyPrint) {
this(prettyPrint, DEFAULT_MAX_DEPTH);
}

/**
* Construct a JsonSerializerImpl.
* @param prettyPrint whether or not to pretty print the payload.
* @param maxDepth the maximum number of nested maps, collections and arrays to serialize,
* counting the outermost object. Anything nested deeper is replaced by a
* placeholder string. Must be at least {@value #MIN_MAX_DEPTH}.
*/
public JsonSerializerImpl(boolean prettyPrint, int maxDepth) {
if (maxDepth < MIN_MAX_DEPTH) {
throw new IllegalArgumentException(
"The max depth must be at least " + MIN_MAX_DEPTH + ", got " + maxDepth);
}
this.prettyPrint = prettyPrint;
this.maxDepth = maxDepth;
}

@Override
Expand All @@ -71,7 +117,10 @@ public String toJson(Payload payload) {
*/
public String toJson(Map<String, Object> map) {
StringBuilder builder = new StringBuilder();
serializeObject(map, builder, 0);
// Identity based: the values being tracked are the ones that may contain themselves, and the
// equals() and hashCode() of such a map or collection recurse forever as well.
Set<Object> visiting = Collections.newSetFromMap(new IdentityHashMap<Object, Boolean>());
serializeValue(builder, map, 0, visiting);
return builder.toString();
}

Expand Down Expand Up @@ -109,7 +158,8 @@ public Result resultFrom(String response) {
}


private void serializeObject(Map<String, Object> content, StringBuilder builder, int level) {
private void serializeObject(Map<String, Object> content, StringBuilder builder, int level,
Set<Object> visiting) {
builder.append('{');

String comma = "";
Expand All @@ -128,7 +178,7 @@ private void serializeObject(Map<String, Object> content, StringBuilder builder,
builder.append(" ");
}

serializeValue(builder, entry.getValue(), level + 1);
serializeValue(builder, entry.getValue(), level + 1, visiting);
}
if (prettyPrint) {
builder.append("\n");
Expand All @@ -137,7 +187,8 @@ private void serializeObject(Map<String, Object> content, StringBuilder builder,
builder.append('}');
}

private void serializeValue(StringBuilder builder, Object value, int level) {
private void serializeValue(StringBuilder builder, Object value, int level,
Set<Object> visiting) {
if (value == null) {
serializeNull(builder);
} else if (value instanceof Boolean) {
Expand All @@ -146,22 +197,51 @@ private void serializeValue(StringBuilder builder, Object value, int level) {
serializeNumber(builder, (Number) value);
} else if (value instanceof String) {
serializeString(builder, (String) value);
} else if (value instanceof JsonSerializable) {
serializeValue(builder, ((JsonSerializable) value).asJson(), level);
} else if (value instanceof Map) {
Map<String, Object> obj = asMap((Map) value);
serializeObject(obj, builder, level);
} else if (value instanceof Collection) {
serializeArray(builder, ((Collection) value).toArray(), level);
} else if (value instanceof Object[]) {
serializeArray(builder, (Object[]) value, level);
} else if (value instanceof JsonSerializable || value instanceof Map
|| value instanceof Collection || value instanceof Object[]) {
serializeNested(builder, value, level, visiting);
} else if (value instanceof Throwable) {
serializeThrowable(builder, (Throwable) value);
} else {
serializeDefault(builder, value);
}
}

/**
* Serializes a value that can contain other values, unless doing so would recurse without
* bound. {@code visiting} holds the values enclosing this one, so finding the value there means
* it contains itself. Values are removed again on the way out, so the same value reached twice
* through different paths, without being its own ancestor, is serialized both times.
*/
private void serializeNested(StringBuilder builder, Object value, int level,
Set<Object> visiting) {
if (visiting.contains(value)) {
serializeString(builder, CIRCULAR_REFERENCE_PLACEHOLDER);
return;
}
if (level >= maxDepth) {
serializeString(builder, MAX_DEPTH_PLACEHOLDER);
return;
}

visiting.add(value);
try {
if (value instanceof JsonSerializable) {
// Not a level of its own: it stands for the value it converts to.
serializeValue(builder, ((JsonSerializable) value).asJson(), level, visiting);
} else if (value instanceof Map) {
Map<String, Object> obj = asMap((Map) value);
serializeObject(obj, builder, level, visiting);
} else if (value instanceof Collection) {
serializeArray(builder, ((Collection) value).toArray(), level, visiting);
} else {
serializeArray(builder, (Object[]) value, level, visiting);
}
} finally {
visiting.remove(value);
}
}

private static void serializeThrowable(StringBuilder builder, Throwable value) {
serializeToString(builder, value);
}
Expand Down Expand Up @@ -220,7 +300,8 @@ private static void serializeNull(StringBuilder builder) {
builder.append("null");
}

private void serializeArray(StringBuilder builder, Object[] array, int level) {
private void serializeArray(StringBuilder builder, Object[] array, int level,
Set<Object> visiting) {
builder.append('[');
String comma = "";
for (Object obj : array) {
Expand All @@ -231,7 +312,7 @@ private void serializeArray(StringBuilder builder, Object[] array, int level) {
builder.append("\n");
indent(builder, level);
}
serializeValue(builder, obj, level + 1);
serializeValue(builder, obj, level + 1, visiting);
}
builder.append(']');
}
Expand Down
Loading
Loading