Skip to content

GH-4338: Switch from local JSON parsers to GSON - #4339

Merged
afs merged 1 commit into
apache:mainfrom
afs:json-by-gson
Oct 22, 2026
Merged

afs merged 1 commit into
apache:mainfrom
afs:json-by-gson

Conversation

@afs

@afs afs commented Oct 19, 2026

Copy link
Copy Markdown
Member

GitHub issue resolved #4338


  • Tests are included.
  • Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

}
catch (EOFException ex) {
throw parseException("Empty input", jsonReader.toString());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jsonReader.peek() calls are outside gson.fromJson, so a syntax error they hit is not wrapped: gson throws MalformedJsonException (an IOException) and this branch turns it into RuntimeIOException. That happens for a bad first token (}) or bad trailing content ({"a":1} }, {"a":1},). Callers that catch JsonParseException miss it: riotcmd/json dies with a stack trace, and StoreParamsCodec.read (TDB1/TDB2) no longer ignores a broken tdb.cfg.

+import com.google.gson.stream.MalformedJsonException;
 ...
         catch (EOFException ex) {
             throw parseException("Empty input", jsonReader.toString());
         }
+        catch (MalformedJsonException ex) {
+            throw parseException(null, ex.getMessage());
+        }
         catch (IOException ex) {
             IO.exception(ex);
             return null;
         }

and a test in TestJson:

    @Test
    public void js_value_14() {
        assertThrows(JsonParseException.class, ()->JSON.parse("}"));
        assertThrows(JsonParseException.class, ()->JSON.parse("{\"a\":1} }"));
        assertThrows(JsonParseException.class, ()->JSON.parse("{\"a\":1},"));
    }

@OyvindLGjesdal OyvindLGjesdal Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further findings in https://gist.github.com/OyvindLGjesdal/51fc834b359dbcba1265f9f4218f5480

I wanted to comment inline since it is kind of a verbose wall of text, but I thought the content of the review looks relevant and could be useful.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issues addressed.

  • Removed the JSON grammar.
  • Convert exceptions to Jena's JsonParseException

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attached is copy of the findings report. jena-json-gson.md

My ideal would be strict JSON + allow single quote delimiters + comments, but that set alone isn't an option.

The default strictness for new Gson() is null that becomes Strictness.LENIENT for reading.

@afs
afs merged commit 004459b into apache:main Oct 22, 2026
1 check passed
@afs
afs deleted the json-by-gson branch October 22, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retire custom JSON parsers

3 participants