Skip to content

Commit fd53853

Browse files
committed
fix: don't JSON pre-parse string args whose annotation only accepts str or None
pre_parse_json decided whether to json.loads a string argument with 'field_info.annotation is not str', which treats 'str | None' like a non-string annotation. A valid string value that happens to parse as a JSON object or array (e.g. a JSON-serialized template body) was silently replaced with a dict/list and then failed model validation. Only skip pre-parsing when every union member is str or None, so a plain string is already fully valid. Unions with non-str members such as 'str | list[str]' keep the existing stringified-JSON handling. Fixes #3055
1 parent 53117cb commit fd53853

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

src/mcp/server/mcpserver/utilities/func_metadata.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,25 @@ def _is_input_required_type(obj: Any) -> bool:
3333
return isinstance(obj, type) and issubclass(obj, InputRequiredResult)
3434

3535

36+
def _accepts_only_str_or_none(annotation: Any) -> bool:
37+
"""Whether every member of the annotation is `str` or `None`.
38+
39+
For such annotations a string value is already fully valid, so JSON
40+
pre-parsing could only corrupt it (e.g. `body: str | None` receiving a
41+
JSON-serialized string). Unions with non-str members (e.g. `str | list[str]`)
42+
still opt in to pre-parsing, since the string may be a stringified value
43+
for one of those members.
44+
"""
45+
if annotation is str or annotation is type(None):
46+
return True
47+
origin = get_origin(annotation)
48+
if origin is Annotated:
49+
return _accepts_only_str_or_none(get_args(annotation)[0])
50+
if is_union_origin(origin):
51+
return all(_accepts_only_str_or_none(arg) for arg in get_args(annotation))
52+
return False
53+
54+
3655
class StrictJsonSchema(GenerateJsonSchema):
3756
"""A JSON schema generator that raises exceptions instead of emitting warnings.
3857
@@ -169,7 +188,7 @@ def pre_parse_json(self, data: dict[str, Any]) -> dict[str, Any]:
169188
continue
170189

171190
field_info = key_to_field_info[data_key]
172-
if isinstance(data_value, str) and field_info.annotation is not str:
191+
if isinstance(data_value, str) and not _accepts_only_str_or_none(field_info.annotation):
173192
try:
174193
pre_parsed = json.loads(data_value)
175194
except json.JSONDecodeError:

tests/server/mcpserver/test_func_metadata.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,48 @@ def func_with_str_types(str_or_list: str | list[str]): # pragma: no cover
204204
assert result["str_or_list"] == ["hello", "world"]
205205

206206

207+
def test_optional_str_not_parsed_as_json():
208+
"""A `str | None` parameter must keep JSON-looking string values as strings.
209+
210+
Regression test for https://github.com/modelcontextprotocol/python-sdk/issues/3055:
211+
the annotation is not literally `str`, but a plain string is already fully
212+
valid for it, so pre-parsing could only corrupt the value.
213+
"""
214+
215+
def func_with_optional_str(body: str | None = None): # pragma: no cover
216+
return body
217+
218+
meta = func_metadata(func_with_optional_str)
219+
220+
json_object_str = '{"blocks": ["a", "b"]}'
221+
result = meta.pre_parse_json({"body": json_object_str})
222+
assert result["body"] == json_object_str
223+
validated = meta.arg_model.model_validate(result)
224+
assert getattr(validated, "body") == json_object_str
225+
226+
json_array_str = '["a", "b"]'
227+
result = meta.pre_parse_json({"body": json_array_str})
228+
assert result["body"] == json_array_str
229+
230+
# Annotated str members are unwrapped before the check
231+
def func_with_annotated_str(
232+
body: Annotated[str, Field(description="a body")] | None = None,
233+
): # pragma: no cover
234+
return body
235+
236+
meta = func_metadata(func_with_annotated_str)
237+
result = meta.pre_parse_json({"body": json_object_str})
238+
assert result["body"] == json_object_str
239+
240+
# A union with a non-str member still opts in to pre-parsing
241+
def func_with_str_list_none(value: str | list[str] | None = None): # pragma: no cover
242+
return value
243+
244+
meta = func_metadata(func_with_str_list_none)
245+
result = meta.pre_parse_json({"value": '["a", "b"]'})
246+
assert result["value"] == ["a", "b"]
247+
248+
207249
def test_skip_names():
208250
"""Test that skipped parameters are not included in the model"""
209251

0 commit comments

Comments
 (0)