diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index be4afb4e9..26aae518e 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -33,6 +33,23 @@ def _is_input_required_type(obj: Any) -> bool: return isinstance(obj, type) and issubclass(obj, InputRequiredResult) +def _is_optional_str(annotation: Any) -> bool: + """Whether `annotation` is `str | None` (`Optional[str]`), modulo `None`. + + Used by `pre_parse_json` to skip JSON pre-parsing for such annotations: + unlike e.g. `str | list[str]`, no member of this union other than `str` + could ever be produced by decoding a string as JSON, so attempting to + parse it only risks corrupting a valid string value that happens to look + like a JSON object/array (e.g. a JSON-serialized payload passed as a + plain string parameter). + """ + origin = get_origin(annotation) + if not is_union_origin(origin): + return False + non_none_args = [arg for arg in get_args(annotation) if arg is not type(None)] + return non_none_args == [str] + + class StrictJsonSchema(GenerateJsonSchema): """A JSON schema generator that raises exceptions instead of emitting warnings. @@ -169,7 +186,11 @@ def pre_parse_json(self, data: dict[str, Any]) -> dict[str, Any]: continue field_info = key_to_field_info[data_key] - if isinstance(data_value, str) and field_info.annotation is not str: + if ( + isinstance(data_value, str) + and field_info.annotation is not str + and not _is_optional_str(field_info.annotation) + ): try: pre_parsed = json.loads(data_value) except json.JSONDecodeError: diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index 62a9612b9..2c8cd8e49 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -204,6 +204,35 @@ def func_with_str_types(str_or_list: str | list[str]): # pragma: no cover assert result["str_or_list"] == ["hello", "world"] +def test_optional_str_is_never_json_pre_parsed(): + """`str | None` must never be JSON-decoded, even if the value looks like a + JSON object/array (e.g. a JSON-serialized payload passed as a plain string). + + Regression test: `field_info.annotation is not str` used to be True for + `str | None`, so a value like a JSON-serialized template body would get + silently turned into a dict and fail pydantic validation downstream. + """ + + def func_with_optional_str(body: str | None = None): # pragma: no cover + return body + + meta = func_metadata(func_with_optional_str) + + json_payload = '{"blocks": [{"type": "text", "value": "hi"}]}' + result = meta.pre_parse_json({"body": json_payload}) + assert result["body"] == json_payload + + json_list_payload = '["hello", "world"]' + result = meta.pre_parse_json({"body": json_list_payload}) + assert result["body"] == json_list_payload + + # Plain strings and None are unaffected. + result = meta.pre_parse_json({"body": "hello"}) + assert result["body"] == "hello" + result = meta.pre_parse_json({"body": None}) + assert result["body"] is None + + def test_skip_names(): """Test that skipped parameters are not included in the model"""