Repository navigation
Conversation
Media types are case-insensitive (RFC 9110, section 8.3.1), and StreamableHTTPServerTransport._check_accept_headers already lowercases the Accept media types before comparing. _check_content_type did not, so a spec-valid request with a mixed/upper-case Content-Type (e.g. "Application/JSON") was rejected with 415 Unsupported Media Type. Lowercase the parsed Content-Type media type before comparing to CONTENT_TYPE_JSON, consistent with _check_accept_headers. Adds a unit test for case-insensitive matching (the _check_content_type path was previously no-cover).
FileResource.set_binary_from_mime_type decides whether to read a file as text
or bytes via `mime_type.startswith("text/")`, but media types are
case-insensitive (RFC 9110, section 8.3.1). A FileResource declared with an
upper/mixed-case text type (e.g. "Text/Markdown") was misclassified as binary
and read with read_bytes instead of read_text.
Normalize with .lower() before the prefix check, consistent with the other
media-type checks in the SDK (transport_security, client streamable_http).
Adds a regression test.
|
Thanks for catching the case-sensitivity issue, @ly-wang19. While verifying this I think I found a deeper problem in the same validator that affects this PR:
A consequence for this PR: the new test Minimal repro: from mcp.server.mcpserver.resources.types import FileResource
r = FileResource(uri="file:///x.png", name="x", path="/abs/x.png", mime_type="image/png")
print(r.is_binary) # False — expected True; read() will then read_text() a binary fileThe root-cause fix is one line — add Happy to either push that one-liner + a default-path test into this PR, or open a small complementary PR that lands first — whichever you and the maintainers prefer. Either way your case-insensitivity fix is needed; this just makes it reachable. |
|
Correction to my earlier comment: I was wrong about the root cause. After re-verification, the The real bug is solely the case-sensitivity issue that this PR addresses:
So the |
| content_type_parts = [part.strip() for part in content_type.split(";")[0].split(",")] | ||
| # Media types are case-insensitive (RFC 9110, section 8.3.1), so normalize | ||
| # to lower case before comparing — consistent with _check_accept_headers. | ||
| content_type_parts = [part.strip().lower() for part in content_type.split(";")[0].split(",")] |
There was a problem hiding this comment.
nice, I was reading through this today and it aligns with RFC 9110 to have the normalization here. I opened an issue for tracking
that you can link. Thank you
|
Thanks for the PR, and sorry it sat here without a proper review. We're closing most of the open PR backlog. v2 is out and changed a lot of the SDK, so many older PRs no longer apply as written, and we're a small team that realistically doesn't have the capacity to work through the rest. If this still matters to you on v2, the most useful thing you can do is open an issue (or comment on the existing one) with your use case and a repro. Hearing why it matters to you is what we use to decide what to prioritise. |
Summary
FileResource.set_binary_from_mime_typedecides whether to read a file as text or bytes from itsmime_typeviamime_type.startswith("text/"). Media types are case-insensitive (RFC 9110, §8.3.1), so aFileResourcedeclared with an upper/mixed-case text type — e.g.mime_type="Text/Markdown"— is misclassified as binary and read withread_bytesinstead ofread_text, returning abytesblob to clients instead of text.Fix
Lower-case the mime type before the prefix check, consistent with the SDK's other media-type checks (
transport_security, clientstreamable_http).Test
Added
test_uppercase_text_mime_type_is_treated_as_text: aFileResource(mime_type="Text/Markdown")keepsis_binaryFalse.Verified red→green: the new test fails on
main(treated as binary) and passes with the fix.uv run pytest tests/server/mcpserver/resources/test_file_resources.py→ 8 passed;ruff check/ruff format --checkclean.