FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Enhance resource template to support values with slashes (Fix #159) by g-votte · Pull Request #197 · modelcontextprotocol/python-sdk · GitHub

/ python-sdk Public

Enhance resource template to support values with slashes (Fix #159) - #197

Closed
g-votte wants to merge 2 commits into
modelcontextprotocol:mainfrom
g-votte:resource-template-support-slashes
ConversationCommits (2)ChecksFiles changed
Closed

Enhance resource template to support values with slashes (Fix #159)#197
g-votte wants to merge 2 commits into
modelcontextprotocol:mainfrom
g-votte:resource-template-support-slashes

Conversation

g-votte commented Feb 9, 2025 •
edited
Loading

Copy link
Copy Markdown

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:

Analysis

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

python-sdk/src/mcp/server/fastmcp/resources/templates.py

Line 58 in f10665d

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

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:

python-sdk/tests/issues/test_141_resource_templates.py

Lines 61 to 64 in f10665d

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.

g-votte mentioned this pull request Feb 9, 2025
Closed

ICeZer0 commented Feb 15, 2025

Copy link
Copy Markdown

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 commented Feb 20, 2025

Copy link
Copy Markdown
Member

Additional context

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

python-sdk/tests/issues/test_141_resource_templates.py

Lines 61 to 64 in f10665d

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 closed this Feb 20, 2025
maxisbey mentioned this pull request Mar 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL