Index: trunk/src/org/openstreetmap/josm/io/OsmJsonReader.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmJsonReader.java (revision 18815) +++ trunk/src/org/openstreetmap/josm/io/OsmJsonReader.java (revision 18816) @@ -4,6 +4,6 @@ import static org.openstreetmap.josm.tools.I18n.tr; +import java.io.IOException; import java.io.InputStream; -import java.net.SocketException; import java.util.Collection; import java.util.Map.Entry; @@ -33,5 +33,5 @@ /** * Parser for the Osm API (JSON output). Read from an input stream and construct a dataset out of it. - * + *

* For each json element, there is a dedicated method. * @since 14086 @@ -190,6 +190,6 @@ throw new IllegalDataException(exception); } catch (JsonException exception) { - if (exception.getCause() instanceof SocketException) { - SocketException soe = (SocketException) exception.getCause(); + if (exception.getCause() instanceof IOException) { + IOException soe = (IOException) exception.getCause(); soe.addSuppressed(exception); // Add the caught exception as a suppressed exception throw new IllegalDataException(soe); // NOPMD -- PreserveStackTrace should be fixed with PMD 7 Index: trunk/test/unit/org/openstreetmap/josm/io/OsmJsonReaderTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/io/OsmJsonReaderTest.java (revision 18815) +++ trunk/test/unit/org/openstreetmap/josm/io/OsmJsonReaderTest.java (revision 18816) @@ -15,9 +15,13 @@ import java.time.Instant; import java.util.Iterator; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; +import java.util.stream.Stream; import jakarta.json.JsonException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; @@ -29,4 +33,5 @@ import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.tools.ExceptionUtil; +import org.openstreetmap.josm.tools.JosmRuntimeException; /** @@ -255,4 +260,22 @@ } + static Stream testException() { + final byte[] smallJson = "{\"type\", \"node\", \"id\": 1, \"lat\": 1.0, \"lon\": 2.0}".getBytes(StandardCharsets.UTF_8); + return Stream.of( + // Check that a SocketException is properly reported + Arguments.of(IllegalDataException.class, new ThrowableInputStream<>(() -> new SocketException("Read timed out"), smallJson), + "java.net.SocketException: Read timed out"), + // Check that a random JsonException is reported + Arguments.of(JsonException.class, new ThrowableInputStream<>(() -> new JsonException("Some random json exception"), smallJson), + "Some random json exception"), + // Check that bad data still throws an IllegalDataException + Arguments.of(IllegalDataException.class, new ThrowableInputStream<>(() -> null, smallJson), + "jakarta.json.stream.JsonParsingException: Invalid token=COMMA at (line no=1, column no=8, offset=7). " + + "Expected tokens are: [COLON]"), + // Check that an IOException is properly thrown when the stream is closed + Arguments.of(IllegalDataException.class, new ThrowableInputStream<>(() -> new JsonException("I/O error while parsing JSON", + new IOException("stream is closed")), smallJson), "java.io.IOException: stream is closed") + ); + } /** @@ -266,42 +289,51 @@ * */ - @SuppressWarnings("resource") - @Test - void testException() { - final ByteArrayInputStream bais = - new ByteArrayInputStream("{\"type\", \"node\", \"id\": 1, \"lat\": 1.0, \"lon\": 2.0}".getBytes(StandardCharsets.UTF_8)); - final AtomicBoolean throwJson = new AtomicBoolean(); - final InputStream socketExceptionStream = new InputStream() { - int read = 0; // Necessary, since otherwise the exception might not be wrapped in a Json exception - @Override - public int read() throws IOException { - try { - if (read > 0 && !throwJson.get()) { - throw new SocketException("Read timed out"); - } else if (read > 0 && throwJson.get()) { - throw new JsonException("Some random json exception"); + @ParameterizedTest + @MethodSource + void testException(Class exceptionClass, ThrowableInputStream exceptionStream, String exceptionMessage) { + E exception = assertThrows(exceptionClass, () -> OsmJsonReader.parseDataSet(exceptionStream, NullProgressMonitor.INSTANCE)); + assertEquals(exceptionMessage, ExceptionUtil.explainException(exception)); + assertDoesNotThrow(exceptionStream::close); + } + + /** + * An {@link InputStream} that will throw a specified exception after the first byte is read + * @param The exception to be thrown + */ + private static class ThrowableInputStream extends InputStream { + private final Supplier exceptionSupplier; + private final ByteArrayInputStream bais; + private int read = 0; // Necessary, since otherwise the exception might not be wrapped in a Json exception + + ThrowableInputStream(Supplier exceptionSupplier, byte[] source) { + this.exceptionSupplier = exceptionSupplier; + this.bais = new ByteArrayInputStream(source); + } + + @Override + public int read() throws IOException { + try { + if (read > 0) { + E exception = this.exceptionSupplier.get(); + if (exception instanceof IOException) { + throw (IOException) exception; + } else if (exception instanceof RuntimeException) { + throw (RuntimeException) exception; + } else if (exception != null) { + // This shouldn't be possible in actual code, so if something hits this, it is a failure. + throw new JosmRuntimeException(exception); } - return bais.read(); - } finally { - read++; } + return bais.read(); + } finally { + read++; } - }; - // Check that a SocketException is properly reported - IllegalDataException ide = assertThrows(IllegalDataException.class, - () -> OsmJsonReader.parseDataSet(socketExceptionStream, NullProgressMonitor.INSTANCE)); - assertEquals("java.net.SocketException: Read timed out", ExceptionUtil.explainException(ide)); - assertDoesNotThrow(socketExceptionStream::close); - bais.reset(); - // Check that a generic exception is properly thrown -- we only want to handle known "good" cases specially - throwJson.set(true); - assertThrows(JsonException.class, () -> OsmJsonReader.parseDataSet(socketExceptionStream, NullProgressMonitor.INSTANCE)); - bais.reset(); - // Check that a generic parsing error is properly reported - ide = assertThrows(IllegalDataException.class, () -> OsmJsonReader.parseDataSet(bais, NullProgressMonitor.INSTANCE)); - assertEquals("jakarta.json.stream.JsonParsingException: Invalid token=COMMA at (line no=1, column no=8, offset=7). " + - "Expected tokens are: [COLON]", ExceptionUtil.explainException(ide)); - bais.reset(); - // Check that an unknown exception is thrown properly + } + + @Override + public void close() throws IOException { + this.bais.close(); + super.close(); + } } }