Skip to content

Conversation

@g-votte
Copy link

@g-votte g-votte commented Feb 9, 2025

This PR enhances resource templates so that they can handle values containing slashes.

Motivation and Context

Motivation

This PR fixes the bug reported in #159 where the following example raises an error for paths containing slashes such as resource://content/path/with/slashes.

from mcp.server.fastmcp import FastMCP

mcp = FastMCP("My App")

@mcp.resource("resource://content/{path}")
def get_content(path: str) -> str:
    return f"Path: {path}"

Currently, this raises mcp.shared.exceptions.McpError: Unknown resource: resource://content/path/with/slashes.

Screenshot from MCP inspector:
image

Analysis

The root cause is in ResourceTemplate.matches() which explicitly excludes slashes from the regex matching:

pattern = self.uri_template.replace("{", "(?P<").replace("}", ">[^/]+)")

Content of Fix

  • Modified the regex pattern to allow slashes in parameter values
  • Added test cases in tests/server/fastmcp/resources/test_resource_template.py
  • Removed test case in tests/issues/test_141_resource_templates.py which expected errors for slash-containing values

How Has This Been Tested?

  • Unit testing: Added a test case in tests/server/fastmcp/resources/test_resource_template.py
  • Integration testing: Verified the example from the Motivation section works correctly in MCP inspector
image

Breaking Changes

While this change primarily enhances functionality without breaking existing behavior, there might be edge cases where applications rely on the current error behavior for paths containing slashes. However, such cases are expected to be rare.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

While #159 marks this as a "bug," there is an existing test case suggesting this might have been intentional behavior:

with pytest.raises(ValueError, match="Unknown resource"):
await mcp.read_resource(
"resource://users/123/posts/456/extra"
) # Extra path component

I would appreciate maintainers' input on the intended behavior. If rejecting slash-containing values is the desired design, please close this PR.

@ICeZer0
Copy link

ICeZer0 commented Feb 15, 2025

I can't get any resource params to show up in my MCP Inspector, not sure if its related to the above.

This works fine.

@mcp.resource("config://app")
def get_config() -> str:
    """Static configuration data"""
    return "App configuration here"

Once I add an input param it does not show up as a listed resource

@mcp.resource("users://{hello}/message")
def get_config(hello: str) -> str:
    """dynamic user message"""
    return "Message here {hello}"

@dsp-ant
Copy link
Member

dsp-ant commented Feb 20, 2025

Additional context

While #159 marks this as a "bug," there is an existing test case suggesting this might have been intentional behavior:

with pytest.raises(ValueError, match="Unknown resource"):
await mcp.read_resource(
"resource://users/123/posts/456/extra"
) # Extra path component

I would appreciate maintainers' input on the intended behavior. If rejecting slash-containing values is the desired design, please close this PR.

Thank you for working on this and the detailed description. After more time thinking about this, I think the current behaviour is desired. If we want to support paths we should go down the route of FastAPI which offers {var:path} convertor.

I'll close this PR, and sorry for you spending the time without it being merged. I think the detailed description made it clear how to best think about this for me. Thank you

@dsp-ant dsp-ant closed this Feb 20, 2025
gspencergoog pushed a commit to gspencergoog/mcp-python-sdk that referenced this pull request Jul 29, 2025
…/jerome/feature/progress-update-message

Added optional message field to progress notifications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants