Repository: rollbar/rollbar-java
Pull request #39 (2017-05-31 02:31:15)

Fix json
For some reason we have our own JSON serializer, and as it turns out it doesn't handle the edge cases inside strings well (who woulda thought?). This makes the serialization of strings better by borrowing how gson does this. The reality is that this whole thing needs a rewrite, or better yet probably we should just bring in gson or jackson. But nonetheless, this should fix some issues with strings with escaped quotes inside them. I added some tests too.


Patch:
diff --git a/rollbar-utilities/src/main/java/com/rollbar/utilities/RollbarSerializer.java b/rollbar-utilities/src/main/java/com/rollbar/utilities/RollbarSerializer.java
index b4150ec..98e593e 100644
--- a/rollbar-utilities/src/main/java/com/rollbar/utilities/RollbarSerializer.java
+++ b/rollbar-utilities/src/main/java/com/rollbar/utilities/RollbarSerializer.java
@@ -6,6 +6,20 @@ import java.util.*;
  * A Payload Serializer
  */
 public class RollbarSerializer implements JsonSerializer {
+    private static final String[] REPLACEMENT_CHARS;
+    static {
+      REPLACEMENT_CHARS = new String[128];
+      for (int i = 0; i <= 0x1f; i++) {
+        REPLACEMENT_CHARS[i] = String.format("\\u%04x", (int) i);
+      }
+      REPLACEMENT_CHARS['"'] = "\\\"";
+      REPLACEMENT_CHARS['\\'] = "\\\\";
+      REPLACEMENT_CHARS['\t'] = "\\t";
+      REPLACEMENT_CHARS['\b'] = "\\b";
+      REPLACEMENT_CHARS['\n'] = "\\n";
+      REPLACEMENT_CHARS['\r'] = "\\r";
+      REPLACEMENT_CHARS['\f'] = "\\f";
+    }
     private final boolean prettyPrint;
 
     /**
@@ -130,9 +144,36 @@ public class RollbarSerializer implements JsonSerializer {
         }
     }
 
+    // Borrowed from
+    // https://github.com/google/gson/blob/59edfc1caf2bb30e30f523f8502f23e8f8edc38e/gson/src/main/java/com/google/gson/stream/JsonWriter.java
     private static void serializeString(StringBuilder builder, String str) {
         builder.append('"');
-        builder.append(str.replace("\"", "\\\""));
+        int last = 0;
+        int length = str.length();
+        for (int i = 0; i < length; i++) {
+          char c = str.charAt(i);
+          String replacement;
+          if (c < 128) {
+            replacement = REPLACEMENT_CHARS[c];
+            if (replacement == null) {
+              continue;
+            }
+          } else if (c == '\u2028') {
+            replacement = "\\u2028";
+          } else if (c == '\u2029') {
+            replacement = "\\u2029";
+          } else {
+            continue;
+          }
+          if (last < i) {
+            builder.append(str, last, i);
+          }
+          builder.append(replacement);
+          last = i + 1;
+        }
+        if (last < length) {
+          builder.append(str, last, length);
+        }
         builder.append('"');
     }
 


Test patch:
diff --git a/rollbar-utilities/src/test/java/com/rollbar/utilities/RollbarSerializerTest.java b/rollbar-utilities/src/test/java/com/rollbar/utilities/RollbarSerializerTest.java
new file mode 100644
index 0000000..2ca2553
--- /dev/null
+++ b/rollbar-utilities/src/test/java/com/rollbar/utilities/RollbarSerializerTest.java
@@ -0,0 +1,79 @@
+package com.rollbar.utilities;
+
+import org.junit.Test;
+
+import org.junit.Assert;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class RollbarSerializerTest {
+    @Test
+    public void testSerializeObject() throws Exception {
+        RollbarSerializer s = new RollbarSerializer();
+        Map<String, Object> obj = new HashMap<String, Object>();
+        obj.put("a", "a \" b");
+        obj.put("other", "some \\\" string");
+        FakeData d = new FakeData(obj);
+        String result = s.serialize(d);
+        String expected1 = "{\"a\":\"a \\\" b\",\"other\":\"some \\\\\\\" string\"}";
+        String expected2 = "{\"other\":\"some \\\\\\\" string\",\"a\":\"a \\\" b\"}";
+        Assert.assertTrue(expected1.equals(result) || expected2.equals(result));
+    }
+
+    @Test
+    public void testSerializeArray() throws Exception {
+        RollbarSerializer s = new RollbarSerializer();
+        Map<String, Object> obj = new HashMap<String, Object>();
+        obj.put("k", "v");
+        Map<String, Object> innerObj = new HashMap<String, Object>();
+        innerObj.put("x", "y");
+        obj.put("inner", innerObj);
+        Object[] a = {42,obj,"hello"};
+        FakeData d = new FakeData(a);
+        String result = s.serialize(d);
+        String expected1 = "[42,{\"k\":\"v\",\"inner\":{\"x\":\"y\"}},\"hello\"]";
+        String expected2 = "[42,{\"inner\":{\"x\":\"y\"},\"k\":\"v\"},\"hello\"]";
+        Assert.assertTrue(expected1.equals(result) || expected2.equals(result));
+    }
+
+    @Test
+    public void testSerializePretty() throws Exception {
+        RollbarSerializer s = new RollbarSerializer(true);
+        Map<String, Object> obj = new HashMap<String, Object>();
+        obj.put("key1", 42);
+        FakeData d = new FakeData(obj);
+        String result = s.serialize(d);
+        Assert.assertEquals(
+            "should serialize object properly",
+            "{\n" +
+            "  \"key1\": 42\n" +
+            "}",
+            result);
+    }
+
+    @Test
+    public void testSerializeNull() throws Exception {
+        RollbarSerializer s = new RollbarSerializer();
+        Map<String, Object> obj = new HashMap<String, Object>();
+        obj.put("key1", null);
+        FakeData d = new FakeData(obj);
+        String result = s.serialize(d);
+        Assert.assertEquals(
+            "should serialize object properly",
+            "{\"key1\":null}",
+            result);
+    }
+}
+
+class FakeData implements JsonSerializable {
+  private Object object;
+
+  public FakeData(Object object) {
+    this.object = object;
+  }
+
+  public Object asJson() {
+    return this.object;
+  }
+}