Skip to content

Commit

Permalink
Remove EOFException special casing of JsonStreamParser.next() (#2281)
Browse files Browse the repository at this point in the history
* Remove EOFException special casing of JsonStreamParser.next()

The previous behavior violated the Iterator contract where for
`JsonStreamParser("[")` a call to `hasNext()` would return true,
but `next()` would throw a NoSuchElementException.

* Fix incorrect documented thrown exception type for JsonStreamParser
  • Loading branch information
Marcono1234 authored Dec 14, 2022
1 parent 6c3cf22 commit f63a1b8
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 22 deletions.
22 changes: 9 additions & 13 deletions gson/src/main/java/com/google/gson/JsonStreamParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,16 @@
*/
package com.google.gson;

import java.io.EOFException;
import com.google.gson.internal.Streams;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.MalformedJsonException;
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.util.Iterator;
import java.util.NoSuchElementException;

import com.google.gson.internal.Streams;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.MalformedJsonException;

/**
* A streaming parser that allows reading of multiple {@link JsonElement}s from the specified reader
* asynchronously. The JSON data is parsed in lenient mode, see also
Expand Down Expand Up @@ -61,7 +59,7 @@ public final class JsonStreamParser implements Iterator<JsonElement> {
public JsonStreamParser(String json) {
this(new StringReader(json));
}

/**
* @param reader The data stream containing JSON elements concatenated to each other.
* @since 1.4
Expand All @@ -71,13 +69,13 @@ public JsonStreamParser(Reader reader) {
parser.setLenient(true);
lock = new Object();
}

/**
* Returns the next available {@link JsonElement} on the reader. Throws a
* {@link NoSuchElementException} if no element is available.
*
* @return the next available {@code JsonElement} on the reader.
* @throws JsonSyntaxException if the incoming stream is malformed JSON.
* @throws JsonParseException if the incoming stream is malformed JSON.
* @throws NoSuchElementException if no {@code JsonElement} is available.
* @since 1.4
*/
Expand All @@ -86,22 +84,20 @@ public JsonElement next() throws JsonParseException {
if (!hasNext()) {
throw new NoSuchElementException();
}

try {
return Streams.parse(parser);
} catch (StackOverflowError e) {
throw new JsonParseException("Failed parsing JSON source to Json", e);
} catch (OutOfMemoryError e) {
throw new JsonParseException("Failed parsing JSON source to Json", e);
} catch (JsonParseException e) {
throw e.getCause() instanceof EOFException ? new NoSuchElementException() : e;
}
}

/**
* Returns true if a {@link JsonElement} is available on the input for consumption
* @return true if a {@link JsonElement} is available on the input, false otherwise
* @throws JsonSyntaxException if the incoming stream is malformed JSON.
* @throws JsonParseException if the incoming stream is malformed JSON.
* @since 1.4
*/
@Override
Expand Down
74 changes: 65 additions & 9 deletions gson/src/test/java/com/google/gson/JsonStreamParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,38 @@
*/
package com.google.gson;

import junit.framework.TestCase;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.EOFException;
import java.util.NoSuchElementException;
import org.junit.Before;
import org.junit.Test;

/**
* Unit tests for {@link JsonStreamParser}
*
*
* @author Inderjeet Singh
*/
public class JsonStreamParserTest extends TestCase {
public class JsonStreamParserTest {
private JsonStreamParser parser;

@Override
protected void setUp() throws Exception {
super.setUp();

@Before
public void setUp() throws Exception {
parser = new JsonStreamParser("'one' 'two'");
}

@Test
public void testParseTwoStrings() {
String actualOne = parser.next().getAsString();
assertEquals("one", actualOne);
String actualTwo = parser.next().getAsString();
assertEquals("two", actualTwo);
}

@Test
public void testIterator() {
assertTrue(parser.hasNext());
assertEquals("one", parser.next().getAsString());
Expand All @@ -48,20 +55,22 @@ public void testIterator() {
assertFalse(parser.hasNext());
}

@Test
public void testNoSideEffectForHasNext() throws Exception {
assertTrue(parser.hasNext());
assertTrue(parser.hasNext());
assertTrue(parser.hasNext());
assertEquals("one", parser.next().getAsString());

assertTrue(parser.hasNext());
assertTrue(parser.hasNext());
assertEquals("two", parser.next().getAsString());

assertFalse(parser.hasNext());
assertFalse(parser.hasNext());
}

@Test
public void testCallingNextBeyondAvailableInput() {
parser.next();
parser.next();
Expand All @@ -71,4 +80,51 @@ public void testCallingNextBeyondAvailableInput() {
} catch (NoSuchElementException expected) {
}
}

@Test
public void testEmptyInput() {
JsonStreamParser parser = new JsonStreamParser("");
try {
parser.next();
fail();
} catch (JsonIOException e) {
assertTrue(e.getCause() instanceof EOFException);
}

parser = new JsonStreamParser("");
try {
parser.hasNext();
fail();
} catch (JsonIOException e) {
assertTrue(e.getCause() instanceof EOFException);
}
}

@Test
public void testIncompleteInput() {
JsonStreamParser parser = new JsonStreamParser("[");
assertTrue(parser.hasNext());
try {
parser.next();
fail();
} catch (JsonSyntaxException e) {
}
}

@Test
public void testMalformedInput() {
JsonStreamParser parser = new JsonStreamParser(":");
try {
parser.hasNext();
fail();
} catch (JsonSyntaxException e) {
}

parser = new JsonStreamParser(":");
try {
parser.next();
fail();
} catch (JsonSyntaxException e) {
}
}
}

0 comments on commit f63a1b8

Please sign in to comment.