Skip to content

fix: never fall back to an unescaped CFG for JSON string patterns - #986

Open
mahmoudhas wants to merge 1 commit into
mlc-ai:mainfrom
mahmoudhas:fix/json-pattern-safe-fsm
Open

mahmoudhas wants to merge 1 commit into
mlc-ai:mainfrom
mahmoudhas:fix/json-pattern-safe-fsm

Conversation

@mahmoudhas

Copy link
Copy Markdown

Problem

JSONSchemaConverter::RegexExpression falls back to a raw regex CFG when the JSON-safe FSM is unavailable/empty, and unconditionally routes non-ASCII/control-containing patterns to that fallback. The surrounding string quotes are then added without JSON escaping.

Minimal examples that the current grammar accepts as complete:

  • {"type":"string","pattern":"^\"$"} accepts raw """.
  • A literal-newline pattern accepts an unescaped newline inside the JSON string.
  • {"type":"string","pattern":"^[é\\s\\S]{1,64}$"} accepts "hello"world" and raw control bytes.

These are syntactically invalid JSON, regardless of independent schema validation.

Change

  • Let the existing JSON-string FSM handle UTF-8 patterns, preserving its quote/backslash/control exclusions.
  • Never fall back to an unescaped CFG for json_string=true.
  • Reject patterns whose JSON-safe canonical language is empty/unsupported at compilation. This intentionally rejects escaped-only patterns (e.g. a pattern requiring a literal quote) rather than emitting malformed JSON.
  • Preserve the raw regex/numeric CFG fallback.

This does not implement arbitrary escaped spellings or change the other existing regex semantics. Independent output validation should remain enabled.

Tests

75/75 C++ tests passed, including new tests covering Unicode/BMP/astral literals, wildcard patterns, all 32 C0 control bytes, quote exclusion, nested schemas, and pre-generation rejection of escaped-only patterns. Raw regex literal-quote matching remains supported.

Built against the native revision bundled in Rust xgrammar 0.5.1 (82505d0), with clang 18 on Linux aarch64. The relevant converter implementation still has the same fallback on current main.

@mahmoudhas
mahmoudhas marked this pull request as ready for review October 15, 2026 18:34
Copilot AI balanced review requested due to automatic review settings October 15, 2026 18:34
@mahmoudhas

Copy link
Copy Markdown
Author

One known remaining edge for reviewer awareness (non-blocking for us): this patch removes the unescaped-CFG fallback for JSON-string patterns, but the format: keyword path can still natively build a raw CFG inside quotes. In our downstream consumer (Corten) that path is unreachable because schema normalization rejects format as an unsupported keyword, so it does not affect us — but upstream consumers that accept format: may still hit it. Flagging in case you'd like to cover it here or in a follow-up. Thanks for the review!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Large-repeat helpers still bypass JSON exclusions, and existing Python tests retain incompatible expectations.

2 open findings
What changed in this PR

This PR hardens JSON Schema string-pattern conversion by removing the unescaped regex fallback while preserving raw regex behavior.

Changes:

  • Routes Unicode patterns through the JSON-safe FSM.
  • Rejects unsupported or empty JSON-safe pattern languages.
  • Adds C++ coverage for Unicode, forbidden characters, and escaped-only patterns.
File Description
tests/​cpp/​test_json.cc Adds pattern-safety and raw-regex regression tests.
cpp/​json_schema_converter.cc Enables Unicode FSM conversion and blocks unsafe JSON-string fallback.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +2220 to 2221
if (!force_cfg_expansion) {
auto fsm_result = GrammarFSMBuilder::Regex(regex, json_string);
Comment on lines +2236 to +2239
XGRAMMAR_CHECK(!json_string)
<< "JSON string pattern has no JSON-safe canonical representation or uses unsupported regex "
"syntax; patterns requiring escaped quotes, backslashes or control characters are not "
"supported";

This branch has not been deployed

No deployments
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.

2 participants