Skip to content

Fix trailing data accepted after top-level null#3008

Open
andrewstellman wants to merge 3 commits intogoogle:mainfrom
andrewstellman:fix-null-trailing-data-bypass
Open

Fix trailing data accepted after top-level null#3008
andrewstellman wants to merge 3 commits intogoogle:mainfrom
andrewstellman:fix-null-trailing-data-bypass

Conversation

@andrewstellman
Copy link
Copy Markdown
Contributor

@andrewstellman andrewstellman commented Apr 4, 2026

Summary

Gson.fromJson(String/Reader, ...) and JsonParser.parseReader(Reader) documentedly reject trailing data, but both currently make an exception when the parsed top-level value is JSON null.

For non-null values, trailing data already causes a JsonSyntaxException. For top-level null, the trailing-data check is skipped:

  • In Gson.java, assertFullConsumption() only peeks when the deserialized object is non-null.
  • In JsonParser.java, parseReader(Reader) only peeks when the parsed JsonElement is not JsonNull.

That means inputs like null trailing are silently accepted by the high-level "consume the whole document" APIs, even though those methods document that trailing data should be rejected.

Fix

Apply the trailing-data check regardless of whether the parsed value is null, while still preserving Gson's existing backward-compatible handling of empty or whitespace-only input.

Concretely:

  • remove the obj != null / !element.isJsonNull() guards
  • keep using reader.peek() / jsonReader.peek() to verify END_DOCUMENT
  • catch EOFException from that final peek and treat it as "input fully exhausted"

That EOFException handling is important because these APIs have long accepted empty or whitespace-only input as null / JsonNull, and this change should not alter that behavior.

This change only affects the high-level wrappers which are supposed to consume the entire document:

  • Gson.fromJson(String/Reader, ...)
  • JsonParser.parseReader(Reader)

The lower-level streaming entry points:

  • Gson.fromJson(JsonReader, ...)
  • JsonParser.parseReader(JsonReader)

intentionally allow trailing data and are unchanged.

Example

Gson gson = new Gson();

// Already rejected before this change
gson.fromJson("\"hello\" trailing", String.class); // JsonSyntaxException

// Previously accepted because top-level null skipped the full-consumption check
gson.fromJson("null trailing", String.class); // returned null

// After this change, top-level null is treated the same as any other value
gson.fromJson("null trailing", String.class); // JsonSyntaxException

Tests

Three regression tests cover the affected high-level APIs:

  • GsonTest.testFromJsonRejectsTrailingDataAfterNull_String
  • GsonTest.testFromJsonRejectsTrailingDataAfterNull_Reader
  • JsonParserTest.testParseReaderRejectsTrailingDataAfterNull

The String overload test and the JsonParser test also verify the existing non-null baseline ("hello" trailing) still throws.

Focused compatibility checks also confirm that the fix preserves accepted empty-input behavior:

  • JsonParserTest.testParseEmptyWhitespaceInput
  • ObjectTest.testEmptyStringDeserialization

A clean full gson module test run passes with:

mvn -pl gson clean test

Comment thread gson/src/test/java/com/google/gson/GsonTest.java Outdated
Comment thread gson/src/test/java/com/google/gson/JsonParserTest.java Outdated
andrewstellman and others added 2 commits April 5, 2026 09:48
fix comment

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
fix comment

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
@eamonnmcmanus
Copy link
Copy Markdown
Member

(Just an update here. I tried running this past Google's internal tests and I did encounter at least one failure. Unfortunately debugging the failure was not straightforward so I don't yet know if this is something that really would be affected by the behaviour change.)

Senrian pushed a commit to Senrian/gson that referenced this pull request Apr 15, 2026
Fixes google#3008

Both Gson.fromJson(Reader, ...) and JsonParser.parseReader(Reader)
documentedly reject trailing data, but made an exception when the
parsed top-level value was JSON 'null'. This change removes the
exception so trailing data is consistently rejected regardless of
the parsed value.

Changes:
- Gson.assertFullConsumption(): Changed condition from
  'obj != null && reader.peek() != END_DOCUMENT' to
  'obj == null || reader.peek() != END_DOCUMENT'
- JsonParser.parseReader(): Removed '!element.isJsonNull() &&' guard

Added test cases in JsonParserTest.java to verify trailing data
is rejected after parsing top-level null.
Senrian pushed a commit to Senrian/gson that referenced this pull request Apr 15, 2026
Fixes google#3008

Both Gson.fromJson(Reader, ...) and JsonParser.parseReader(Reader)
documentedly reject trailing data, but both made an exception when
the parsed top-level value was JSON 'null'.

Changes:
- Gson.assertFullConsumption(): if obj==null || peek()!=END_DOCUMENT
- JsonParser.parseReader(): removed !element.isJsonNull() guard

Added test cases in JsonParserTest.java.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants