From c8dface36ae6b006f16049fb09f5510eb8d31a28 Mon Sep 17 00:00:00 2001 From: cutekibry Date: Fri, 25 Apr 2025 15:24:12 +0800 Subject: [PATCH] fix: unintended type convert The function tries to convert `str`` into `dict` without validate if it is suitable for the type annotation. This will cause unintended type converting. Refs: #251 --- src/fastmcp/utilities/func_metadata.py | 20 +++++++-- tests/utilities/test_func_metadata.py | 56 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/fastmcp/utilities/func_metadata.py b/src/fastmcp/utilities/func_metadata.py index 777e7d744..138cb4389 100644 --- a/src/fastmcp/utilities/func_metadata.py +++ b/src/fastmcp/utilities/func_metadata.py @@ -7,7 +7,15 @@ from typing import ( ForwardRef, ) -from pydantic import BaseModel, ConfigDict, Field, WithJsonSchema, create_model +from pydantic import ( + BaseModel, + ConfigDict, + Field, + TypeAdapter, + ValidationError, + WithJsonSchema, + create_model, +) from pydantic._internal._typing_extra import eval_type_backport from pydantic.fields import FieldInfo from pydantic_core import PydanticUndefined @@ -80,14 +88,18 @@ class FuncMetadata(BaseModel): dicts (JSON objects) as JSON strings, which can be pre-parsed here. """ new_data = data.copy() # Shallow copy - for field_name, _field_info in self.arg_model.model_fields.items(): + for field_name, field_info in self.arg_model.model_fields.items(): if field_name not in data.keys(): continue if isinstance(data[field_name], str): try: pre_parsed = json.loads(data[field_name]) - except json.JSONDecodeError: - continue # Not JSON - skip + + # Check if the pre_parsed value is valid for the field + validator = TypeAdapter(field_info.annotation) + validator.validate_python(pre_parsed) + except (json.JSONDecodeError, ValidationError): + continue # Not JSON or invalid for the field if isinstance(pre_parsed, str | int | float): # This is likely that the raw value is e.g. `"hello"` which we # Should really be parsed as '"hello"' in Python - but if we parse diff --git a/tests/utilities/test_func_metadata.py b/tests/utilities/test_func_metadata.py index 96d8c6f3b..cc4b013b1 100644 --- a/tests/utilities/test_func_metadata.py +++ b/tests/utilities/test_func_metadata.py @@ -174,6 +174,62 @@ def test_str_vs_list_str(): assert result["str_or_list"] == ["hello", "world"] +def test_keep_str_as_str(): + """Test that string arguments are kept as strings""" + + def func_with_str_types(string: str): + return string + + meta = func_metadata(func_with_str_types) + result = meta.pre_parse_json( + {"string": "{'nice to meet you': 'hello', 'goodbye': 5}"} + ) + assert result["string"] == "{'nice to meet you': 'hello', 'goodbye': 5}" + + +def test_keep_str_union_as_str(): + """Test that string arguments are kept as strings""" + + def func_with_str_types(string: str | dict[int, str] | None): + return string + + meta = func_metadata(func_with_str_types) + result = meta.pre_parse_json( + {"string": "{'nice to meet you': 'hello', 'goodbye': 5}"} + ) + assert result["string"] == "{'nice to meet you': 'hello', 'goodbye': 5}" + + +def test_keep_str_complex_type_as_str(): + """Test that string arguments are kept as strings because it's invalid for the field""" + + class SomeModel(BaseModel): + x: int + y: dict[int, str] + + def func_with_str_types(string: str | SomeModel | None): + return string + + meta = func_metadata(func_with_str_types) + result = meta.pre_parse_json({"string": '{"x": 1, "y": {"invalid": "hello"}}'}) + assert result["string"] == '{"x": 1, "y": {"invalid": "hello"}}' + + +def test_convert_str_to_complex_type(): + """Test that string arguments are converted to the complex type because it's valid for the field""" + + class SomeModel(BaseModel): + x: int + y: dict[int, str] + + def func_with_str_types(string: str | SomeModel | None): + return string + + meta = func_metadata(func_with_str_types) + result = meta.pre_parse_json({"string": '{"x": 1, "y": {"1": "hello"}}'}) + assert result["string"] == {"x": 1, "y": {"1": "hello"}} + + def test_skip_names(): """Test that skipped parameters are not included in the model"""