Skip to content

JSON: Add opt-in comments and trailing comma flags - #23336

Open
otar wants to merge 12 commits into
php:masterfrom
otar:feature/json-comments-and-trailing-comma-parsing
Open

JSON: Add opt-in comments and trailing comma flags#23336
otar wants to merge 12 commits into
php:masterfrom
otar:feature/json-comments-and-trailing-comma-parsing

Conversation

@otar

@otar otar commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown

RFC: https://wiki.php.net/rfc/json_comments_trailing_commas
Discussion: https://news-web.php.net/php.internals/132868

Note: I used claude code for parts of the implementation and the tests. I reviewed and tested every change.

Configuration files often use a narrow JSON extension with JS-style comments, trailing commas, or both. PHP currently requires a separate parser or a string-aware preprocessing pass. This PR adds native parser support. Strict JSON remains the default.

Public API

Constant Value Accepted by Effect
JSON_ALLOW_COMMENTS 1 << 23 json_decode(), json_validate() Allows // and non-nesting /* ... */ comments at JSON whitespace boundaries.
JSON_ALLOW_TRAILING_COMMAS 1 << 24 json_decode(), json_validate() Allows one final comma in a nonempty array or object.

The flags are independent. Neither flag applies to json_encode() or is enabled by default.

Semantics

  • Line comments end at LF, CRLF, lone CR, or EOF. Block comments do not nest and end at the first */.
  • Comments are allowed before and after the root value and between tokens. They are not allowed inside a token. Comment-like text in a JSON string is unchanged.
  • U+2028/U+2029 and valid controls, including embedded NUL, are comment content. Malformed UTF-8 follows the existing decode flags. Validation still accepts JSON_INVALID_UTF8_IGNORE, but not JSON_INVALID_UTF8_SUBSTITUTE.
  • Unterminated block comments produce JSON_ERROR_SYNTAX at the opening slash.
  • A trailing comma is allowed only after the last element or member of a nonempty container. Empty or missing-element forms such as [,], {,}, and [1,,] remain invalid.
  • The implementation retains existing behavior for depth, duplicate keys, bigints, associative objects, exceptions, error codes, and original-input line and column numbers.

php/php-src#22473 uses the same flag values (1<<23 and 1<<24) for JSON_DUPLICATE_KEY_MERGE and JSON_DUPLICATE_KEY_ARRAY. The PR that merges second must move its flags to the next free bits.

@otar
otar requested review from bukka and kocsismate as code owners August 17, 2026 17:07
@otar
otar marked this pull request as draft August 17, 2026 17:08
@otar

otar commented Aug 17, 2026

Copy link
Copy Markdown
Author

Sorry @bukka and @kocsismate, I accidentally tagged you on this PR. Please ignore this draft...

@otar
otar force-pushed the feature/json-comments-and-trailing-comma-parsing branch from a4cd163 to 340b078 Compare October 15, 2026 07:05
@otar

otar commented Oct 15, 2026

Copy link
Copy Markdown
Author

Update: I did a git rebase to remove a merge commit.

otar and others added 12 commits October 30, 2026 12:28
@otar @claude
Add opt-in JSON comments and trailing comma parsing
395d929
@claude
Print the exception class and message in JSON relaxed-syntax tests
1b8360b 
Catch Throwable instead of a specific exception class, so that an
unexpected exception type shows in the test output instead of escaping
the try block. Print $e::class, the code and the message, and also assert
the ValueError message for the invalid json_validate() flags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XaWra1qpfXbgU9JZ4LozYB
@claude
Do not count a bare CR in JSON comments as a new line
d56f23e 
Outside comments, the JSON scanner treats a bare CR as whitespace and
counts only LF and CRLF as line endings. The comment rules also counted
a bare CR as a line ending. As a result, an error location after a bare
CR in a comment did not agree with the same input without the comment.

A bare CR still ends a line comment, but it no longer starts a new line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Reject NUL bytes in JSON comments
27d9c55 
Outside comments and inside strings, an embedded NUL byte is a control
character error. The comment rules skipped it as comment text. Some
other parsers end a comment at a NUL byte, so the same input decoded to
different values in PHP and in those parsers.

An embedded NUL in a line or block comment now gives
JSON_ERROR_CTRL_CHAR at the NUL byte. Other control characters stay
valid comment text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Test that strict JSON parsing keeps scanner error codes after a comma
578f43e 
The trailing-comma rules add a default reduction after a comma. Without
JSON_ALLOW_TRAILING_COMMAS, this reduction also runs when the scanner
returns an error, and only the errcode check in its action keeps the
scanner's error code. The test ran only with the relaxed flags, so it
did not run this check.

Run the cases also without flags, add an object case, and check
json_validate() too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Test invalid UTF-8 only inside JSON comments
9c81a38 
The block comment case had the invalid byte in the comment and also in
the string. Without flags, the string also gave error 5, and the test
did not print the location. Thus the test did not show that the comment
itself reports the error. A block comment rule that always skipped
invalid bytes also passed the test. The line comment case did not test
JSON_INVALID_UTF8_SUBSTITUTE or json_validate().

Put the invalid byte only in the comment, run the same checks for both
comment types, and print the error message so that the location is
also checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Use real newlines in JSON comment error tests
b6e4161 
Two cases used '\n' in single quotes, so the input contained a backslash
and an "n" instead of a newline. Use double quotes so that each case
tests the input that its comment describes. The expected output does
not change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Add edge-case tests for JSON comment delimiters
e3c10c6 
Add regression tests for comment delimiters that touch: an empty block
comment, an extra '*' before the closer, and a '/' right after the
opener. Also test a comment directly after a float and an exponent,
and a comment inside a number.

All cases already behave correctly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Use one grammar rule for JSON trailing commas
bdd462f 
The "member ','" and "element ','" rules had the same action. When
trailing commas were not allowed, each action freed the value by hand
before YYERROR, because bison does not run %destructor for the symbols
of the rule whose action calls YYERROR.

Move the comma into its own trailing_comma rule, as object_end and
array_end already do. YYERROR then removes only the comma, and the
%destructor frees the member or element value. php_json_yyerror()
keeps an error code that the scanner already set.

There is no change in behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Remove an unused copy of the block comment opener in the JSON scanner
7320cc7 
While the scanner is in COMMENT_BLOCK, the rules jump to the condition
label and do not pass the std label, so s->token keeps the position of
the "/*" opener. The comment_token copy and its restore had no effect.

Keep the line and line start copies, because the line ending rules
change them. Correct the code comment, which said that comments are
always fully consumed before the function returns.

There is no change in behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Add JSON comment and trailing comma tokens to the fuzzer dictionary
69c2885 
The dictionary had "//" and "/**/", but not the block comment
delimiters alone, a trailing comma before a closer, or a bare CR, which
ends a line comment. Add "/*", "*/", ",]", ",}" and "\x0d".

libFuzzer only accepts \\, \" and \xNN escapes in a dictionary, so the
CR is written as "\x0d".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@claude
Add UPGRADING notes for JSON comments and trailing commas
9342e09 
Describe the JSON_ALLOW_COMMENTS and JSON_ALLOW_TRAILING_COMMAS flags
under New Features, the new json_decode() and json_validate() flags
under Changed Functions, and the two constants under New Global
Constants.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCee9RRsbzYcC4ovMxhU6H
@otar
otar force-pushed the feature/json-comments-and-trailing-comma-parsing branch from 340b078 to 9342e09 Compare October 30, 2026 12:32
@otar
otar marked this pull request as ready for review October 30, 2026 18:03
@bukka

bukka commented Oct 30, 2026

Copy link
Copy Markdown
Member

I personally don't like non standard additions as they might complicate future optimizations in the parser. I think we need to consider them carefully so requesting RFC here.

@otar

otar commented Nov 2, 2026

Copy link
Copy Markdown
Author

@bukka Thank you for the review. This PR already has an RFC, which is under discussion, I forgot to add that to the PR description, now updated it...

I planned to open the vote today. I will postpone it, because I want to answer your concern in the RFC first.

Which optimizations do you have in mind? With some details I can describe the constraint correctly in the RFC.

Some facts about the current design:

  • Both flags are opt-in. Without them, the parser gives the same results and errors as before.
  • A comment can start only at a / byte outside a string. In strict JSON, this byte is already a syntax error. The comment rules are in separate scanner states.
  • Trailing commas use one small grammar rule.
  • A future fast parser can handle strict JSON only. When a flag is set, it can fall back to the current parser. So the flags do not limit optimizations for strict JSON.
  • In strict mode, the PR costs about 1.6% more CPU instructions (Callgrind, 7.6 MB input). A small scanner change reduces this to about 0.6%. I can add it to this PR.

@otar

otar commented Nov 5, 2026

Copy link
Copy Markdown
Author

Hi @bukka and @kocsismate

Should I wait for your review or is it OK if I open a voting on this feature?

@bukka

bukka commented Nov 5, 2026

Copy link
Copy Markdown
Member

I meant completely replacing parser with something close to simdjson which would make it harder to support those extensions without sacrificing performance / maintainability. But thinking about it, we might just keep alternative (slow parser) for extensions like this one. I got some plans with JsonSchema support (that got a bit on hold) but it would effectively need the same so I guess it's probably fine.

@otar

otar commented Nov 5, 2026

Copy link
Copy Markdown
Author

@bukka Thank you, this is clear.

I will add this point to the RFC, with a link to your comment. After the cooldown period, I will open the vote. Review comments on the code are still welcome and I will apply them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner
@bukka bukka Awaiting requested review from bukka bukka is a code owner
1 more reviewer
@NickSdot NickSdot NickSdot left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ABI break Extension: json RFC SAPI: fuzzer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@otar @bukka @NickSdot @TimWolla @claude