From 9c97f08c015eadc73154c3b8c2c69c968272da36 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Thu, 2 Jul 2026 11:33:30 +0100 Subject: [PATCH 1/4] [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. --- .../org/openqa/selenium/json/JsonInput.java | 44 ++++++++++++++++-- .../openqa/selenium/json/JsonInputTest.java | 45 +++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index 10e891e87eb17..41354c9238bb9 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -49,6 +49,10 @@ public class JsonInput implements Closeable { // Used when reading maps and collections so that we handle de-nesting and // figuring out whether we're expecting a NAME properly. private final Deque stack = new ArrayDeque<>(); + // Parallel stack tracking whether the current container has seen at least + // one element. Used by hasNext() to enforce comma separators between + // elements while remaining lenient about a single trailing comma. + private final Deque containerHasElement = new ArrayDeque<>(); JsonInput(Reader source, JsonTypeCoercer coercer, PropertySetting setter) { @@ -353,13 +357,29 @@ public boolean hasNext() { } skipWhitespace(input); + boolean seenElement = Boolean.TRUE.equals(containerHasElement.peekFirst()); + if (input.peek() == ',') { + if (!seenElement) { + throw new JsonException( + "Unexpected ',' before first element of container. " + input); + } input.read(); - return true; + skipWhitespace(input); + JsonType afterComma = peek(); + // Trailing comma leniency: '[1,]' and '{"a":1,}' are accepted. + return afterComma != JsonType.END_COLLECTION && afterComma != JsonType.END_MAP; } JsonType type = peek(); - return type != JsonType.END_COLLECTION && type != JsonType.END_MAP; + if (type == JsonType.END_COLLECTION || type == JsonType.END_MAP) { + return false; + } + if (seenElement) { + throw new JsonException( + "Expected ',' or end of container but saw " + type + ". " + input); + } + return true; } /** @@ -370,6 +390,7 @@ public boolean hasNext() { public void beginArray() { expect(JsonType.START_COLLECTION); stack.addFirst(Container.COLLECTION); + containerHasElement.addFirst(false); input.read(); } @@ -381,6 +402,7 @@ public void beginArray() { public void endArray() { expect(JsonType.END_COLLECTION); Container expectation = stack.removeFirst(); + containerHasElement.removeFirst(); if (expectation != Container.COLLECTION) { // The only other thing we could be closing is a map throw new JsonException( @@ -397,6 +419,7 @@ public void endArray() { public void beginObject() { expect(JsonType.START_MAP); stack.addFirst(Container.MAP_NAME); + containerHasElement.addFirst(false); input.read(); } @@ -408,6 +431,7 @@ public void beginObject() { public void endObject() { expect(JsonType.END_MAP); Container expectation = stack.removeFirst(); + containerHasElement.removeFirst(); if (expectation != Container.MAP_NAME) { // The only other thing we could be closing is a map throw new JsonException("Attempt to close a JSON Map, but not ready to. " + input); @@ -565,10 +589,24 @@ private void expect(JsonType type) { return; // End of Name handling } - // Handle the case where we're reading a value + // Handle the case where we're reading a value. + if (type == JsonType.END_COLLECTION || type == JsonType.END_MAP) { + // Closing the container - don't treat as a new element in it. + return; + } if (top == Container.MAP_VALUE) { stack.removeFirst(); stack.addFirst(Container.MAP_NAME); + markElementRead(); + } else if (top == Container.COLLECTION) { + markElementRead(); + } + } + + private void markElementRead() { + if (!containerHasElement.isEmpty()) { + containerHasElement.removeFirst(); + containerHasElement.addFirst(true); } } diff --git a/java/test/org/openqa/selenium/json/JsonInputTest.java b/java/test/org/openqa/selenium/json/JsonInputTest.java index 037c5714c5ca7..187b63e32bf3d 100644 --- a/java/test/org/openqa/selenium/json/JsonInputTest.java +++ b/java/test/org/openqa/selenium/json/JsonInputTest.java @@ -384,6 +384,51 @@ void shouldReportEndOfInputAsEofNotAsAUnicodeReplacement() { } } + @Test + void shouldAcceptTrailingCommaInArray() { + try (JsonInput input = newInput("[1,2,3,]")) { + input.beginArray(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(2L); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(3L); + assertThat(input.hasNext()).isFalse(); + input.endArray(); + } + } + + @Test + void shouldAcceptTrailingCommaInObject() { + try (JsonInput input = newInput("{\"a\":1,}")) { + input.beginObject(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextName()).isEqualTo("a"); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThat(input.hasNext()).isFalse(); + input.endObject(); + } + } + + @Test + void shouldRejectMissingCommaBetweenArrayElements() { + try (JsonInput input = newInput("[1 2]")) { + input.beginArray(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext); + } + } + + @Test + void shouldRejectLeadingCommaInArray() { + try (JsonInput input = newInput("[,1]")) { + input.beginArray(); + assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext); + } + } + @Test void nullInputsShouldCoerceAsNullValues() throws IOException { try (InputStream is = new ByteArrayInputStream(new byte[0]); From d9c9e393f0396620dcbf30b873137b20a4b538ae Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Thu, 2 Jul 2026 14:10:34 +0100 Subject: [PATCH 2/4] [java] Format long JsonException throws to satisfy google-java-format --- java/src/org/openqa/selenium/json/JsonInput.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index 41354c9238bb9..feb1dc6383970 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -361,8 +361,7 @@ public boolean hasNext() { if (input.peek() == ',') { if (!seenElement) { - throw new JsonException( - "Unexpected ',' before first element of container. " + input); + throw new JsonException("Unexpected ',' before first element of container. " + input); } input.read(); skipWhitespace(input); @@ -376,8 +375,7 @@ public boolean hasNext() { return false; } if (seenElement) { - throw new JsonException( - "Expected ',' or end of container but saw " + type + ". " + input); + throw new JsonException("Expected ',' or end of container but saw " + type + ". " + input); } return true; } From 73987abbc9ac3b1f564b311965bb5f42575d741e Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Fri, 3 Jul 2026 15:23:35 +0100 Subject: [PATCH 3/4] [java] Make JsonInput.hasNext() idempotent between elements hasNext() consumed the comma between elements but did not clear the 'container has element' flag, so a second hasNext() before reading the next value threw as if the comma were missing. Iterator-style callers that probe multiple times per element saw spurious failures on valid input like [1,2]. --- .../org/openqa/selenium/json/JsonInput.java | 10 ++++++ .../openqa/selenium/json/JsonInputTest.java | 33 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index feb1dc6383970..d2fc7f78b8950 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -364,6 +364,9 @@ public boolean hasNext() { throw new JsonException("Unexpected ',' before first element of container. " + input); } input.read(); + // We've moved past the separator, so we're once again expecting an element rather than + // another comma. Clear the flag so a repeat hasNext() before reading is a no-op. + clearSeenElement(); skipWhitespace(input); JsonType afterComma = peek(); // Trailing comma leniency: '[1,]' and '{"a":1,}' are accepted. @@ -608,6 +611,13 @@ private void markElementRead() { } } + private void clearSeenElement() { + if (!containerHasElement.isEmpty()) { + containerHasElement.removeFirst(); + containerHasElement.addFirst(false); + } + } + /** * Read the next element from the JSON input stream, converting with the supplied mapper if it's * the expected string. diff --git a/java/test/org/openqa/selenium/json/JsonInputTest.java b/java/test/org/openqa/selenium/json/JsonInputTest.java index 187b63e32bf3d..affe5298cac88 100644 --- a/java/test/org/openqa/selenium/json/JsonInputTest.java +++ b/java/test/org/openqa/selenium/json/JsonInputTest.java @@ -429,6 +429,39 @@ void shouldRejectLeadingCommaInArray() { } } + @Test + void hasNextIsIdempotentBetweenElementsInArray() { + // Iterator-style probing (peek/hasNext repeatedly before reading) must not falsely fail + // once hasNext() has already consumed the comma before the next element. + try (JsonInput input = newInput("[1,2]")) { + input.beginArray(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThat(input.hasNext()).isTrue(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextNumber()).isEqualTo(2L); + assertThat(input.hasNext()).isFalse(); + assertThat(input.hasNext()).isFalse(); + input.endArray(); + } + } + + @Test + void hasNextIsIdempotentBetweenEntriesInObject() { + try (JsonInput input = newInput("{\"a\":1,\"b\":2}")) { + input.beginObject(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextName()).isEqualTo("a"); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThat(input.hasNext()).isTrue(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextName()).isEqualTo("b"); + assertThat(input.nextNumber()).isEqualTo(2L); + assertThat(input.hasNext()).isFalse(); + input.endObject(); + } + } + @Test void nullInputsShouldCoerceAsNullValues() throws IOException { try (InputStream is = new ByteArrayInputStream(new byte[0]); From b91099f700756c4538c9756551548ab26fd44f8d Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Fri, 3 Jul 2026 16:06:44 +0100 Subject: [PATCH 4/4] [java] Validate container type before popping parser state endArray()/endObject() popped the container stacks before checking the container type, so a mismatched close corrupted parser state on top of throwing. Check first, pop after. Also cover object-side comma enforcement with tests to match the existing array cases. --- .../org/openqa/selenium/json/JsonInput.java | 13 ++++--- .../openqa/selenium/json/JsonInputTest.java | 35 +++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index d2fc7f78b8950..d4b23c9760b47 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -402,13 +402,13 @@ public void beginArray() { */ public void endArray() { expect(JsonType.END_COLLECTION); - Container expectation = stack.removeFirst(); - containerHasElement.removeFirst(); - if (expectation != Container.COLLECTION) { + if (stack.peekFirst() != Container.COLLECTION) { // The only other thing we could be closing is a map throw new JsonException( "Attempt to close a JSON List, but a JSON Object was expected. " + input); } + stack.removeFirst(); + containerHasElement.removeFirst(); input.read(); } @@ -431,12 +431,11 @@ public void beginObject() { */ public void endObject() { expect(JsonType.END_MAP); - Container expectation = stack.removeFirst(); - containerHasElement.removeFirst(); - if (expectation != Container.MAP_NAME) { - // The only other thing we could be closing is a map + if (stack.peekFirst() != Container.MAP_NAME) { throw new JsonException("Attempt to close a JSON Map, but not ready to. " + input); } + stack.removeFirst(); + containerHasElement.removeFirst(); input.read(); } diff --git a/java/test/org/openqa/selenium/json/JsonInputTest.java b/java/test/org/openqa/selenium/json/JsonInputTest.java index affe5298cac88..23ba6734759f8 100644 --- a/java/test/org/openqa/selenium/json/JsonInputTest.java +++ b/java/test/org/openqa/selenium/json/JsonInputTest.java @@ -429,6 +429,41 @@ void shouldRejectLeadingCommaInArray() { } } + @Test + void shouldRejectMissingCommaBetweenObjectEntries() { + try (JsonInput input = newInput("{\"a\":1 \"b\":2}")) { + input.beginObject(); + assertThat(input.hasNext()).isTrue(); + assertThat(input.nextName()).isEqualTo("a"); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext); + } + } + + @Test + void shouldRejectLeadingCommaInObject() { + try (JsonInput input = newInput("{,\"a\":1}")) { + input.beginObject(); + assertThatExceptionOfType(JsonException.class).isThrownBy(input::hasNext); + } + } + + @Test + void mismatchedCloseLeavesParserStateIntact() { + // endObject() while inside an array must fail without popping the container stack, + // so the parser still knows it is inside the array afterwards. + try (JsonInput input = newInput("[1}")) { + input.beginArray(); + assertThat(input.nextNumber()).isEqualTo(1L); + assertThatExceptionOfType(JsonException.class) + .isThrownBy(input::endObject) + .withMessageStartingWith("Attempt to close a JSON Map"); + // Before the fix this threw "not in a container type" because the array's + // stack entry had already been popped. + assertThat(input.hasNext()).isFalse(); + } + } + @Test void hasNextIsIdempotentBetweenElementsInArray() { // Iterator-style probing (peek/hasNext repeatedly before reading) must not falsely fail