Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method fromJson throws JsonParseException instead of declared JsonSyntaxException #2816

Open
DGorokhov123 opened this issue Feb 24, 2025 · 2 comments · May be fixed by #2820
Open

Method fromJson throws JsonParseException instead of declared JsonSyntaxException #2816

DGorokhov123 opened this issue Feb 24, 2025 · 2 comments · May be fixed by #2820
Labels

Comments

@DGorokhov123
Copy link

DGorokhov123 commented Feb 24, 2025

Gson version

I found a bug in GSON library. It's actual fo version 2.12.1

Description

Bug description:

Method fromJson signature looks like that:

public <T> T fromJson(String json, Class<T> classOfT) throws JsonSyntaxException {

so, it's obvious to coders to catch JsonSyntaxException:

try {
    goodGson.fromJson(badJson, User.class);
} catch (JsonSyntaxException e) {
    System.out.println("Good deserializer caught JsonSyntaxException");
}

Such construction works until we add Deserializer:

Gson badGson = new GsonBuilder()
          .registerTypeAdapter(User.class, new UserBadDeserializer())
          .create();

class UserBadDeserializer implements JsonDeserializer<User> {
    @Override
    public User deserialize(JsonElement json, Type typeOfT, 
                         JsonDeserializationContext context) throws JsonParseException {
        JsonObject jsonObject = json.getAsJsonObject();
        if (!jsonObject.has("name")) 
                     throw new JsonParseException("json should contain the name field!");
        String name = jsonObject.get("name").getAsString();
        return new User(name);
    }
}

The signature of deserialize method we should override, throws another
exception - JsonParseException, which is parent to JsonSyntaxException
and isn't caught by try-catch construction above
It flies further and may cause unexpected behaviour.

        try {
            try {
                badGson.fromJson(badJson, User.class);
            } catch (JsonSyntaxException e) {
                System.out.println("Good deserializer caught JsonSyntaxException");
            }
        } catch (RuntimeException e) {
            System.out.println("Bad deserializer didn't catch JsonSyntaxException, and " 
                        + e.getClass().getSimpleName() + " flew further");
        }

Solution 1 : Change signature of deserialize method to throw JsonSyntaxException.

Solution 2 : Change signature of fromJson method (and other methods calling
deserialize) to throw JsonParseException.

I've made a simple example to show this bug:
https://github.com/DGorokhov123/gson-bug

(С) Dmitriy Gorokhov [email protected], @DGorokhov123

@Shershah03
Copy link

Shershah03 commented Mar 5, 2025

Its seems changing it T _com.google.gson.JsonDeserializer.deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonSyntaxException is the best option.

@Marcono1234
Copy link
Collaborator

Thanks for raising this issue!

This is a bit related to #2359 (and other issues as well); Gson is currently not that consistent regarding which exception is thrown when.

The description of JsonSyntaxException sounds (at least to me) like it is mainly intended for actual malformed JSON. But there can be other cases where it necessary to throw an exception, for example an unexpected JSON null or a missing property. In those cases it might be more appropriate to throw a JsonParseException?
There are also cases where Gson itself is throwing a JsonParseException, see ToNumberPolicy or ReflectiveTypeAdapterFactory.

So maybe rather the Gson#fromJson methods should mention that JsonParseException can be thrown.

(This is just my personal opinion on this though. I don't know the original intentions for JsonParseException and JsonSyntaxException.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants