diff --git a/rollbar-java/src/main/java/com/rollbar/notifier/sender/json/JsonSerializerImpl.java b/rollbar-java/src/main/java/com/rollbar/notifier/sender/json/JsonSerializerImpl.java
index aeede921..8ffe61df 100755
--- a/rollbar-java/src/main/java/com/rollbar/notifier/sender/json/JsonSerializerImpl.java
+++ b/rollbar-java/src/main/java/com/rollbar/notifier/sender/json/JsonSerializerImpl.java
@@ -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}.
+ *
+ *
+ * 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}.
+ *
*/
public class JsonSerializerImpl implements JsonSerializer {
@@ -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 = "";
+
+ static final String MAX_DEPTH_PLACEHOLDER = "";
+
private final boolean prettyPrint;
+ private final int maxDepth;
+
/**
* Construct a JsonSerializerImpl that does not pretty print the Payload.
*/
@@ -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
@@ -71,7 +108,10 @@ public String toJson(Payload payload) {
*/
public String toJson(Map 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 visiting = Collections.newSetFromMap(new IdentityHashMap());
+ serializeValue(builder, map, 0, visiting);
return builder.toString();
}
@@ -109,7 +149,8 @@ public Result resultFrom(String response) {
}
- private void serializeObject(Map content, StringBuilder builder, int level) {
+ private void serializeObject(Map content, StringBuilder builder, int level,
+ Set visiting) {
builder.append('{');
String comma = "";
@@ -128,7 +169,7 @@ private void serializeObject(Map content, StringBuilder builder,
builder.append(" ");
}
- serializeValue(builder, entry.getValue(), level + 1);
+ serializeValue(builder, entry.getValue(), level + 1, visiting);
}
if (prettyPrint) {
builder.append("\n");
@@ -137,7 +178,8 @@ private void serializeObject(Map content, StringBuilder builder,
builder.append('}');
}
- private void serializeValue(StringBuilder builder, Object value, int level) {
+ private void serializeValue(StringBuilder builder, Object value, int level,
+ Set visiting) {
if (value == null) {
serializeNull(builder);
} else if (value instanceof Boolean) {
@@ -146,15 +188,9 @@ 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 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 {
@@ -162,6 +198,41 @@ private void serializeValue(StringBuilder builder, Object value, int level) {
}
}
+ /**
+ * 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 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 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);
}
@@ -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 visiting) {
builder.append('[');
String comma = "";
for (Object obj : array) {
@@ -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(']');
}
diff --git a/rollbar-java/src/test/java/com/rollbar/notifier/sender/json/JsonSerializerImplTest.java b/rollbar-java/src/test/java/com/rollbar/notifier/sender/json/JsonSerializerImplTest.java
index 7a139001..3fba12e9 100644
--- a/rollbar-java/src/test/java/com/rollbar/notifier/sender/json/JsonSerializerImplTest.java
+++ b/rollbar-java/src/test/java/com/rollbar/notifier/sender/json/JsonSerializerImplTest.java
@@ -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 {
@@ -218,7 +225,148 @@ public void shouldSerializeNullMapKeyWithoutProducingInvalidJson() {
assertValidJson(serialized);
Map custom = getValue(fromString(serialized), "data", "custom");
- assertThat(custom.get(""), equalTo((Object) "a value"));
+ assertThat(custom.get(""), equalTo("a value"));
+ }
+
+ @Test
+ public void shouldReplaceMapThatContainsItself() {
+ Map map = new HashMap<>();
+ map.put("self", map);
+ map.put("name", "value");
+
+ String serialized = new JsonSerializerImpl().toJson(map);
+
+ assertValidJson(serialized);
+ Map result = fromString(serialized);
+ assertThat(result.get("self"), equalTo(CIRCULAR_REFERENCE_PLACEHOLDER));
+ assertThat(result.get("name"), equalTo("value"));
+ }
+
+ @Test
+ public void shouldReplaceCycleThroughNestedCollections() {
+ Map parent = new LinkedHashMap<>();
+ List children = new ArrayList<>();
+ Map 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 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 list = new ArrayList<>();
+ list.add("first");
+ list.add(list);
+
+ String serialized = new JsonSerializerImpl().toJson(payloadWithCustom("list", list));
+
+ assertValidJson(serialized);
+ List 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 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 result = getValue(fromString(serialized), "data", "custom", "value");
+ assertThat(result.get("self"), equalTo(CIRCULAR_REFERENCE_PLACEHOLDER));
+ }
+
+ @Test
+ public void shouldSerializeSharedValueEveryTimeItIsReached() {
+ Map shared = new HashMap<>();
+ shared.put("key", "value");
+ Map map = new LinkedHashMap<>();
+ map.put("first", shared);
+ map.put("second", shared);
+
+ String serialized = new JsonSerializerImpl().toJson(map);
+
+ assertValidJson(serialized);
+ Map result = fromString(serialized);
+ assertThat(result.get("first"), equalTo(shared));
+ assertThat(result.get("second"), equalTo(shared));
+ }
+
+ @Test
+ public void shouldReplaceValuesNestedDeeperThanMaxDepth() {
+ Map innermost = new HashMap<>();
+ innermost.put("key", "value");
+ Map middle = new HashMap<>();
+ middle.put("innermost", innermost);
+ Map outer = new HashMap<>();
+ outer.put("middle", middle);
+ outer.put("list", asList("a", "b"));
+
+ String serialized = new JsonSerializerImpl(false, 2).toJson(outer);
+
+ assertValidJson(serialized);
+ Map 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 root = new HashMap<>();
+ Map current = root;
+ for (int i = 0; i < 100_000; i++) {
+ Map 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 json = new HashMap<>();
+ json.put("self", this);
+ return json;
+ }
}
private Payload payloadWithCustom(String key, Object value) {