mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-20 20:44:17 +02:00
fix(tools): decode JSON-string structural arguments before validation
Pydantic's lax mode already coerces scalar strings (e.g. "7" -> int), but never JSON-decodes a string into list/dict/set/model types, so tools with a list/dict/model parameter reject clients that send it as a JSON string. Retry validation once with json.loads'd top-level string arguments when the first pass fails structurally; gated on strict_input_validation. Fixes #553 🤖 Generated with Claude Code
This commit is contained in:
parent
06fee6d300
commit
df3f4df0bd
2 changed files with 203 additions and 8 deletions
|
|
@ -4,6 +4,7 @@ from __future__ import annotations
|
|||
|
||||
import functools
|
||||
import inspect
|
||||
import json
|
||||
import logging
|
||||
from collections.abc import Callable
|
||||
from dataclasses import dataclass, field
|
||||
|
|
@ -136,6 +137,54 @@ def _strict_input_validation() -> bool:
|
|||
return context.fastmcp.strict_input_validation
|
||||
|
||||
|
||||
def _coerce_json_string_arguments(
|
||||
arguments: dict[str, Any], errors: list[Any]
|
||||
) -> dict[str, Any] | None:
|
||||
"""Decode top-level arguments that failed validation as a JSON string.
|
||||
|
||||
Some MCP clients serialize structured arguments (lists, dicts, models) as
|
||||
JSON strings instead of native JSON values. Pydantic's lax mode already
|
||||
coerces string scalars (e.g. the string ``"7"`` into an ``int``); this
|
||||
extends the same leniency to structural types that lax mode can't coerce
|
||||
on its own. Returns ``None`` if no argument could be decoded, so the
|
||||
caller raises the original error unchanged. See #553.
|
||||
"""
|
||||
coerced: dict[str, Any] | None = None
|
||||
for error in errors:
|
||||
loc = error["loc"]
|
||||
if len(loc) != 1:
|
||||
continue # only top-level arguments; nested errors aren't ours to fix
|
||||
name = loc[0]
|
||||
value = arguments.get(name)
|
||||
if not isinstance(name, str) or not isinstance(value, str):
|
||||
continue
|
||||
try:
|
||||
decoded = json.loads(value)
|
||||
except ValueError:
|
||||
continue
|
||||
if coerced is None:
|
||||
coerced = dict(arguments)
|
||||
coerced[name] = decoded
|
||||
return coerced
|
||||
|
||||
|
||||
def _validate_call_arguments(
|
||||
type_adapter: TypeAdapter[Any], arguments: dict[str, Any], *, strict: bool
|
||||
) -> Any:
|
||||
"""Validate call arguments, retrying with JSON-decoded values for any
|
||||
argument that arrived as a JSON string but failed structural validation.
|
||||
"""
|
||||
try:
|
||||
return type_adapter.validate_python(arguments, strict=strict)
|
||||
except PydanticValidationError as original:
|
||||
if strict:
|
||||
raise
|
||||
coerced = _coerce_json_string_arguments(arguments, original.errors())
|
||||
if coerced is None:
|
||||
raise
|
||||
return type_adapter.validate_python(coerced, strict=strict)
|
||||
|
||||
|
||||
F = TypeVar("F", bound=Callable[..., Any])
|
||||
|
||||
|
||||
|
|
@ -464,21 +513,23 @@ class FunctionTool(Tool):
|
|||
|
||||
When ``strict`` is set (server-level ``strict_input_validation``),
|
||||
pydantic validates in strict mode, so lax coercions such as the JSON
|
||||
string ``"10"`` into an ``int`` are rejected rather than coerced.
|
||||
string ``"10"`` into an ``int`` are rejected rather than coerced. In
|
||||
non-strict mode, ``_validate_call_arguments`` also decodes structural
|
||||
arguments (list/dict/model) sent as a JSON string.
|
||||
"""
|
||||
# Combining timeout with run_in_thread=False on a sync function is
|
||||
# rejected at registration (see FunctionTool.from_function), so this only
|
||||
# needs to handle async and threadpool-sync under a timeout.
|
||||
if exec_is_async:
|
||||
# Argument validation is synchronous; the body runs on await below.
|
||||
result = type_adapter.validate_python(arguments, strict=strict)
|
||||
result = _validate_call_arguments(type_adapter, arguments, strict=strict)
|
||||
elif self.run_in_thread:
|
||||
# Sync function: run in threadpool to avoid blocking the event loop.
|
||||
result = await call_sync_fn_in_threadpool(
|
||||
type_adapter.validate_python, arguments, strict=strict
|
||||
_validate_call_arguments, type_adapter, arguments, strict=strict
|
||||
)
|
||||
else:
|
||||
result = type_adapter.validate_python(arguments, strict=strict)
|
||||
result = _validate_call_arguments(type_adapter, arguments, strict=strict)
|
||||
|
||||
try:
|
||||
if inspect.isawaitable(result):
|
||||
|
|
|
|||
|
|
@ -131,15 +131,34 @@ class TestPydanticModelArguments:
|
|||
assert "Alice" in result.content[0].text
|
||||
assert "30" in result.content[0].text
|
||||
|
||||
async def test_stringified_json_not_auto_parsed_for_pydantic_models(self):
|
||||
"""Stringified JSON is rejected for Pydantic model parameters.
|
||||
async def test_stringified_json_auto_parsed_for_pydantic_models(self):
|
||||
"""Stringified JSON is decoded for Pydantic model parameters.
|
||||
|
||||
Some LLM clients send stringified JSON (a JSON string containing a
|
||||
JSON object) instead of a proper JSON object. FastMCP does not
|
||||
auto-parse these; callers get a validation error.
|
||||
JSON object) instead of a proper JSON object. Without strict
|
||||
validation, FastMCP decodes it before validating, the same leniency
|
||||
already applied to scalar coercion (e.g. ``"7"`` -> ``int``). See #553.
|
||||
"""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def create_user(profile: UserProfile) -> str:
|
||||
"""Create a user from a profile."""
|
||||
return f"Created user {profile.name}, age {profile.age}"
|
||||
|
||||
async with Client(mcp) as client:
|
||||
stringified = json.dumps(
|
||||
{"name": "Bob", "age": 25, "email": "bob@example.com"}
|
||||
)
|
||||
|
||||
result = await client.call_tool("create_user", {"profile": stringified})
|
||||
assert isinstance(result.content[0], TextContent)
|
||||
assert "Bob" in result.content[0].text
|
||||
|
||||
async def test_stringified_json_rejected_with_strict_validation(self):
|
||||
"""Stringified JSON is still rejected for Pydantic models under strict validation."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=True)
|
||||
|
||||
@mcp.tool
|
||||
def create_user(profile: UserProfile) -> str:
|
||||
"""Create a user from a profile."""
|
||||
|
|
@ -153,6 +172,19 @@ class TestPydanticModelArguments:
|
|||
with pytest.raises(ToolError, match="validation"):
|
||||
await client.call_tool("create_user", {"profile": stringified})
|
||||
|
||||
async def test_invalid_stringified_json_still_raises_clear_error(self):
|
||||
"""A string that isn't valid JSON still surfaces the original error."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def create_user(profile: UserProfile) -> str:
|
||||
"""Create a user from a profile."""
|
||||
return f"Created user {profile.name}, age {profile.age}"
|
||||
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(ToolError, match="validation"):
|
||||
await client.call_tool("create_user", {"profile": "not json"})
|
||||
|
||||
async def test_pydantic_model_with_coercion(self):
|
||||
"""Pydantic models should benefit from coercion without strict validation."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
|
@ -202,6 +234,118 @@ class TestPydanticModelArguments:
|
|||
)
|
||||
|
||||
|
||||
class TestStructuralArgumentsAsJsonStrings:
|
||||
"""Some LLM clients serialize list/dict/set arguments as JSON strings
|
||||
instead of native JSON values. Without strict validation, FastMCP decodes
|
||||
them before validating, extending the existing scalar coercion leniency
|
||||
(e.g. ``"7"`` -> ``int``) to structural types. See #553.
|
||||
"""
|
||||
|
||||
async def test_list_argument_as_json_string(self):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def total(values: list[int]) -> int:
|
||||
return sum(values)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("total", {"values": "[1, 2, 3]"})
|
||||
assert result.data == 6
|
||||
|
||||
async def test_optional_list_argument_as_json_string(self):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def total(values: list[int] | None = None) -> int:
|
||||
return sum(values) if values else 0
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("total", {"values": "[1, 2, 3]"})
|
||||
assert result.data == 6
|
||||
|
||||
async def test_dict_argument_as_json_string(self):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def get_keys(mapping: dict) -> list[str]:
|
||||
return sorted(mapping.keys())
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("get_keys", {"mapping": '{"a": 1, "b": 2}'})
|
||||
assert result.data == ["a", "b"]
|
||||
|
||||
async def test_set_argument_as_json_string(self):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def count_unique(values: set[int]) -> int:
|
||||
return len(values)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("count_unique", {"values": "[1, 2, 2]"})
|
||||
assert result.data == 2
|
||||
|
||||
async def test_native_structural_argument_still_works(self):
|
||||
"""Passing the already-decoded value must keep working (no regression)."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def total(values: list[int]) -> int:
|
||||
return sum(values)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("total", {"values": [1, 2, 3]})
|
||||
assert result.data == 6
|
||||
|
||||
async def test_structural_argument_as_json_string_rejected_with_strict_validation(
|
||||
self,
|
||||
):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=True)
|
||||
|
||||
@mcp.tool
|
||||
def total(values: list[int]) -> int:
|
||||
return sum(values)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(ToolError, match="validation"):
|
||||
await client.call_tool("total", {"values": "[1, 2, 3]"})
|
||||
|
||||
async def test_plain_string_argument_unaffected(self):
|
||||
"""A plain ``str`` parameter must never be decoded as JSON."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def echo(value: str) -> str:
|
||||
return value
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("echo", {"value": "[1, 2, 3]"})
|
||||
assert result.data == "[1, 2, 3]"
|
||||
|
||||
async def test_optional_string_argument_keeps_literal_null_string(self):
|
||||
"""A ``str | None`` parameter must not confuse the string "null" with None."""
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def echo(value: str | None = None) -> str | None:
|
||||
return value
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("echo", {"value": "null"})
|
||||
assert result.data == "null"
|
||||
|
||||
async def test_invalid_json_string_for_list_argument_raises_clear_error(self):
|
||||
mcp = FastMCP("TestServer", strict_input_validation=False)
|
||||
|
||||
@mcp.tool
|
||||
def total(values: list[int]) -> int:
|
||||
return sum(values)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(ToolError, match="validation"):
|
||||
await client.call_tool("total", {"values": "not json"})
|
||||
|
||||
|
||||
class TestValidationErrorMessages:
|
||||
"""Test the quality of validation error messages."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue