fix: GSON parser now throws IOException on invalid JSON input (#1355)

Previously, the `GsonParser` class threw an `IllegalArgument` (unchecked) exception. This does not change the interface for `JsonParser` which already declares `throws IOException`
googleapis · Aug 26, 2021 · 0a505a7 · 0a505a7
1 parent 4d85543
commit 0a505a7
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 39 deletions.
diff --git a/google-http-client-gson/pom.xml b/google-http-client-gson/pom.xml
@@ -81,10 +81,5 @@
       <groupId>com.google.code.gson</groupId>
       <artifactId>gson</artifactId>
     </dependency>
-    <dependency>
-      <groupId>com.google.guava</groupId>
-      <artifactId>guava</artifactId>
-      <scope>test</scope>
-    </dependency>
   </dependencies>
 </project>
diff --git a/google-http-client-gson/src/main/java/com/google/api/client/json/gson/GsonParser.java b/google-http-client-gson/src/main/java/com/google/api/client/json/gson/GsonParser.java
@@ -17,7 +17,6 @@
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonParser;
 import com.google.api.client.json.JsonToken;
-import com.google.api.client.util.Preconditions;
 import com.google.gson.stream.JsonReader;
 import java.io.EOFException;
 import java.io.IOException;
@@ -44,8 +43,7 @@ class GsonParser extends JsonParser {
   GsonParser(GsonFactory factory, JsonReader reader) {
     this.factory = factory;
     this.reader = reader;
-    // lenient to allow top-level values of any type
-    reader.setLenient(true);
+    reader.setLenient(false);
   }
 
   @Override
@@ -69,56 +67,58 @@ public JsonFactory getFactory() {
   }
 
   @Override
-  public byte getByteValue() {
+  public byte getByteValue() throws IOException {
     checkNumber();
     return Byte.parseByte(currentText);
   }
 
   @Override
-  public short getShortValue() {
+  public short getShortValue() throws IOException {
     checkNumber();
     return Short.parseShort(currentText);
   }
 
   @Override
-  public int getIntValue() {
+  public int getIntValue() throws IOException {
     checkNumber();
     return Integer.parseInt(currentText);
   }
 
   @Override
-  public float getFloatValue() {
+  public float getFloatValue() throws IOException {
     checkNumber();
     return Float.parseFloat(currentText);
   }
 
   @Override
-  public BigInteger getBigIntegerValue() {
+  public BigInteger getBigIntegerValue() throws IOException {
     checkNumber();
     return new BigInteger(currentText);
   }
 
   @Override
-  public BigDecimal getDecimalValue() {
+  public BigDecimal getDecimalValue() throws IOException {
     checkNumber();
     return new BigDecimal(currentText);
   }
 
   @Override
-  public double getDoubleValue() {
+  public double getDoubleValue() throws IOException {
     checkNumber();
     return Double.parseDouble(currentText);
   }
 
   @Override
-  public long getLongValue() {
+  public long getLongValue() throws IOException {
     checkNumber();
     return Long.parseLong(currentText);
   }
 
-  private void checkNumber() {
-    Preconditions.checkArgument(
-        currentToken == JsonToken.VALUE_NUMBER_INT || currentToken == JsonToken.VALUE_NUMBER_FLOAT);
+  private void checkNumber() throws IOException {
+    if (currentToken != JsonToken.VALUE_NUMBER_INT
+        && currentToken != JsonToken.VALUE_NUMBER_FLOAT) {
+      throw new IOException("Token is not a number");
+    }
   }
 
   @Override

diff --git a/google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java b/google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Google Inc.
+ * Copyright 2010 Google Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -17,8 +17,7 @@
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.JsonParser;
 import com.google.api.client.test.json.AbstractJsonFactoryTest;
-import com.google.common.base.Charsets;
-import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.util.ArrayList;
 
 /**
@@ -80,10 +79,19 @@ public final void testToPrettyString_Feed() throws Exception {
     assertEquals(JSON_FEED_PRETTY, newFactory().toPrettyString(feed));
   }
 
-  public final void testParse_directValue() throws Exception {
-    byte[] jsonData = Charsets.UTF_8.encode("123").array();
-    JsonParser jp =
-        newFactory().createJsonParser(new ByteArrayInputStream(jsonData), Charsets.UTF_8);
-    assertEquals(123, jp.parse(Integer.class, true));
+  public final void testParse_directValue() throws IOException {
+    JsonParser parser = newFactory().createJsonParser("123");
+    assertEquals(123, parser.parse(Integer.class, true));
+  }
+
+  public final void testGetByteValue() throws IOException {
+    JsonParser parser = newFactory().createJsonParser("123");
+
+    try {
+      parser.getByteValue();
+      fail("should throw IOException");
+    } catch (IOException ex) {
+      assertNotNull(ex.getMessage());
+    }
   }
 }
diff --git a/google-http-client-jackson2/pom.xml b/google-http-client-jackson2/pom.xml
@@ -80,10 +80,5 @@
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-core</artifactId>
     </dependency>
-    <dependency>
-      <groupId>com.google.guava</groupId>
-      <artifactId>guava</artifactId>
-      <scope>test</scope>
-    </dependency>
   </dependencies>
 </project>
diff --git a/...client-jackson2/src/test/java/com/google/api/client/json/jackson2/JacksonFactoryTest.java b/...client-jackson2/src/test/java/com/google/api/client/json/jackson2/JacksonFactoryTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012 Google Inc.
+ * Copyright 2012 Google Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License. You may obtain a copy of the License at
@@ -18,8 +18,7 @@
 import com.google.api.client.json.JsonParser;
 import com.google.api.client.test.json.AbstractJsonFactoryTest;
 import com.google.api.client.util.StringUtils;
-import com.google.common.base.Charsets;
-import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.util.ArrayList;
 
 /**
@@ -74,9 +73,18 @@ public final void testToPrettyString_Feed() throws Exception {
   }
 
   public final void testParse_directValue() throws Exception {
-    byte[] jsonData = Charsets.UTF_8.encode("123").array();
-    JsonParser jp =
-        newFactory().createJsonParser(new ByteArrayInputStream(jsonData), Charsets.UTF_8);
-    assertEquals(123, jp.parse(Integer.class, true));
+    JsonParser parser = newFactory().createJsonParser("123");
+    assertEquals(123, parser.parse(Integer.class, true));
+  }
+
+  public final void testGetByteValue() throws IOException {
+    JsonParser parser = newFactory().createJsonParser("123");
+
+    try {
+      parser.getByteValue();
+      fail("should throw IOException");
+    } catch (IOException ex) {
+      assertNotNull(ex.getMessage());
+    }
   }
 }
diff --git a/...ttp-client-test/src/main/java/com/google/api/client/test/json/AbstractJsonParserTest.java b/...ttp-client-test/src/main/java/com/google/api/client/test/json/AbstractJsonParserTest.java
@@ -22,6 +22,7 @@
 import java.math.BigDecimal;
 import java.nio.charset.StandardCharsets;
 import junit.framework.TestCase;
+import org.junit.Assert;
 
 public abstract class AbstractJsonParserTest extends TestCase {
 
@@ -44,6 +45,29 @@ public void testParse_basic() throws IOException {
     assertEquals(Boolean.FALSE, json.get("boolValue"));
   }
 
+  public void testGetWrongType() throws IOException {
+    JsonObjectParser parser = new JsonObjectParser(newJsonFactory());
+    InputStream inputStream = new ByteArrayInputStream(TEST_JSON.getBytes(StandardCharsets.UTF_8));
+    GenericJson json = parser.parseAndClose(inputStream, StandardCharsets.UTF_8, GenericJson.class);
+    assertTrue(json.get("strValue") instanceof String);
+    assertEquals("bar", json.get("strValue"));
+    assertTrue(json.get("intValue") instanceof BigDecimal);
+    assertEquals(new BigDecimal(123), json.get("intValue"));
+    assertTrue(json.get("boolValue") instanceof Boolean);
+    assertEquals(Boolean.FALSE, json.get("boolValue"));
+  }
+
+  public void testParse_badJson() throws IOException {
+    JsonObjectParser parser = new JsonObjectParser(newJsonFactory());
+    InputStream inputStream = new ByteArrayInputStream("not json".getBytes(StandardCharsets.UTF_8));
+    try {
+      parser.parseAndClose(inputStream, StandardCharsets.UTF_8, GenericJson.class);
+      fail("Malformed JSON not detected");
+    } catch (IOException ex) {
+      Assert.assertNotNull(ex.getMessage());
+    }
+  }
+
   public void testParse_bigDecimal() throws IOException {
     JsonObjectParser parser = new JsonObjectParser(newJsonFactory());
     InputStream inputStream =