diff --git a/docs/patterns/openapi.mdx b/docs/patterns/openapi.mdx index 4d37e9cdb..fb2524446 100644 --- a/docs/patterns/openapi.mdx +++ b/docs/patterns/openapi.mdx @@ -106,6 +106,38 @@ mcp = await FastMCP.from_openapi( - It sends the request through the provided httpx client - It translates the HTTP response to the appropriate MCP format +### Request Parameter Handling + +FastMCP carefully handles different types of parameters in OpenAPI requests: + +#### Query Parameters + +By default, FastMCP will only include query parameters that have non-empty values. Parameters with `None` values or empty strings (`""`) are automatically filtered out of requests. This ensures that API servers don't receive unnecessary empty parameters that might cause issues. + +For example, if you call a tool with these parameters: +```python +await client.call_tool("search_products", { + "category": "electronics", # Will be included + "min_price": 100, # Will be included + "max_price": None, # Will be excluded + "brand": "", # Will be excluded +}) +``` + +The resulting HTTP request will only include `category=electronics&min_price=100`. + +#### Path Parameters + +For path parameters, which are typically required by REST APIs, FastMCP filters out `None` values and checks that all required path parameters are provided. If a required path parameter is missing or `None`, an error will be raised. + +```python +# This will work +await client.call_tool("get_product", {"product_id": 123}) + +# This will raise ValueError: "Missing required path parameters: {'product_id'}" +await client.call_tool("get_product", {"product_id": None}) +``` + ## Complete Example ```python [expandable] diff --git a/src/fastmcp/server/openapi.py b/src/fastmcp/server/openapi.py index ce26dd879..fd483e399 100644 --- a/src/fastmcp/server/openapi.py +++ b/src/fastmcp/server/openapi.py @@ -149,19 +149,37 @@ class OpenAPITool(Tool): path = self._route.path # Replace path parameters with values from kwargs + # Path parameters should never be None as they're typically required + # but we'll handle that case anyway path_params = { p.name: kwargs.get(p.name) for p in self._route.parameters if p.location == "path" + and p.name in kwargs + and kwargs.get(p.name) is not None } + + # Ensure all path parameters are provided + required_path_params = { + p.name + for p in self._route.parameters + if p.location == "path" and p.required + } + missing_params = required_path_params - path_params.keys() + if missing_params: + raise ValueError(f"Missing required path parameters: {missing_params}") + for param_name, param_value in path_params.items(): path = path.replace(f"{{{param_name}}}", str(param_value)) - # Prepare query parameters + # Prepare query parameters - filter out None and empty strings query_params = { p.name: kwargs.get(p.name) for p in self._route.parameters - if p.location == "query" and p.name in kwargs + if p.location == "query" + and p.name in kwargs + and kwargs.get(p.name) is not None + and kwargs.get(p.name) != "" } # Prepare headers - fix typing by ensuring all values are strings @@ -312,9 +330,18 @@ class OpenAPIResource(Resource): for param_name, param_value in path_params.items(): path = path.replace(f"{{{param_name}}}", str(param_value)) + # Filter any query parameters - get query parameters and filter out None/empty values + query_params = {} + for param in self._route.parameters: + if param.location == "query" and hasattr(self, f"_{param.name}"): + value = getattr(self, f"_{param.name}") + if value is not None and value != "": + query_params[param.name] = value + response = await self._client.request( method=self._route.method, url=path, + params=query_params, timeout=self._timeout, ) diff --git a/tests/server/test_openapi.py b/tests/server/test_openapi.py index 8ef8c0560..430e4f2ad 100644 --- a/tests/server/test_openapi.py +++ b/tests/server/test_openapi.py @@ -14,6 +14,7 @@ from pydantic.networks import AnyUrl from fastmcp import FastMCP from fastmcp.client import Client +from fastmcp.exceptions import ClientError from fastmcp.server.openapi import ( FastMCPOpenAPI, OpenAPIResource, @@ -53,6 +54,22 @@ def fastapi_app(users_db: dict[int, User]) -> FastAPI: """Get all users.""" return sorted(users_db.values(), key=lambda x: x.id) + @app.get("/search", tags=["search"]) + async def search_users( + name: str | None = None, active: bool | None = None, min_id: int | None = None + ) -> list[User]: + """Search users with optional filters.""" + results = list(users_db.values()) + + if name is not None: + results = [u for u in results if name.lower() in u.name.lower()] + if active is not None: + results = [u for u in results if u.active == active] + if min_id is not None: + results = [u for u in results if u.id >= min_id] + + return sorted(results, key=lambda x: x.id) + @app.get("/users/{user_id}", tags=["users", "detail"]) async def get_user(user_id: int) -> User | None: """Get a user by ID.""" @@ -304,7 +321,7 @@ class TestResources: """ async with Client(fastmcp_openapi_server) as client: resources = await client.list_resources() - assert len(resources) == 3 + assert len(resources) == 4 assert resources[0].uri == AnyUrl("resource://openapi/get_users_users_get") assert resources[0].name == "get_users_users_get" @@ -904,7 +921,7 @@ class TestMountFastMCP: # Check that resources are available with prefixed URIs async with Client(mcp) as client: resources = await client.list_resources() - assert len(resources) == 3 + assert len(resources) == 4 # Updated to account for new search endpoint # We're checking the key used by mcp to store the resource # The prefixed URI is used as the key, but the resource's original uri is preserved prefixed_uri = "fastapi+resource://openapi/get_users_users_get" @@ -932,3 +949,88 @@ class TestMountFastMCP: async with Client(mcp) as client: prompts = await client.list_prompts() assert len(prompts) == 0 + + +async def test_empty_query_parameters_not_sent( + fastapi_app: FastAPI, api_client: httpx.AsyncClient +): + """Test that empty and None query parameters are not sent in the request.""" + + # Create a TransportAdapter to track requests + class RequestCapture(httpx.AsyncBaseTransport): + def __init__(self, wrapped): + self.wrapped = wrapped + self.requests = [] + + async def handle_async_request(self, request): + self.requests.append(request) + return await self.wrapped.handle_async_request(request) + + # Use our transport adapter to wrap the original one + capture = RequestCapture(api_client._transport) + api_client._transport = capture + + # Create the OpenAPI server with new route map to make search endpoint a tool + openapi_spec = fastapi_app.openapi() + mcp_server = FastMCPOpenAPI( + openapi_spec=openapi_spec, + client=api_client, + route_maps=[ + RouteMap(methods=["GET"], pattern=r".*", route_type=RouteType.TOOL) + ], + ) + + # Call the search tool with mixed parameter values + async with Client(mcp_server) as client: + await client.call_tool( + "search_users_search_get", + { + "name": "", # Empty string should be excluded + "active": None, # None should be excluded + "min_id": 2, # Has value, should be included + }, + ) + + # Verify that the request URL only has min_id parameter + assert len(capture.requests) > 0 + request = capture.requests[-1] # Get the last request + + # URL should only contain min_id=2, not name= or active= + url = str(request.url) + assert "min_id=2" in url, f"URL should contain min_id=2, got: {url}" + assert "name=" not in url, f"URL should not contain name=, got: {url}" + assert "active=" not in url, f"URL should not contain active=, got: {url}" + + # More direct check - parse the URL to examine query params + from urllib.parse import parse_qs, urlparse + + parsed_url = urlparse(url) + query_params = parse_qs(parsed_url.query) + + assert "min_id" in query_params + assert "name" not in query_params + assert "active" not in query_params + + +async def test_none_path_parameters_rejected( + fastapi_app: FastAPI, api_client: httpx.AsyncClient +): + """Test that None values for path parameters are properly rejected.""" + # Create the OpenAPI server + openapi_spec = fastapi_app.openapi() + mcp_server = FastMCPOpenAPI( + openapi_spec=openapi_spec, + client=api_client, + ) + + # Create a client and try to call a tool with a None path parameter + async with Client(mcp_server) as client: + # get_user has a required path parameter user_id + with pytest.raises(ClientError, match="Missing required path parameters"): + await client.call_tool( + "update_user_name_users__user_id__name_patch", + { + "user_id": None, # This should cause an error + "name": "New Name", + }, + )