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 1 commit 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
1 commit
Select commit
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
102 changes: 87 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,19 @@ 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;

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 +74,22 @@ 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 1.
*/
public JsonSerializerImpl(boolean prettyPrint, int maxDepth) {
if (maxDepth < 1) {
throw new IllegalArgumentException("The max depth must be at least 1, got " + maxDepth);
}
this.prettyPrint = prettyPrint;
this.maxDepth = maxDepth;
}

@Override
Expand All @@ -71,7 +108,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 +149,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 +169,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 +178,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 +188,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 +291,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 +303,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
150 changes: 149 additions & 1 deletion rollbar-java/src/test/java/com/rollbar/notifier/sender/json/JsonSerializerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,25 @@
import static com.rollbar.notifier.sender.json.JsonTestHelper.assertValidJson;
import static com.rollbar.notifier.sender.json.JsonTestHelper.fromString;
import static com.rollbar.notifier.sender.json.JsonTestHelper.getValue;
import static com.rollbar.notifier.sender.json.JsonSerializerImpl.CIRCULAR_REFERENCE_PLACEHOLDER;
import static com.rollbar.notifier.sender.json.JsonSerializerImpl.MAX_DEPTH_PLACEHOLDER;
import static java.lang.String.format;
import static java.util.Arrays.asList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.core.Is.is;

import com.rollbar.api.json.JsonSerializable;
import com.rollbar.api.payload.Payload;
import com.rollbar.api.payload.data.Data;
import com.rollbar.notifier.sender.result.Result;
import org.junit.Test;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

public class JsonSerializerImplTest {
Expand Down Expand Up @@ -218,7 +225,148 @@ public void shouldSerializeNullMapKeyWithoutProducingInvalidJson() {

assertValidJson(serialized);
Map<String, Object> custom = getValue(fromString(serialized), "data", "custom");
assertThat(custom.get(""), equalTo((Object) "a value"));
assertThat(custom.get(""), equalTo("a value"));
}

@Test
public void shouldReplaceMapThatContainsItself() {
Map<String, Object> map = new HashMap<>();
map.put("self", map);
map.put("name", "value");

String serialized = new JsonSerializerImpl().toJson(map);

assertValidJson(serialized);
Map<String, Object> result = fromString(serialized);
assertThat(result.get("self"), equalTo(CIRCULAR_REFERENCE_PLACEHOLDER));
assertThat(result.get("name"), equalTo("value"));
}

@Test
public void shouldReplaceCycleThroughNestedCollections() {
Map<String, Object> parent = new LinkedHashMap<>();
List<Object> children = new ArrayList<>();
Map<String, Object> child = new LinkedHashMap<>();
child.put("parent", parent);
children.add(child);
parent.put("children", children);

String serialized = new JsonSerializerImpl().toJson(payloadWithCustom("parent", parent));

assertValidJson(serialized);
List<Object> serializedChildren = getValue(fromString(serialized),
"data", "custom", "parent", "children");
Map<?, ?> serializedChild = (Map<?, ?>) serializedChildren.get(0);
assertThat(serializedChild.get("parent"), equalTo(CIRCULAR_REFERENCE_PLACEHOLDER));
}

@Test
public void shouldReplaceCollectionThatContainsItself() {
List<Object> list = new ArrayList<>();
list.add("first");
list.add(list);

String serialized = new JsonSerializerImpl().toJson(payloadWithCustom("list", list));

assertValidJson(serialized);
List<Object> result = getValue(fromString(serialized), "data", "custom", "list");
assertThat(result, equalTo(asList((Object) "first", CIRCULAR_REFERENCE_PLACEHOLDER)));
}

@Test
public void shouldReplaceArrayThatContainsItself() {
Object[] array = new Object[2];
array[0] = "first";
array[1] = array;

String serialized = new JsonSerializerImpl().toJson(payloadWithCustom("array", array));

assertValidJson(serialized);
List<Object> result = getValue(fromString(serialized), "data", "custom", "array");
assertThat(result, equalTo(asList((Object) "first", CIRCULAR_REFERENCE_PLACEHOLDER)));
}

@Test
public void shouldReplaceJsonSerializableThatContainsItself() {
SelfReferencing value = new SelfReferencing();

String serialized = new JsonSerializerImpl().toJson(payloadWithCustom("value", value));

assertValidJson(serialized);
Map<String, Object> result = getValue(fromString(serialized), "data", "custom", "value");
assertThat(result.get("self"), equalTo(CIRCULAR_REFERENCE_PLACEHOLDER));
}

@Test
public void shouldSerializeSharedValueEveryTimeItIsReached() {
Map<String, Object> shared = new HashMap<>();
shared.put("key", "value");
Map<String, Object> map = new LinkedHashMap<>();
map.put("first", shared);
map.put("second", shared);

String serialized = new JsonSerializerImpl().toJson(map);

assertValidJson(serialized);
Map<String, Object> result = fromString(serialized);
assertThat(result.get("first"), equalTo(shared));
assertThat(result.get("second"), equalTo(shared));
}

@Test
public void shouldReplaceValuesNestedDeeperThanMaxDepth() {
Map<String, Object> innermost = new HashMap<>();
innermost.put("key", "value");
Map<String, Object> middle = new HashMap<>();
middle.put("innermost", innermost);
Map<String, Object> outer = new HashMap<>();
outer.put("middle", middle);
outer.put("list", asList("a", "b"));

String serialized = new JsonSerializerImpl(false, 2).toJson(outer);

assertValidJson(serialized);
Map<String, Object> result = fromString(serialized);
assertThat(result.get("list"), equalTo(asList("a", "b")));
Map<?, ?> serializedMiddle = (Map<?, ?>) result.get("middle");
assertThat(serializedMiddle.get("innermost"), equalTo(MAX_DEPTH_PLACEHOLDER));
}

@Test
public void shouldLimitDeeplyNestedValuesToDefaultMaxDepth() {
Map<String, Object> root = new HashMap<>();
Map<String, Object> current = root;
for (int i = 0; i < 100_000; i++) {
Map<String, Object> next = new HashMap<>();
current.put("next", next);
current = next;
}

String serialized = new JsonSerializerImpl().toJson(root);

assertValidJson(serialized);
Object value = fromString(serialized);
int depth = 0;
while (value instanceof Map) {
value = ((Map<?, ?>) value).get("next");
depth++;
}
assertThat(depth, is(JsonSerializerImpl.DEFAULT_MAX_DEPTH));
assertThat(value, equalTo(MAX_DEPTH_PLACEHOLDER));
}

@Test(expected = IllegalArgumentException.class)
public void shouldRejectMaxDepthBelowOne() {
new JsonSerializerImpl(false, 0);
}

private static class SelfReferencing implements JsonSerializable {
@Override
public Object asJson() {
Map<String, Object> json = new HashMap<>();
json.put("self", this);
return json;
}
}

private Payload payloadWithCustom(String key, Object value) {
Expand Down
Loading