Skip to content

Commit 82c7465

Browse files
committed
[java] Enforce commas between JSON elements; accept trailing commas
hasNext() previously consumed a leading comma if present, but never required one. That accepted spec-invalid inputs like '[1 2 3]' (missing separator) and '[,1]' (leading comma). It also rejected trailing commas like '[1,2,3,]' because it consumed the comma and then tried to read another element. Track whether the current container has seen an element (parallel deque kept in sync with the container stack, marked in expect()) and use it in hasNext() to require a comma between elements while allowing a single trailing comma before the container's closer.
1 parent 3e8e318 commit 82c7465

2 files changed

Lines changed: 86 additions & 3 deletions

File tree

java/src/org/openqa/selenium/json/JsonInput.java

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ public class JsonInput implements Closeable {
4949
// Used when reading maps and collections so that we handle de-nesting and
5050
// figuring out whether we're expecting a NAME properly.
5151
private final Deque<Container> stack = new ArrayDeque<>();
52+
// Parallel stack tracking whether the current container has seen at least
53+
// one element. Used by hasNext() to enforce comma separators between
54+
// elements while remaining lenient about a single trailing comma.
55+
private final Deque<Boolean> containerHasElement = new ArrayDeque<>();
5256

5357
JsonInput(Reader source, JsonTypeCoercer coercer, PropertySetting setter) {
5458

@@ -318,13 +322,29 @@ public boolean hasNext() {
318322
}
319323

320324
skipWhitespace(input);
325+
boolean seenElement = Boolean.TRUE.equals(containerHasElement.peekFirst());
326+
321327
if (input.peek() == ',') {
328+
if (!seenElement) {
329+
throw new JsonException(
330+
"Unexpected ',' before first element of container. " + input);
331+
}
322332
input.read();
323-
return true;
333+
skipWhitespace(input);
334+
JsonType afterComma = peek();
335+
// Trailing comma leniency: '[1,]' and '{"a":1,}' are accepted.
336+
return afterComma != JsonType.END_COLLECTION && afterComma != JsonType.END_MAP;
324337
}
325338

326339
JsonType type = peek();
327-
return type != JsonType.END_COLLECTION && type != JsonType.END_MAP;
340+
if (type == JsonType.END_COLLECTION || type == JsonType.END_MAP) {
341+
return false;
342+
}
343+
if (seenElement) {
344+
throw new JsonException(
345+
"Expected ',' or end of container but saw " + type + ". " + input);
346+
}
347+
return true;
328348
}
329349

330350
/**
@@ -335,6 +355,7 @@ public boolean hasNext() {
335355
public void beginArray() {
336356
expect(JsonType.START_COLLECTION);
337357
stack.addFirst(Container.COLLECTION);
358+
containerHasElement.addFirst(false);
338359
input.read();
339360
}
340361

@@ -346,6 +367,7 @@ public void beginArray() {
346367
public void endArray() {
347368
expect(JsonType.END_COLLECTION);
348369
Container expectation = stack.removeFirst();
370+
containerHasElement.removeFirst();
349371
if (expectation != Container.COLLECTION) {
350372
// The only other thing we could be closing is a map
351373
throw new JsonException(
@@ -362,6 +384,7 @@ public void endArray() {
362384
public void beginObject() {
363385
expect(JsonType.START_MAP);
364386
stack.addFirst(Container.MAP_NAME);
387+
containerHasElement.addFirst(false);
365388
input.read();
366389
}
367390

@@ -373,6 +396,7 @@ public void beginObject() {
373396
public void endObject() {
374397
expect(JsonType.END_MAP);
375398
Container expectation = stack.removeFirst();
399+
containerHasElement.removeFirst();
376400
if (expectation != Container.MAP_NAME) {
377401
// The only other thing we could be closing is a map
378402
throw new JsonException("Attempt to close a JSON Map, but not ready to. " + input);
@@ -530,10 +554,24 @@ private void expect(JsonType type) {
530554
return; // End of Name handling
531555
}
532556

533-
// Handle the case where we're reading a value
557+
// Handle the case where we're reading a value.
558+
if (type == JsonType.END_COLLECTION || type == JsonType.END_MAP) {
559+
// Closing the container - don't treat as a new element in it.
560+
return;
561+
}
534562
if (top == Container.MAP_VALUE) {
535563
stack.removeFirst();
536564
stack.addFirst(Container.MAP_NAME);
565+
markElementRead();
566+
} else if (top == Container.COLLECTION) {
567+
markElementRead();
568+
}
569+
}
570+
571+
private void markElementRead() {
572+
if (!containerHasElement.isEmpty()) {
573+
containerHasElement.removeFirst();
574+
containerHasElement.addFirst(true);
537575
}
538576
}
539577

java/test/org/openqa/selenium/json/JsonInputTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,51 @@ void shouldReadU_FFFF_AsALiteralCharacterAndNotEndOfInput() {
309309
}
310310
}
311311

312+
@Test
313+
void shouldAcceptTrailingCommaInArray() {
314+
try (JsonInput input = newInput("[1,2,3,]")) {
315+
input.beginArray();
316+
assertThat(input.hasNext()).isTrue();
317+
assertThat(input.nextNumber()).isEqualTo(1L);
318+
assertThat(input.hasNext()).isTrue();
319+
assertThat(input.nextNumber()).isEqualTo(2L);
320+
assertThat(input.hasNext()).isTrue();
321+
assertThat(input.nextNumber()).isEqualTo(3L);
322+
assertThat(input.hasNext()).isFalse();
323+
input.endArray();
324+
}
325+
}
326+
327+
@Test
328+
void shouldAcceptTrailingCommaInObject() {
329+
try (JsonInput input = newInput("{\"a\":1,}")) {
330+
input.beginObject();
331+
assertThat(input.hasNext()).isTrue();
332+
assertThat(input.nextName()).isEqualTo("a");
333+
assertThat(input.nextNumber()).isEqualTo(1L);
334+
assertThat(input.hasNext()).isFalse();
335+
input.endObject();
336+
}
337+
}
338+
339+
@Test
340+
void shouldRejectMissingCommaBetweenArrayElements() {
341+
try (JsonInput input = newInput("[1 2]")) {
342+
input.beginArray();
343+
assertThat(input.hasNext()).isTrue();
344+
assertThat(input.nextNumber()).isEqualTo(1L);
345+
assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext);
346+
}
347+
}
348+
349+
@Test
350+
void shouldRejectLeadingCommaInArray() {
351+
try (JsonInput input = newInput("[,1]")) {
352+
input.beginArray();
353+
assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext);
354+
}
355+
}
356+
312357
@Test
313358
void nullInputsShouldCoerceAsNullValues() throws IOException {
314359
try (InputStream is = new ByteArrayInputStream(new byte[0]);

0 commit comments

Comments
 (0)