Several regressions caused by the switch to Gson

[hiranya911]

In #1661 this library moved from the old Jackson-based JSON parser to a Gson-based JSON parser. This change seems to be causing some regressions in the downstream codebases like Firebase.

1. JSON-parser leniency

Gson is a lot more lenient in the inputs it accept as valid JSON. As a result the following test case would pass in the older versions of the API client, but fails in more recent versions.

@Test
public void testLenience() {
    JsonParser parser = Utils.getDefaultJsonFactory().createJsonParser("not json");
    Map<String, Object> destination = new HashMap<>();
    try {
      parser.parseAndClose(destination);
      fail("No error thrown");
    } catch (IOException ex) {
      // expected
    }
}

It seems when using the Gson API directly, there's a way to set parser to a "strict" mode by turning off leniency, but this option is not exposed when parsing using the Google API client.

2. Malformed JSON exceptions

This is more of a consequence of the above issue. Trying to parse certain malformed JSON strings into objects used to result in an IOException. But now it can result in other runtime exceptions like IllegalArgumentException.

@Test
public void testExceptionType()
    JsonParser parser = Utils.getDefaultJsonFactory().createJsonParser("not json");
    try {
      parser.parseAndClose(Foo.class); 
    } catch (IOException e) {
      e.printStackTrace();
    }
}

public static class Foo {
  @Key("name")
  private String name;
}

3. Broken float serialization

This seems to be due google/gson#1127. Serializing floating point values is not working correctly.

System.out.println(Utils.getDefaultJsonFactory().toString(0.2f));
// Output: 0.20000000298023224

Note that there is again a workaround when using Gson directly. But that's not possible when using the Google API client.

[elharo]

Thanks. These are good bugs. I need to think about where they belong. #3 is I think a GSON bug as you point out. #1 and #2 are possibly bugs in google-http-client, not this product but I'll need to dig deeper.

The Utils class you use here is marked @Beta and in hindsight should probably never have been added to this repo. Its behavior and interface is not guaranteed. Eliminating that from your code base would give you somewhat more control over the choice of JSON parser though go/json-madness still applies as a far as I know.

[elharo]

#2 is definitely an issue in the http-java-client: googleapis/google-http-java-client#1353

[elharo]

#1 looks like a deep issue in GSON. Maybe it can be fixed in http-java-client but I'm not sure about that:

https://stackoverflow.com/questions/43233898/how-to-check-if-json-is-valid-in-java-using-gson/47890960#47890960

[ghost]

I'm not sure if I'm having an issue with the mentioned createJsonParser method because the My Business APIs return a different error model or because the GoogleJsonResponseException class has an issue.
Please let me know where I shall file a new ticket for the following issue:

We are using the google-api-services-mybusinessbusinessinformation Java client library that in turn uses the Google API client.

I've been able to track down our issue down to GoogleJsonResponseException:102
parser = jsonFactory.createJsonParser(response.getContent());

Where evaluating

IOUtils.toString(response.getContent(), "UTF-8")

yields a full response like this one:

{
  "error": {
    "code": 400,
    "message": "Request contains an invalid argument.",
    "status": "INVALID_ARGUMENT",
    "details": [
      {
        "@type": "type.googleapis.com/google.mybusiness.v4.ValidationError",
        "errorDetails": [
          {
            "code": 3,
            "field": "regular_hours.periods.close_time",
            "message": "Time field must follow hh:mm format.",
            "value": "25:00"
          }
        ]
      }
    ]
  }
}

When the GoogleJsonResponseException is then later received in our code, the details block is not part of the error instance.

Update: separate ticket created here: #1974

[eamonnmcmanus]

It will probably be worth revisiting this soon. Bug number 3 (floating-point serialization) has been fixed. The next release of Gson will have a way to ask for strict parsing without having to use the ugly workarounds in the cited StackOverflow discussion, which should address bug number 1. Bug number 2 is just a question of using the right exception handling for the underlying JSON library.