Existing MCP servers trigger panics for resource templates#365
Merged
jba merged 5 commits intomodelcontextprotocol:mainfrom Aug 25, 2025
Merged
Existing MCP servers trigger panics for resource templates#365jba merged 5 commits intomodelcontextprotocol:mainfrom
jba merged 5 commits intomodelcontextprotocol:mainfrom
Conversation
Contributor
|
I just re-read the resources section of the spec. There is nothing about requiring absolute URIs. I think we should remove all our checks for schemes. And even if we kept the check, it's a bug to use url.Parse for it in Do you want to repurpose this PR to do this? |
Contributor
Author
|
Okay, I'll update this PR and remove the scheme check for both resources and resource templates. For resourceTemplates, I'll remove the Url parse entirely because it's not clear we should be trying to make any additional assumptions about these strings in the SDK. |
05bca41 to
9a8084d
Compare
jba
requested changes
Aug 25, 2025
| s.changeAndNotify(notificationResourceListChanged, &ResourceListChangedParams{}, | ||
| func() bool { | ||
| u, err := url.Parse(r.URI) | ||
| _, err := url.Parse(r.URI) |
Contributor
There was a problem hiding this comment.
nit: if _, err := ...; err != nil
Contributor
Author
There was a problem hiding this comment.
Oh ya! Updated that
Contributor
Author
|
@jba I think it's probably okay to just remove those failing tests since they we're not going assume anything about ResourceTemplate strings for now. |
yasomaru
pushed a commit
to yasomaru/go-sdk
that referenced
this pull request
Aug 28, 2025
remove checks for URI scheme; nothing in the spec requires one
# Background
When adding resource templates, we are currently doing checks for
schemes on absolute uris. It is still common for MCP servers to use
custom uris that would not pass these checks. For example, if the
official GitHub MCP server were to use this sdk, it would show failures
like:
```
"repo://{owner}/{repo}/contents{/path*}": parse "repo://{owner}/{repo}/contents{/path*}": invalid character "{" in host name
"repo://{owner}/{repo}/refs/heads/{branch}/contents{/path*}": parse "repo://{owner}/{repo}/refs/heads/{branch}/contents{/path*}": invalid character "{" in host name
"repo://{owner}/{repo}/sha/{sha}/contents{/path*}": parse "repo://{owner}/{repo}/sha/{sha}/contents{/path*}": invalid character "{" in host name
invalid resource template uri "repo://{owner}/{repo}/refs/pull/{prNumber}/head/contents{/path*}": parse "repo://{owner}/{repo}/refs/pull/{prNumber}/head/contents{/path*}": invalid character "{" in host name
"repo://{owner}/{repo}/refs/tags/{tag}/contents{/path*}": parse "repo://{owner}/{repo}/refs/tags/{tag}/contents{/path*}": invalid character "{" in host name
```
The wikipedia MCP would also show problems with missing schemes.
```
"/search/{query}": <nil>
"/article/{title}": <nil>
"/summary/{title}": <nil>
"/summary/{title}/query/{query}/length/{max_length}": <nil>
"/summary/{title}/section/{section_title}/length/{max_length}": <nil>
"/sections/{title}": <nil>
"/links/{title}": <nil>
"/facts/{title}/topic/{topic_within_article}/count/{count}": <nil>
"/coordinates/{title}": <nil>
```
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
When adding resource templates, we are currently doing checks for schemes on absolute uris. It is still common for MCP servers to use custom uris that would not pass these checks. For example, if the official GitHub MCP server were to use this sdk, it would show failures like:
The wikipedia MCP would also show problems with missing schemes.
What I did
I have left the URI.parse check in the AddResource but have removed the scheme check for absolute uris.
For AddResourceTemplate, I have removed the Uri.parse entirely because it's not clear we can make any assumptions about these strings.