Skip to content

Fix JsonCodec decoder accepting malformed JSON with trailing characters - #1136

Open
chengdazhi wants to merge 1 commit into
zio:mainfrom
chengdazhi:fix/json-codec-reject-trailing-characters
Open

Fix JsonCodec decoder accepting malformed JSON with trailing characters#1136
chengdazhi wants to merge 1 commit into
zio:mainfrom
chengdazhi:fix/json-codec-reject-trailing-characters

Conversation

@chengdazhi

Copy link
Copy Markdown

/claim 712
Closes #712

Problem

JsonCodec's decoders stop after the first valid JSON value and silently ignore any remaining input, so malformed documents like {}}, {"a":1}} or "foo"" decode successfully (see the Scastie reproducer in the issue).

Fix

The strictness cannot be added by overriding decodeJson, because zio-json declares it final; it funnels through unsafeDecode with a FastStringReader, while the streaming entry points (decodeJsonPipeline, decodeJsonStream, ...) use other RetractReader implementations. This PR therefore:

  • adds a private StrictJsonDecoder wrapper whose unsafeDecode rejects any non-whitespace remainder, but only when the reader is a FastStringReader, so single-value decoding becomes strict while streaming stays lenient;
  • handles a zio-json quirk where the number parsers retract one character after a number, stepping back into the number's last digit when the number ends exactly at the end of the input (top-level numbers like 42 would otherwise be wrongly rejected);
  • returns the strict decoder from jsonDecoder(schema) and uses it in JsonDecoder.decode, schemaBasedBinaryCodec.decode and zioJsonBinaryCodec.decode;
  • keeps schemaBasedBinaryCodec.streamDecoder and zioJsonBinaryCodec.streamDecoder lenient (via JsonDecoder.decodeLenient and unwrapping of StrictJsonDecoder), preserving existing behavior such as 1 2, 3;;; 4x5 decoding as a stream of five ints and partial Fallback decodes ([30,"hello"] as Fallback.Left(30)).

Tests

New trailing characters after a valid JSON value suite in JsonCodecSpec, covering the issue reproducers ({}}, {"a":1}}, "foo""), extra characters after arrays/booleans/numbers, top-level numbers (the retract quirk), acceptance of leading/trailing whitespace, strictness of schemaBasedBinaryCodec.decode and zioJsonBinaryCodec.decode, and unchanged newline-delimited stream decoding.

Verified locally: zioSchemaJsonJVM/test on Scala 2.12.21, 2.13.18 and 3.3.8 (312 tests), zioSchemaJsonJS/test (307 tests), zioSchemaJsonNative/Test/compile, fmtCheck, fixCheck and mimaReportBinaryIssues.

Comparison with #724

#724 applies the trailing check inside schemaDecoder itself, so it also runs after every nested value (e.g. right after a field value, where the object's closing } follows), which breaks decoding of nested structures; it also reformats the whole schemaDecoder match and currently has merge conflicts with main. This PR applies the check only at the top-level decode entry points, leaves nested decoding untouched, and additionally covers the BinaryCodec entry points and the number-retract edge case described above.


This PR was prepared with the assistance of an AI coding agent; every change was verified by running the test suites and lint checks listed above.

Reject non-whitespace input remaining after a valid JSON value in the
single-value decode entry points (jsonDecoder(...).decodeJson,
JsonDecoder.decode, schemaBasedBinaryCodec.decode, zioJsonBinaryCodec.decode),
while keeping stream decoding lenient.

Closes zio#712
@chengdazhi
chengdazhi requested a review from a team as a code owner July 13, 2026 08:26
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@chengdazhi

Copy link
Copy Markdown
Author

Note on the failing build (temurin:21, 3.3.8, Native) check: this is a pre-existing Scala Native failure unrelated to this PR — the same job family currently fails on main itself (see main CI run, build (temurin:21, 2.13.18, Native), SIGSEGV in the native test runner). All JVM (2.12/2.13/3.3) and JS checks pass.

@chengdazhi

Copy link
Copy Markdown
Author

Note on CI: the single failing job, build (temurin:21, 3.3.8, Native), crashed with a SIGSEGV (Process ... finished with non-zero value 139) in testsNative, after all tests in the module had reported as passed. The same crash signature failed today's main CI run (build (temurin:21, 2.13.18, Native), run 29231522208), so this looks like the pre-existing scala-native runner flakiness rather than something introduced here — this PR only touches shared Scala code in zio-schema-json, whose own Native jobs (including temurin:17, 3.3.8, Native) are green. I don't have permission to re-run the job; could a maintainer retry it?

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.

JsonCodec decoder accepts malformed JSON string with extra characters

2 participants