Bound and offload grammar validation for response_format by windreamer · Pull Request #4987 · InternLM/lmdeploy

windreamer · 2026-09-18T06:46:42Z

What

response_format grammar sources are now validated within fixed bounds before XGrammar compilation, and validation runs off the asyncio event loop under a hard timeout:

Reject grammar sources larger than 16 KiB and JSON schemas deeper than 128 raw JSON nesting levels (~64 schema levels through properties wrappers); over-limit requests are rejected with HTTP 400.
Run ensure_response_format_compilable in a worker thread with a 5-second budget (asyncio.wait_for(asyncio.to_thread(...))), so validation can never stall the event loop, health endpoint, or streaming responses.
The bounds live in _grammar_source, the single choke point shared by API-process validation and engine-side compilation (PyTorch GuidedDecodingManager, TurboMind), so all paths are covered. No engine-side code change is required.

Why

Grammar compilation cost grows exponentially with schema nesting depth, and XGrammar (<= 0.2.x) exposes no depth or time limit on that path. Compiling client-supplied schemas synchronously on the event loop without bounds is a resource-safety hazard for the serving process. Details in the security advisory (private until publication).

Tests

13 new offline unit tests (tests/test_lmdeploy/test_guided_decoding.py, no GPU/tokenizer needed) cover:

Depth/size boundary: schema at the cap passes, one level over is rejected
Deep string schemas, oversized regex and flat schemas
Engine-side compile path inherits the bounds
Validation runs off the event loop; timeout surfaces as ValueError; event loop stays responsive (heartbeat) during a real uncached validation

Docs: limits section added to structed_output.md (en/zh).

A deeply nested response_format JSON schema makes XGrammar's compile time grow exponentially with depth. The compile ran synchronously on the asyncio event loop with no depth, size, or time bound, so a small request body could stall the whole single-process API server for tens of seconds (GHSA-6rm2-wfp4-m82q). - reject grammar sources over 16 KiB and JSON schemas deeper than 128 raw JSON levels (~64 schema levels) with HTTP 400 before compilation; the check lives in _grammar_source so both API-process validation and engine-side compilation inherit it - run validation in a worker thread under a 5 s budget via asyncio.wait_for(asyncio.to_thread(...)) so the event loop, health endpoint, and streaming responses stay responsive

Copilot

🟡 Changes recommended

The byte limit can be bypassed with multibyte Unicode, and timed-out worker-thread compilations continue running without cancellation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds bounded, asynchronous response_format grammar validation to reduce XGrammar resource risks.

Changes:

Enforces grammar size and JSON nesting limits.
Moves validation off the asyncio event loop with a timeout.
Adds unit tests and English/Chinese documentation.

File summaries

File	Description
`lmdeploy/_guided_decoding.py`	Implements bounds and asynchronous validation.
`lmdeploy/serve/core/async_engine.py`	Awaits request validation.
`tests/test_lmdeploy/test_guided_decoding.py`	Tests limits, timeout, and responsiveness.
`docs/en/advance/structed_output.md`	Documents validation limits.
`docs/zh_cn/advance/structed_output.md`	Documents validation limits in Chinese.

Review details

Files reviewed: 5/5 changed files
Comments generated: 2
Review effort level: Lite

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

Copilot

🟡 Changes recommended

The byte limit counts characters, timed-out worker compiles continue running, and one new deep-schema test can fail during setup.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

lmdeploy/_guided_decoding.py:50

MAX_GRAMMAR_SOURCE_BYTES is documented and named in bytes, but len(source) counts Unicode code points. A schema or regex containing multibyte UTF-8 characters can therefore exceed 16 KiB while passing this check; measure the UTF-8 encoded length so the enforced limit matches the advertised bound.

    if len(source) > MAX_GRAMMAR_SOURCE_BYTES:

lmdeploy/_guided_decoding.py:144

asyncio.wait_for cancels the awaitable on timeout, but it cannot stop the underlying to_thread worker. A timed-out XGrammar compile therefore continues consuming a default-executor slot and CPU; enough concurrent hostile requests can exhaust the workers or keep compiling after the request was rejected, so this is not a hard bound on the validation work. Enforce cancellation/backpressure with a bounded worker that can actually terminate the compile (for example, process isolation).

        await asyncio.wait_for(
            asyncio.to_thread(_ensure_response_format_compilable, response_format), timeout=GRAMMAR_COMPILE_TIMEOUT
        )

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

- count grammar source size in UTF-8 bytes, not code points; with ensure_ascii=False a code-point count understates multibyte sources by up to 4x - run validation in a small dedicated executor instead of the default one: a compile that outlives its request's 5 s timeout cannot be cancelled, and isolating it keeps such stragglers from occupying the interpreter's default executor shared by other offloaded work

Copilot

🟡 Changes recommended

Validation failures can leak API-created sessions because cleanup runs before session assignment.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

preprocess ran the response_format check before taking charge of the session, so a rejection raised ValueError while the local session variable was still None and remove_session() no-op'ed, leaking the caller-created session (api_server registers it in SessionManager before calling the engine). Resolve the session and check its conflict state first, then validate; cleanup now covers rejections.

Copilot

🟡 Changes recommended

Structural tags can still bypass the JSON-schema nesting-depth limit.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🔵 Needs a closer look

It changes request admission and compilation concurrency, with an unresolved depth-walker defect requiring correction.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Balanced

The depth walker only counted dict/list levels, so in-process callers could smuggle depth through tuples, which json.dumps serializes as arrays, and a cyclic dict spun the walker forever on the synchronous engine-side compile_response_format path (previously it failed with json.dumps' circular-reference ValueError). The walk now counts every container type json.dumps serializes, stops as soon as the cap is exceeded, and rejects cycles on the active path.

Merge same-path cases (depth cap pass/reject, string-path branches, async valid/invalid), drop the redundant over-deep dict case (identical rejection branch as one-past-cap), and keep every branch covered: the tuple and cycle walkers, the string-path size bound, the multibyte byte-count regression, engine-path bounds, pool isolation, timeout, loop liveness, and session cleanup.

windreamer · 2026-09-18T08:20:41Z

Replying to the 2026-09-18 07:59 Copilot review — both findings from that round are resolved on the branch:

structural_tag bypass: not exploitable — the gpt-oss wrap path is capped by pydantic serialization (~124 levels, raises a ValueError subclass → clean 400 before preprocess), and the OpenAI-protocol ResponseFormat.type is a closed Literal, so the exponential compile region is unreachable. Details and measurements in the inline thread; the size-only bound is deliberate.
depth-walker defect: fixed in 095f56c — tuples now count (json.dumps serializes them as arrays) and cycles are rejected on the active traversal path, restoring the fast ValueError the engine-side compile path had before. Regression tests cover both.

The request-admission change (session resolved before response_format validation) also carries a regression test asserting cleanup covers rejections. Tests were then consolidated 18 → 13 in 8959f80 without losing branch coverage.

Copilot

🟢 Approval recommended

Validation safeguards, cleanup behavior, tests, and documentation are consistent and complete.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot AI lite review requested due to automatic review settings September 18, 2026 06:46

Copilot started reviewing on behalf of windreamer September 18, 2026 06:47 View session