Merge pull request #2818 from xfxyjwf/i1470

Don't expose gson exceptions in JsonFormat.

2 files changed, 52 insertions, 3 deletions

diff --git a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
index ac712c94..838700f7 100644
--- a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
+++ b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
@@ -35,6 +35,7 @@ import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
+import com.google.gson.JsonIOException;
 import com.google.gson.JsonNull;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
@@ -1067,9 +1068,23 @@ public class JsonFormat {
     }
 
     void merge(Reader json, Message.Builder builder) throws IOException {
-      JsonReader reader = new JsonReader(json);
-      reader.setLenient(false);
-      merge(jsonParser.parse(reader), builder);
+      try {
+        JsonReader reader = new JsonReader(json);
+        reader.setLenient(false);
+        merge(jsonParser.parse(reader), builder);
+      } catch (InvalidProtocolBufferException e) {
+        throw e;
+      } catch (JsonIOException e) {
+        // Unwrap IOException.
+        if (e.getCause() instanceof IOException) {
+          throw (IOException) e.getCause();
+        } else {
+          throw new InvalidProtocolBufferException(e.getMessage());
+        }
+      } catch (Exception e) {
+        // We convert all exceptions from JSON parsing to our own exceptions.
+        throw new InvalidProtocolBufferException(e.getMessage());
+      }
     }
 
     void merge(String json, Message.Builder builder) throws InvalidProtocolBufferException {
diff --git a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
index 883706c1..de02c117 100644
--- a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
+++ b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
@@ -62,6 +62,10 @@ import com.google.protobuf.util.JsonTestProto.TestStruct;
 import com.google.protobuf.util.JsonTestProto.TestTimestamp;
 import com.google.protobuf.util.JsonTestProto.TestWrappers;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.StringReader;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.HashMap;
@@ -1417,4 +1421,34 @@ public class JsonFormatTest extends TestCase {
       // Expected.
     }
   }
+
+  // Test that we are not leaking out JSON exceptions.
+  public void testJsonException() throws Exception {
+    InputStream throwingInputStream = new InputStream() {
+      public int read() throws IOException {
+        throw new IOException("12345");
+      }
+    };
+    InputStreamReader throwingReader = new InputStreamReader(throwingInputStream);
+    // When the underlying reader throws IOException, JsonFormat should forward
+    // through this IOException.
+    try {
+      TestAllTypes.Builder builder = TestAllTypes.newBuilder();
+      JsonFormat.parser().merge(throwingReader, builder);
+      fail("Exception is expected.");
+    } catch (IOException e) {
+      assertEquals("12345", e.getMessage());
+    }
+
+    Reader invalidJsonReader = new StringReader("{ xxx - yyy }");
+    // When the JSON parser throws parser exceptions, JsonFormat should turn
+    // that into InvalidProtocolBufferException.
+    try {
+      TestAllTypes.Builder builder = TestAllTypes.newBuilder();
+      JsonFormat.parser().merge(invalidJsonReader, builder);
+      fail("Exception is expected.");
+    } catch (InvalidProtocolBufferException e) {
+      // Expected.
+    }
+  }
 }