Skip to content

Commit d90f502

Browse files
OstrzycielOstrzyciel
authored
core: improve error handling in decoder & transcoder (#339)
Instead of throwing random NPEs and arrayoutofbounds, throw something nicer. This covers the some of the most common types of exceptions, should be enough. This should address the complaints made here: - Jelly-RDF/cli#81 (comment) (will require also changes in cli, but I will do that in a moment) - #338
1 parent 368ce01 commit d90f502

6 files changed

Lines changed: 98 additions & 39 deletions

File tree

‎core/src/main/java/eu/ostrzyciel/jelly/core/internal/NameDecoderImpl.java‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public NameDecoderImpl(int prefixTableSize, int nameTableSize, Function<String,
6262
/**
6363
* Update the name table with a new entry.
6464
* @param nameEntry name row
65-
* @throws ArrayIndexOutOfBoundsException if the identifier is out of bounds
65+
* @throws RdfProtoDeserializationError if the identifier is out of bounds
6666
*/
6767
@Override
6868
public void updateNames(RdfNameEntry nameEntry) {
@@ -72,33 +72,44 @@ public void updateNames(RdfNameEntry nameEntry) {
7272
// else lastNameIdSet = id;
7373
// Same code is used in the methods below.
7474
lastNameIdSet = ((lastNameIdSet + 1) & ((id - 1) >> 31)) + id;
75-
NameLookupEntry entry = nameLookup[lastNameIdSet];
76-
entry.name = nameEntry.value();
77-
// Enough to invalidate the last IRI – we don't have to touch the serial number.
78-
entry.lastPrefixId = 0;
79-
// Set to null is required to avoid a false positive in the decode method for cases without a prefix.
80-
entry.lastIri = null;
75+
try {
76+
NameLookupEntry entry = nameLookup[lastNameIdSet];
77+
entry.name = nameEntry.value();
78+
// Enough to invalidate the last IRI – we don't have to touch the serial number.
79+
entry.lastPrefixId = 0;
80+
// Set to null is required to avoid a false positive in the decode method for cases without a prefix.
81+
entry.lastIri = null;
82+
} catch (ArrayIndexOutOfBoundsException | NullPointerException e) {
83+
throw JellyExceptions.rdfProtoDeserializationError(
84+
"Name entry with ID %d is out of bounds of the name lookup table.".formatted(id)
85+
);
86+
}
8187
}
8288

8389
/**
8490
* Update the prefix table with a new entry.
8591
* @param prefixEntry prefix row
86-
* @throws ArrayIndexOutOfBoundsException if the identifier is out of bounds
92+
* @throws RdfProtoDeserializationError if the identifier is out of bounds
8793
*/
8894
@Override
8995
public void updatePrefixes(RdfPrefixEntry prefixEntry) {
9096
int id = prefixEntry.id();
9197
lastPrefixIdSet = ((lastPrefixIdSet + 1) & ((id - 1) >> 31)) + id;
92-
PrefixLookupEntry entry = prefixLookup[lastPrefixIdSet];
93-
entry.prefix = prefixEntry.value();
94-
entry.serial++;
98+
try {
99+
PrefixLookupEntry entry = prefixLookup[lastPrefixIdSet];
100+
entry.prefix = prefixEntry.value();
101+
entry.serial++;
102+
} catch (ArrayIndexOutOfBoundsException | NullPointerException e) {
103+
throw JellyExceptions.rdfProtoDeserializationError(
104+
"Prefix entry with ID %d is out of bounds of the prefix lookup table.".formatted(id)
105+
);
106+
}
95107
}
96108

97109
/**
98110
* Reconstruct an IRI from its prefix and name ids.
99111
* @param iri IRI row from the Jelly proto
100112
* @return full IRI combining the prefix and the name
101-
* @throws ArrayIndexOutOfBoundsException if IRI had indices out of lookup table bounds
102113
* @throws RdfProtoDeserializationError if the IRI reference is invalid
103114
* @throws NullPointerException if the IRI reference is invalid
104115
*/
@@ -107,8 +118,15 @@ public void updatePrefixes(RdfPrefixEntry prefixEntry) {
107118
public TIri decode(RdfIri iri) {
108119
int nameId = iri.nameId();
109120
lastNameIdReference = ((lastNameIdReference + 1) & ((nameId - 1) >> 31)) + nameId;
110-
NameLookupEntry nameEntry = nameLookup[lastNameIdReference];
111-
121+
NameLookupEntry nameEntry;
122+
try {
123+
nameEntry = nameLookup[lastNameIdReference];
124+
} catch (ArrayIndexOutOfBoundsException e) {
125+
throw JellyExceptions.rdfProtoDeserializationError(
126+
("Encountered an invalid name table reference (out of bounds). " +
127+
"Name ID: %d, Prefix ID: %d").formatted(nameId, iri.prefixId())
128+
);
129+
}
112130
int prefixId = iri.prefixId();
113131
// Branchless way to update the prefix ID
114132
// Equivalent to:
@@ -117,7 +135,15 @@ public TIri decode(RdfIri iri) {
117135
lastPrefixIdReference = prefixId = (((prefixId - 1) >> 31) & lastPrefixIdReference) + prefixId;
118136
if (prefixId != 0) {
119137
// Name and prefix
120-
PrefixLookupEntry prefixEntry = prefixLookup[prefixId];
138+
PrefixLookupEntry prefixEntry;
139+
try {
140+
prefixEntry = prefixLookup[prefixId];
141+
} catch (ArrayIndexOutOfBoundsException e) {
142+
throw JellyExceptions.rdfProtoDeserializationError(
143+
("Encountered an invalid prefix table reference (out of bounds). " +
144+
"Prefix ID: %d, Name ID: %d").formatted(prefixId, nameId)
145+
);
146+
}
121147
if (nameEntry.lastPrefixId != prefixId || nameEntry.lastPrefixSerial != prefixEntry.serial) {
122148
// Update the last prefix
123149
nameEntry.lastPrefixId = prefixId;
@@ -128,13 +154,13 @@ public TIri decode(RdfIri iri) {
128154
}
129155
if (nameEntry.lastIri == null) {
130156
throw JellyExceptions.rdfProtoDeserializationError(
131-
"Encountered an invalid IRI reference. " + "Prefix ID: " + iri.prefixId() + ", Name ID: " + nameId
157+
"Encountered an invalid IRI reference. Prefix ID: %d, Name ID: %d".formatted(iri.prefixId(), nameId)
132158
);
133159
}
134160
} else if (nameEntry.lastIri == null) {
135161
if (nameEntry.name == null) {
136162
throw JellyExceptions.rdfProtoDeserializationError(
137-
"Encountered an invalid IRI reference. " + "No prefix, Name ID: " + nameId
163+
"Encountered an invalid IRI reference. No prefix, Name ID: %d".formatted(nameId)
138164
);
139165
}
140166
// Name only, no need to check the prefix lookup

‎core/src/main/java/eu/ostrzyciel/jelly/core/internal/TranscoderLookup.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package eu.ostrzyciel.jelly.core.internal;
22

3+
import eu.ostrzyciel.jelly.core.JellyExceptions;
34
import java.util.Arrays;
45

56
/**
@@ -101,7 +102,9 @@ int remap(int id) {
101102
*/
102103
void newInputStream(int size) {
103104
if (size > outputSize) {
104-
throw new IllegalArgumentException("Input lookup size cannot be greater than the output lookup size");
105+
throw JellyExceptions.rdfProtoTranscodingError(
106+
"Input lookup size cannot be greater than the output lookup size"
107+
);
105108
}
106109
if (table != null) {
107110
// Only set this for streams 2 and above (counting from 1)

‎core/src/main/scala/eu/ostrzyciel/jelly/core/JellyExceptions.scala‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,12 @@ private object JellyExceptions extends JellyExceptions:
3030
private[core] def rdfProtoSerializationError(msg: String): RdfProtoSerializationError =
3131
new RdfProtoSerializationError(msg)
3232

33+
/**
34+
* Helper method to allow Java code to throw a [[RdfProtoTranscodingError]].
35+
* @param msg error message
36+
* @return an instance of [[RdfProtoTranscodingError]]
37+
*/
38+
private[core] def rdfProtoTranscodingError(msg: String): RdfProtoTranscodingError =
39+
new RdfProtoTranscodingError(msg)
40+
3341
export JellyExceptions.*

‎core/src/test/scala/eu/ostrzyciel/jelly/core/ProtoDecoderSpec.scala‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -272,22 +272,26 @@ class ProtoDecoderSpec extends AnyWordSpec, Matchers:
272272

273273
// The tests for this logic are in internal.NameDecoderSpec
274274
// Here we are just testing if the exceptions are rethrown correctly.
275-
"throw exception on out-of-bounds references to lookups" in {
275+
"throw exception on an invalid IRI term" in {
276276
val decoder = MockConverterFactory.triplesDecoder(None)
277277
val data = wrapEncodedFull(Seq(
278278
JellyOptions.smallGeneralized.withPhysicalType(PhysicalStreamType.TRIPLES),
279+
RdfPrefixEntry(0, null),
280+
RdfNameEntry(0, null),
279281
RdfTriple(
280282
RdfTerm.Bnode("1"),
281283
RdfTerm.Bnode("2"),
282-
RdfIri(10000, 0),
284+
RdfIri(1, 1),
283285
),
284286
))
285287
decoder.ingestRow(data.head)
288+
decoder.ingestRow(data(1))
289+
decoder.ingestRow(data(2))
286290
val error = intercept[RdfProtoDeserializationError] {
287-
decoder.ingestRow(data(1))
291+
decoder.ingestRow(data(3))
288292
}
289293
error.getMessage should include ("Error while decoding term")
290-
error.getCause shouldBe a [ArrayIndexOutOfBoundsException]
294+
error.getCause shouldBe a [NullPointerException]
291295
}
292296
}
293297

@@ -461,20 +465,28 @@ class ProtoDecoderSpec extends AnyWordSpec, Matchers:
461465

462466
// The tests for this logic are in internal.NameDecoderSpec
463467
// Here we are just testing if the exceptions are rethrown correctly.
464-
"throw exception on out-of-bounds references to lookups (graph term)" in {
465-
val decoder = MockConverterFactory.graphsAsQuadsDecoder(None)
468+
"throw exception on an invalid IRI term" in {
469+
val decoder = MockConverterFactory.graphsAsQuadsDecoder()
466470
val data = wrapEncodedFull(Seq(
467471
JellyOptions.smallGeneralized.withPhysicalType(PhysicalStreamType.GRAPHS),
468-
RdfGraphStart(
469-
RdfIri(10000, 0),
472+
RdfPrefixEntry(0, null),
473+
RdfNameEntry(0, null),
474+
RdfGraphStart(RdfDefaultGraph.defaultInstance),
475+
RdfTriple(
476+
RdfTerm.Bnode("1"),
477+
RdfTerm.Bnode("2"),
478+
RdfIri(1, 1),
470479
),
471480
))
472481
decoder.ingestRow(data.head)
482+
decoder.ingestRow(data(1))
483+
decoder.ingestRow(data(2))
484+
decoder.ingestRow(data(3))
473485
val error = intercept[RdfProtoDeserializationError] {
474-
decoder.ingestRow(data(1))
486+
decoder.ingestRow(data(4))
475487
}
476-
error.getMessage should include ("Error while decoding graph term")
477-
error.getCause shouldBe a [ArrayIndexOutOfBoundsException]
488+
error.getMessage should include("Error while decoding term")
489+
error.getCause shouldBe a[NullPointerException]
478490
}
479491
}
480492

‎core/src/test/scala/eu/ostrzyciel/jelly/core/internal/NameDecoderSpec.scala‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,66 +103,75 @@ class NameDecoderSpec extends AnyWordSpec, Matchers:
103103

104104
"not accept a new prefix ID larger than table size" in {
105105
val dec = makeDecoder(smallOptions)
106-
intercept[ArrayIndexOutOfBoundsException] {
106+
val e = intercept[RdfProtoDeserializationError] {
107107
dec.updatePrefixes(RdfPrefixEntry(9, "https://test.org/"))
108108
}
109+
e.getMessage should include ("Prefix entry with ID 9")
109110
}
110111

111112
"not accept a new prefix ID lower than 0 (-1)" in {
112113
val dec = makeDecoder(smallOptions)
113-
intercept[NullPointerException] {
114+
val e = intercept[RdfProtoDeserializationError] {
114115
dec.updatePrefixes(RdfPrefixEntry(-1, "https://test.org/"))
115116
}
117+
e.getMessage should include ("Prefix entry with ID -1")
116118
}
117119

118120
"not accept a new prefix ID lower than 0 (-2)" in {
119121
val dec = makeDecoder(smallOptions)
120-
intercept[ArrayIndexOutOfBoundsException] {
122+
val e = intercept[RdfProtoDeserializationError] {
121123
dec.updatePrefixes(RdfPrefixEntry(-2, "https://test.org/"))
122124
}
125+
e.getMessage should include ("Prefix entry with ID -2")
123126
}
124127

125128
"not retrieve a prefix ID larger than table size" in {
126129
val dec = makeDecoder(smallOptions)
127-
intercept[ArrayIndexOutOfBoundsException] {
130+
val e = intercept[RdfProtoDeserializationError] {
128131
dec.decode(RdfIri(9, 0))
129132
}
133+
e.getMessage should include ("Prefix ID: 9")
130134
}
131135

132136
"not accept a new name ID larger than table size" in {
133137
val dec = makeDecoder(smallOptions)
134-
intercept[ArrayIndexOutOfBoundsException] {
138+
val e = intercept[RdfProtoDeserializationError] {
135139
dec.updateNames(RdfNameEntry(17, "Cake"))
136140
}
141+
e.getMessage should include ("Name entry with ID 17")
137142
}
138143

139144
"not accept a default ID going beyond the table size" in {
140145
val dec = makeDecoder(smallOptions)
141146
dec.updateNames(RdfNameEntry(16, "Cake"))
142-
intercept[ArrayIndexOutOfBoundsException] {
147+
val e = intercept[RdfProtoDeserializationError] {
143148
dec.updateNames(RdfNameEntry(0, "Cake 2"))
144149
}
150+
e.getMessage should include ("Name entry with ID 0")
145151
}
146152

147153
"not accept a new name ID lower than 0 (-1)" in {
148154
val dec = makeDecoder(smallOptions)
149-
intercept[NullPointerException] {
155+
val e = intercept[RdfProtoDeserializationError] {
150156
dec.updateNames(RdfNameEntry(-1, "Cake"))
151157
}
158+
e.getMessage should include ("Name entry with ID -1")
152159
}
153160

154161
"not accept a new name ID lower than 0 (-2)" in {
155162
val dec = makeDecoder(smallOptions)
156-
intercept[ArrayIndexOutOfBoundsException] {
163+
val e = intercept[RdfProtoDeserializationError] {
157164
dec.updateNames(RdfNameEntry(-2, "Cake"))
158165
}
166+
e.getMessage should include ("Name entry with ID -2")
159167
}
160168

161169
"not retrieve a name ID larger than table size" in {
162170
val dec = makeDecoder(smallOptions)
163-
intercept[ArrayIndexOutOfBoundsException] {
171+
val e = intercept[RdfProtoDeserializationError] {
164172
dec.decode(RdfIri(0, 17))
165173
}
174+
e.getMessage should include ("Name ID: 17")
166175
}
167176
}
168177
}

‎core/src/test/scala/eu/ostrzyciel/jelly/core/internal/TranscoderLookupSpec.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package eu.ostrzyciel.jelly.core.internal
22

3+
import eu.ostrzyciel.jelly.core.JellyExceptions.RdfProtoTranscodingError
34
import org.scalatest.matchers.should.Matchers
45
import org.scalatest.wordspec.AnyWordSpec
56

@@ -11,7 +12,7 @@ class TranscoderLookupSpec extends AnyWordSpec, Matchers:
1112
"TranscoderLookup" should {
1213
"throw an exception when trying to set input lookup size greater than the output" in {
1314
val tl = TranscoderLookup(false, 100)
14-
val ex = intercept[IllegalArgumentException] {
15+
val ex = intercept[RdfProtoTranscodingError] {
1516
tl.newInputStream(120)
1617
}
1718
ex.getMessage should include ("Input lookup size cannot be greater than the output lookup size")

0 commit comments

Comments
 (0)