diff --git a/jena-arq/src/main/java/org/apache/jena/riot/rowset/rw/rs_json/RowSetJSONStreaming.java b/jena-arq/src/main/java/org/apache/jena/riot/rowset/rw/rs_json/RowSetJSONStreaming.java index 2a2599137ea..0eeead8606f 100644 --- a/jena-arq/src/main/java/org/apache/jena/riot/rowset/rw/rs_json/RowSetJSONStreaming.java +++ b/jena-arq/src/main/java/org/apache/jena/riot/rowset/rw/rs_json/RowSetJSONStreaming.java @@ -35,9 +35,7 @@ import java.util.function.Function; import java.util.function.Supplier; -import com.google.gson.Gson; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; +import com.google.gson.*; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; @@ -82,7 +80,6 @@ public static RowSetJSONStreaming createUnbuffered(InputStream in, LabelToNod Gson gson = new Gson(); JsonReader reader = gson.newJsonReader(new InputStreamReader(in, StandardCharsets.UTF_8)); - // Set up handling of unexpected json elements UnexpectedJsonEltHandler unexpectedJsonHandler = (_gson, _reader) -> { ErrorHandlers.relay(errorHandler, validationSettings.unexpectedJsonElementSeverity, () -> @@ -94,7 +91,7 @@ public static RowSetJSONStreaming createUnbuffered(InputStream in, LabelToNod // Experiments with an decoder/encoder that returned Bindings directly without // wrapping them as RsJsonEltDft did not show a significant performance difference // that would justify supporting alternative encoder/decoder pairs. - RsJsonEltEncoder eltEncoder = new RsJsonEltEncoderDft(labelMap, null, unexpectedJsonHandler); + RsJsonEltEncoder eltEncoder = new RsJsonEltEncoderDft(labelMap, null, unexpectedJsonHandler, errorHandler); RsJsonEltDecoder eltDecoder = RsJsonEltDecoderDft.INSTANCE; IteratorRsJSON eltIt = new IteratorRsJSON<>(gson, reader, eltEncoder); @@ -335,37 +332,60 @@ public int getUnknownJsonCount() { /* Parsing (gson-based) ------------------------------------------------ */ + private static Type stringListType = new TypeToken>() {}.getType(); /** Parse the vars element from head - may return null */ - static List parseHeadVars(Gson gson, JsonReader reader) throws IOException { + static List parseHeadVars(Gson gson, JsonReader reader, ErrorHandler errorHandler) throws IOException { List result = null; - Type stringListType = new TypeToken>() {}.getType(); - JsonObject headJson = gson.fromJson(reader, JsonObject.class); + JsonObject headJson = fromJsonReader(gson, reader, jsonObjectToken, errorHandler); JsonElement varsJson = headJson.get(kVars); if (varsJson != null) { - List varNames = gson.fromJson(varsJson, stringListType); + // A list ending in "," (bad JSON) cause a java null at the end of the array. + List varNames = fromGsonElement(gson, varsJson, stringListType, errorHandler); + // GSON in lenient mode parses trailing comma and returns a java null. + if ( ! varNames.isEmpty() && varNames.getLast() == null ) { + errorHandler.warning("Bad JSON: \"vars\" array ends in a comma", -1, -1); + //throw new ResultSetException("Bad JSON: Array ends in a comma"); + varNames.removeLast(); + } result = Var.varList(varNames); } return result; } + static TypeToken jsonObjectToken = TypeToken.get(JsonObject.class); static Binding parseBinding( Gson gson, JsonReader reader, LabelToNode labelMap, - Function onUnknownRdfTermType) throws IOException { - JsonObject obj = gson.fromJson(reader, JsonObject.class); - + Function onUnknownRdfTermType, + ErrorHandler errorHandler) throws IOException { + JsonObject obj = fromJsonReader(gson, reader, jsonObjectToken, errorHandler); BindingBuilder bb = BindingFactory.builder(); - for (Entry e : obj.entrySet()) { Var v = Var.alloc(e.getKey()); JsonElement nodeElt = e.getValue(); - Node node = parseOneTerm(nodeElt, labelMap, onUnknownRdfTermType); bb.add(v, node); } - return bb.build(); } + static JsonObject fromJsonReader(Gson gson, JsonReader reader, TypeToken typeOfT, ErrorHandler errorHandler) { + try { + return gson.fromJson(reader, typeOfT); + } catch (JsonParseException ex) { + errorHandler.fatal("JSON Syntax error: "+ex.getMessage(), -1, -1); + return null; + } + } + + static T fromGsonElement(Gson gson, JsonElement jsonElement, Type type, ErrorHandler errorHandler) { + try { + return gson.fromJson(jsonElement, type); + } catch (JsonParseException ex) { + errorHandler.fatal("JSON Syntax error: "+ex.getMessage(), -1, -1); + return null; + } + } + static Node parseOneTerm(JsonElement jsonElt, LabelToNode labelMap, Function onUnknownRdfTermType) { if (jsonElt == null) { @@ -557,19 +577,23 @@ private static class RsJsonEltEncoderDft protected LabelToNode labelMap; protected Function unknownRdfTermTypeHandler; protected UnexpectedJsonEltHandler unexpectedJsonHandler; + protected final ErrorHandler errorHandler; public RsJsonEltEncoderDft(LabelToNode labelMap, Function unknownRdfTermTypeHandler, - UnexpectedJsonEltHandler unexpectedJsonHandler) { + UnexpectedJsonEltHandler unexpectedJsonHandler, + ErrorHandler errorHandler + ) { super(); this.labelMap = labelMap; this.unknownRdfTermTypeHandler = unknownRdfTermTypeHandler; this.unexpectedJsonHandler = unexpectedJsonHandler; + this.errorHandler = errorHandler; } @Override public RsJsonEltDft newHeadElt(Gson gson, JsonReader reader) throws IOException { - List vars = parseHeadVars(gson, reader); + List vars = parseHeadVars(gson, reader, errorHandler); return new RsJsonEltDft(vars); } @@ -581,7 +605,7 @@ public RsJsonEltDft newBooleanElt(Gson gson, JsonReader reader) throws IOExcepti @Override public RsJsonEltDft newBindingElt(Gson gson, JsonReader reader) throws IOException { - Binding binding = parseBinding(gson, reader, labelMap, unknownRdfTermTypeHandler); + Binding binding = parseBinding(gson, reader, labelMap, unknownRdfTermTypeHandler, errorHandler); return new RsJsonEltDft(binding); } @@ -592,7 +616,7 @@ public RsJsonEltDft newResultsElt(Gson gson, JsonReader reader) throws IOExcepti @Override public RsJsonEltDft newUnknownElt(Gson gson, JsonReader reader) throws IOException { - JsonElement jsonElement = unexpectedJsonHandler == null + JsonElement jsonElement = (unexpectedJsonHandler == null) ? null : unexpectedJsonHandler.apply(gson, reader); return new RsJsonEltDft(jsonElement); diff --git a/jena-arq/src/test/java/org/apache/jena/riot/rowset/rw/TestRowSetReader.java b/jena-arq/src/test/java/org/apache/jena/riot/rowset/rw/TestRowSetReader.java index 04606860d72..286fce7e2d5 100644 --- a/jena-arq/src/test/java/org/apache/jena/riot/rowset/rw/TestRowSetReader.java +++ b/jena-arq/src/test/java/org/apache/jena/riot/rowset/rw/TestRowSetReader.java @@ -23,30 +23,38 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import org.junit.jupiter.api.Test; + +import org.apache.commons.lang3.Strings; import org.apache.jena.atlas.io.IOX; import org.apache.jena.atlas.lib.Bytes; +import org.apache.jena.atlas.logging.LogCtl; import org.apache.jena.query.ARQ; import org.apache.jena.riot.Lang; import org.apache.jena.riot.resultset.ResultSetLang; import org.apache.jena.riot.rowset.RowSetReader; import org.apache.jena.sparql.exec.RowSet; import org.apache.jena.sparql.exec.RowSetOps; +import org.apache.jena.sparql.resultset.ResultSetException; import org.apache.jena.sys.JenaSystem; -import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Addition tests for result readers. - * The SPARQl test suite cover most usage. This class adds tests + * The SPARQL test suite cover most usage. This class adds tests. */ public class TestRowSetReader { static { JenaSystem.init(); } - // Check "abc"^^rdf:langString and "abc"^^rdf:dirLangString and " + // Check "abc"^^rdf:langString and "abc"^^rdf:dirLangString - i.e. incomplete forms. @Test public void resultSet_json_1() { String r = """ { @@ -71,6 +79,49 @@ public class TestRowSetReader { assertEquals(2, RowSetOps.count(rowset)); } + // Check error handling of bad JSON + @Test public void resultSet_json_bad_vars() { + // Trailing comma in "vars" array. This is only a warning. + String r = """ + { "head": { "vars": [ "x", "y" , ] } , + "results": { + "bindings": [ + { + "x": { "type": "literal" , "value": "A" } , + "y": { "type": "literal" , "value": "B" } + } + ] + } + } + """; + Logger log = LoggerFactory.getLogger(RowSetReaderJSONStreaming.class); + LogCtl.withLevel(log, "ERROR", ()->{ + RowSet rowset = read(r, ResultSetLang.RS_JSON); + assertNotNull(rowset); + assertEquals(1, RowSetOps.count(rowset)); + }); + } + + // Check error handling of bad JSON + @Test public void resultSet_json_bad_bindings() { + // Trailing comma in "bindings" array. This is an error. It might be file truncation. + String r = """ + { "head": { "vars": [ "x", "y" ] } , + "results": { + "bindings": [ + { + "x": { "type": "literal" , "value": "A" } , + "y": { "type": "literal" , "value": "B" } , + } + ] + } + } + """; + ResultSetException ex = assertThrows(ResultSetException.class, ()-> read(r, ResultSetLang.RS_JSON)); + String msg = ex.getMessage(); + assertTrue(Strings.CI.containsAny(msg, "JSON Syntax error:")); + } + @Test public void resultSet_xml_1() { String r = """ diff --git a/jena-cmds/src/main/java/arq/rset.java b/jena-cmds/src/main/java/arq/rset.java index 3efd228ed16..780e2ea73bd 100644 --- a/jena-cmds/src/main/java/arq/rset.java +++ b/jena-cmds/src/main/java/arq/rset.java @@ -22,6 +22,7 @@ package arq; import org.apache.jena.query.ResultSet; +import org.apache.jena.sparql.resultset.ResultSetException; import org.apache.jena.sparql.util.QueryExecUtils; import arq.cmdline.CmdARQ; import arq.cmdline.ModResultsIn; @@ -57,8 +58,13 @@ protected String getSummary() { @Override protected void exec() { - ResultSet rs = modInput.getResultSet(); - QueryExecUtils.outputResultSet(rs, null, modOutput.getResultsFormat(), System.out); + try { + ResultSet rs = modInput.getResultSet(); + QueryExecUtils.outputResultSet(rs, null, modOutput.getResultsFormat(), System.out); + } catch (ResultSetException ex) { + System.err.println(ex.getMessage()); + } + } @Override