mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-28 01:32:17 +00:00
fix(judge): stop the JSON-fence regex backtracking on an unclosed fence
`parse_json_verdict` pulls the JSON out of a judge model's reply with
re.compile(r"```(?:json)?\s*(.*?)\s*```", re.DOTALL | re.IGNORECASE)
Under DOTALL a dot matches a space, so each `\s*` overlaps the `.*?` next to
it. A reply that opens a fence and never closes one makes the match fail, and
a whitespace run before the failure can be split between the three quantifiers
in cubic-many ways -- the engine walks all of them:
4 KB of padding -> 64 s of CPU
8 KB -> ~8x that
That runs inside `LLMAsAJudgeGuardrail`'s async hook and in
`shadow_eval_logger`, both on the event loop. Measured on the unfixed code, a
2 KB reply parsed for 8.8 s while an asyncio heartbeat ticked 6 times where a
free loop would have ticked ~183.
The judge is fed the end user's own request and response text, so the reply it
produces is steerable by that user: asking for a fenced block and then for
several kilobytes of padding is enough, and no operator setting is involved.
The two `\s*` were only trimming padding that the caller already trims -- the
one call site does `fenced.group(1).strip()`. Dropping them makes the pattern
linear and leaves the parsed value identical: 460,009 inputs (every combination
of fence, quote, brace, newline and space fragments up to length 5, plus random
and realistic replies) produce the same `group(1).strip()` under both patterns.
4 KB unclosed fence: 72 s -> 0.000126 s
1 MB unclosed fence: 0.016 s
Found by sweeping every regex litellm compiles at runtime -- 1,255 distinct
patterns, including the f-string-built ones a source scan misses -- through
regexploit. This was the only catastrophic one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016SxQ2jq4B4KLGwPoLgfHye
This commit is contained in:
parent
29ac88ebc6
commit
74098906b6
2 changed files with 40 additions and 1 deletions
|
|
@ -15,7 +15,11 @@ if TYPE_CHECKING:
|
|||
from litellm.types.llms.openai import AllMessageValues
|
||||
from litellm.types.utils import ModelResponse
|
||||
|
||||
JSON_FENCE_RE: Final = re.compile(r"```(?:json)?\s*(.*?)\s*```", re.DOTALL | re.IGNORECASE)
|
||||
# No `\s*` around the capture: under DOTALL a dot matches a space too, so each
|
||||
# `\s*` overlaps the `.*?` beside it, and a reply that opens a fence without closing
|
||||
# one can be split between them in cubic-many ways -- a failing match walks all of
|
||||
# them. The only caller strips the capture, which is what those `\s*` were doing.
|
||||
JSON_FENCE_RE: Final = re.compile(r"```(?:json)?(.*?)```", re.DOTALL | re.IGNORECASE)
|
||||
|
||||
|
||||
def default_router_provider() -> Router | None:
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
"""Unit tests for the shared LLM-judge primitives: verdict parsing, router resolution, dispatch."""
|
||||
|
||||
import json
|
||||
import time
|
||||
from typing import Final
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
|
@ -27,6 +29,39 @@ def test_parse_json_verdict_tolerates_fences_and_prose(raw, expected):
|
|||
assert parse_json_verdict(raw)["preference"] == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"raw,expected",
|
||||
[
|
||||
('```json {"preference": "A"} ```', "A"),
|
||||
('```\n\n {"preference": "B"} \n\n```', "B"),
|
||||
('```json{"preference": "tie"}```', "tie"),
|
||||
],
|
||||
)
|
||||
def test_parse_json_verdict_still_trims_padding_inside_the_fence(raw, expected):
|
||||
"""Whitespace between the fence and the JSON is dropped, however it is spelled."""
|
||||
assert parse_json_verdict(raw)["preference"] == expected
|
||||
|
||||
|
||||
def test_parse_json_verdict_is_not_quadratic_in_an_unclosed_fence():
|
||||
"""A judge reply that opens a fence and never closes it must not burn the event loop.
|
||||
|
||||
The judge is fed the end user's own text, so the reply it produces is steerable by
|
||||
that user. The fence regex used to put a `\\s*` on each side of its capture; under
|
||||
DOTALL those overlap the capture, and a failing match walks cubic-many ways to split
|
||||
the whitespace run between them -- 4KB of padding took over a minute of CPU, on the
|
||||
loop, inside an async guardrail hook. The budget below is ~500x the fixed cost and
|
||||
~1/14th the unfixed one, so it separates the two without depending on runner speed.
|
||||
"""
|
||||
reply: Final = "```" + " " * 4000 + "no closing fence"
|
||||
|
||||
start: Final = time.perf_counter()
|
||||
with pytest.raises((json.JSONDecodeError, ValueError)):
|
||||
parse_json_verdict(reply)
|
||||
elapsed: Final = time.perf_counter() - start
|
||||
|
||||
assert elapsed < 5.0, f"parsing a 4KB unclosed fence took {elapsed:.1f}s"
|
||||
|
||||
|
||||
def test_parse_json_verdict_rejects_non_object():
|
||||
with pytest.raises(ValueError, match='judge response is not a JSON object'):
|
||||
parse_json_verdict('["not", "an", "object"]')
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue