From a7a9ab58b73b5ca1136a8f8f39691bb09df1d37b Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:32:11 +0000 Subject: [PATCH 1/3] Replace FileResource.is_binary with an encoding field FileResource guessed is_binary from mime_type with a text/* prefix check, and because False doubled as the "not given" sentinel an explicit is_binary=False could never take effect, so application/json files were always served as base64 blobs. Text reads also used Path.read_text() with no encoding, i.e. the platform locale. The field is now encoding: str | None. A string decodes the file and serves it as text; None reads bytes and serves a blob. When omitted the default comes from mime_type: the declared charset if present, else utf-8 for textual types (text/*, +json/+xml suffixes, application/json, xml, javascript), else None. Resource models now reject unknown keyword arguments, so passing the removed is_binary= fails at construction instead of being ignored. --- docs/migration.md | 25 +++ src/mcp/server/mcpserver/resources/base.py | 2 +- src/mcp/server/mcpserver/resources/types.py | 58 +++-- .../resources/test_file_resources.py | 209 +++++++++++------- 4 files changed, 194 insertions(+), 100 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index afe0953631..61656dcbeb 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -826,6 +826,31 @@ Reading a missing resource now returns JSON-RPC error code `-32602` (invalid par The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). +### `FileResource.is_binary` replaced by `encoding` + +`FileResource` used to take `is_binary: bool` and guess its default from `mime_type` (`text/*` → text, anything else → bytes). Two problems fell out of that: `is_binary=False` could not actually be set — `False` doubled as the "not given" sentinel, so `mime_type="application/json"` always came back as a base64 blob — and text reads used `Path.read_text()` with no encoding, i.e. the platform locale (cp1252 on Windows). + +The field is now `encoding: str | None`. A string means "decode with this encoding and serve as text"; `None` means "read bytes and serve as a blob". When omitted it defaults to the `charset` declared in `mime_type` if there is one, otherwise `"utf-8"` for textual mime types (`text/*`, `application/json`, `application/xml`, `application/javascript`, and any `+json`/`+xml` suffix) and `None` for everything else, so JSON and XML files are now served as text without any configuration. + +Passing the removed `is_binary=` argument raises a `ValidationError` at construction rather than being silently ignored — all `Resource` classes now reject unknown keyword arguments. + +**Before (v1):** + +```python +FileResource(uri="file:///logo.png", path=logo, mime_type="image/png", is_binary=True) +FileResource(uri="file:///notes.txt", path=notes) # text, decoded with the locale encoding +``` + +**After (v2):** + +```python +FileResource(uri="file:///logo.png", path=logo, mime_type="image/png") # bytes, from mime_type +FileResource(uri="file:///notes.txt", path=notes) # text, decoded as UTF-8 +FileResource(uri="file:///data.json", path=data, mime_type="application/json") # now text, not a blob +``` + +Pass `encoding=None` to force a blob, or `encoding="latin-1"` (etc.) to decode a text file that isn't UTF-8. + ### Resource templates: matching behavior changes Resource template matching has been rewritten with [RFC 6570](https://datatracker.ietf.org/doc/html/rfc6570) support. diff --git a/src/mcp/server/mcpserver/resources/base.py b/src/mcp/server/mcpserver/resources/base.py index f7bedf6cbe..a1d9cebf3f 100644 --- a/src/mcp/server/mcpserver/resources/base.py +++ b/src/mcp/server/mcpserver/resources/base.py @@ -16,7 +16,7 @@ class Resource(BaseModel, abc.ABC): """Base class for all resources.""" - model_config = ConfigDict(validate_default=True) + model_config = ConfigDict(validate_default=True, extra="forbid") uri: str = Field(default=..., description="URI of the resource") name: str | None = Field(description="Name of the resource", default=None) diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index e6f00dec58..bc3b198c8d 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -4,6 +4,7 @@ import json from collections.abc import Callable +from functools import partial from pathlib import Path from typing import Any @@ -13,12 +14,38 @@ import pydantic import pydantic_core from mcp_types import Annotations, Icon, InputRequiredResult -from pydantic import Field, ValidationInfo, validate_call +from pydantic import Field, validate_call from mcp.server.mcpserver.resources.base import Resource from mcp.shared._callable_inspection import is_async_callable from mcp.shared.exceptions import MCPError +_TEXTUAL_APPLICATION_TYPES = frozenset( + { + "application/json", + "application/xml", + "application/javascript", + "application/ecmascript", + } +) + + +def _default_file_encoding(mime_type: str) -> str | None: + """The encoding a file of this mime type is decoded with by default. + + A declared `charset=` parameter wins. Otherwise textual types (`text/*`, + JSON, XML, JavaScript) are UTF-8 and everything else is bytes (None). + """ + essence, *params = (part.strip() for part in mime_type.split(";")) + for param in params: + name, _, value = param.partition("=") + if name.strip().lower() == "charset" and value: + return value.strip().strip('"') + essence = essence.lower() + if essence.startswith("text/") or essence.endswith(("+json", "+xml")) or essence in _TEXTUAL_APPLICATION_TYPES: + return "utf-8" + return None + class TextResource(Resource): """A resource that reads from a string.""" @@ -122,17 +149,17 @@ def from_function( class FileResource(Resource): """A resource that reads from a file. - Set is_binary=True to read the file as binary data instead of text. + The file is decoded with `encoding` and served as text, or read as bytes and + served as a base64 blob when `encoding` is None. When `encoding` is omitted it + defaults to the `charset` declared in `mime_type`, else `"utf-8"` for textual + mime types (`text/*`, JSON, XML, JavaScript) and None for everything else; + pass it explicitly to override either way. """ path: Path = Field(description="Path to the file") - is_binary: bool = Field( - default=False, - description="Whether to read the file as binary data", - ) - mime_type: str = Field( - default="text/plain", - description="MIME type of the resource content", + encoding: str | None = Field( + default_factory=lambda data: _default_file_encoding(data["mime_type"]), + description="Text encoding used to decode the file, or None to serve its bytes as a blob", ) @pydantic.field_validator("path") @@ -143,21 +170,12 @@ def validate_absolute_path(cls, path: Path) -> Path: raise ValueError("Path must be absolute") return path - @pydantic.field_validator("is_binary") - @classmethod - def set_binary_from_mime_type(cls, is_binary: bool, info: ValidationInfo) -> bool: - """Set is_binary based on mime_type if not explicitly set.""" - if is_binary: - return True - mime_type = info.data.get("mime_type", "text/plain") - return not mime_type.startswith("text/") - async def read(self) -> str | bytes: """Read the file content.""" try: - if self.is_binary: + if self.encoding is None: return await anyio.to_thread.run_sync(self.path.read_bytes) - return await anyio.to_thread.run_sync(self.path.read_text) + return await anyio.to_thread.run_sync(partial(self.path.read_text, encoding=self.encoding)) except Exception as e: raise ValueError(f"Error reading file {self.path}: {e}") diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index 94885113a9..b3753dd2ed 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -3,6 +3,7 @@ from tempfile import NamedTemporaryFile import pytest +from pydantic import ValidationError from mcp.server.mcpserver.resources import FileResource @@ -24,93 +25,143 @@ def temp_file(): pass # File was already deleted by the test -class TestFileResource: - """Test FileResource functionality.""" +def test_file_resource_creation(temp_file: Path): + resource = FileResource( + uri=temp_file.as_uri(), + name="test", + description="test file", + path=temp_file, + ) + assert str(resource.uri) == temp_file.as_uri() + assert resource.name == "test" + assert resource.description == "test file" + assert resource.mime_type == "text/plain" + assert resource.path == temp_file + assert resource.encoding == "utf-8" - def test_file_resource_creation(self, temp_file: Path): - """Test creating a FileResource.""" - resource = FileResource( - uri=temp_file.as_uri(), - name="test", - description="test file", - path=temp_file, - ) - assert str(resource.uri) == temp_file.as_uri() - assert resource.name == "test" - assert resource.description == "test file" - assert resource.mime_type == "text/plain" # default - assert resource.path == temp_file - assert resource.is_binary is False # default - - def test_file_resource_str_path_conversion(self, temp_file: Path): - """Test FileResource handles string paths.""" - resource = FileResource( - uri=f"file://{temp_file}", - name="test", - path=Path(str(temp_file)), - ) - assert isinstance(resource.path, Path) - assert resource.path.is_absolute() - @pytest.mark.anyio - async def test_read_text_file(self, temp_file: Path): - """Test reading a text file.""" - resource = FileResource( - uri=f"file://{temp_file}", +def test_file_resource_str_path_conversion(temp_file: Path): + resource = FileResource( + uri=f"file://{temp_file}", + name="test", + path=Path(str(temp_file)), + ) + assert isinstance(resource.path, Path) + assert resource.path.is_absolute() + + +@pytest.mark.anyio +async def test_read_text_file(temp_file: Path): + resource = FileResource( + uri=f"file://{temp_file}", + name="test", + path=temp_file, + ) + content = await resource.read() + assert content == "test content" + assert resource.mime_type == "text/plain" + + +@pytest.mark.anyio +async def test_encoding_none_reads_bytes(temp_file: Path): + resource = FileResource( + uri=f"file://{temp_file}", + name="test", + path=temp_file, + encoding=None, + ) + content = await resource.read() + assert isinstance(content, bytes) + assert content == b"test content" + + +@pytest.mark.parametrize( + "mime_type", + [ + "text/plain", + "text/html; charset=utf-8", + "application/json", + "application/xml", + "application/vnd.api+json", + "image/svg+xml", + "application/javascript", + ], +) +def test_textual_mime_types_default_to_utf8(temp_file: Path, mime_type: str): + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type) + assert resource.encoding == "utf-8" + + +@pytest.mark.parametrize("mime_type", ["image/png", "application/octet-stream", "application/pdf"]) +def test_binary_mime_types_default_to_no_encoding(temp_file: Path, mime_type: str): + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type) + assert resource.encoding is None + + +@pytest.mark.parametrize( + "mime_type", + ["text/plain; charset=iso-8859-1", 'text/plain; format=flowed; charset="iso-8859-1"'], +) +def test_declared_charset_becomes_default_encoding(temp_file: Path, mime_type: str): + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type) + assert resource.encoding == "iso-8859-1" + + +def test_removed_is_binary_kwarg_is_rejected(temp_file: Path): + """The v1 `is_binary` parameter fails loudly at construction rather than being ignored.""" + with pytest.raises(ValidationError, match="is_binary"): + FileResource.model_validate({"uri": temp_file.as_uri(), "path": temp_file, "is_binary": True}) + + +@pytest.mark.anyio +async def test_json_file_is_served_as_text_by_default(temp_file: Path): + """The mime type that motivated the encoding field: JSON must not become a base64 blob.""" + temp_file.write_text('{"a": 1}', encoding="utf-8") + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="application/json") + assert await resource.read() == '{"a": 1}' + + +@pytest.mark.anyio +async def test_explicit_encoding_overrides_default(temp_file: Path): + """An explicit encoding wins over the mime-type default and is what decodes the file.""" + temp_file.write_bytes("naïve".encode("latin-1")) + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="image/png", encoding="latin-1") + assert resource.encoding == "latin-1" + assert await resource.read() == "naïve" + + +def test_relative_path_error(): + with pytest.raises(ValueError, match="Path must be absolute"): + FileResource( + uri="file:///test.txt", name="test", - path=temp_file, + path=Path("test.txt"), ) - content = await resource.read() - assert content == "test content" - assert resource.mime_type == "text/plain" - @pytest.mark.anyio - async def test_read_binary_file(self, temp_file: Path): - """Test reading a file as binary.""" + +@pytest.mark.anyio +async def test_missing_file_error(temp_file: Path): + missing = temp_file.parent / "missing.txt" + resource = FileResource( + uri="file:///missing.txt", + name="test", + path=missing, + ) + with pytest.raises(ValueError, match="Error reading file"): + await resource.read() + + +@pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows") +@pytest.mark.anyio +async def test_permission_error(temp_file: Path): # pragma: lax no cover + temp_file.chmod(0o000) # Remove all permissions + try: resource = FileResource( - uri=f"file://{temp_file}", + uri=temp_file.as_uri(), name="test", path=temp_file, - is_binary=True, - ) - content = await resource.read() - assert isinstance(content, bytes) - assert content == b"test content" - - def test_relative_path_error(self): - """Test error on relative path.""" - with pytest.raises(ValueError, match="Path must be absolute"): - FileResource( - uri="file:///test.txt", - name="test", - path=Path("test.txt"), - ) - - @pytest.mark.anyio - async def test_missing_file_error(self, temp_file: Path): - """Test error when file doesn't exist.""" - # Create path to non-existent file - missing = temp_file.parent / "missing.txt" - resource = FileResource( - uri="file:///missing.txt", - name="test", - path=missing, ) with pytest.raises(ValueError, match="Error reading file"): await resource.read() - - @pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows") - @pytest.mark.anyio - async def test_permission_error(self, temp_file: Path): # pragma: lax no cover - """Test reading a file without permissions.""" - temp_file.chmod(0o000) # Remove all permissions - try: - resource = FileResource( - uri=temp_file.as_uri(), - name="test", - path=temp_file, - ) - with pytest.raises(ValueError, match="Error reading file"): - await resource.read() - finally: - temp_file.chmod(0o644) # Restore permissions + finally: + temp_file.chmod(0o644) # Restore permissions From 769fd84c52d1ba85512cf775cda82d68799e3df7 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:11:10 +0000 Subject: [PATCH 2/3] Shrink the textual mime set and default text to utf-8-sig The set of application/* types treated as text is now just json and xml, the two ubiquitous un-suffixed textual types the structural rules (text/*, +json/+xml) can't reach. application/javascript and application/ecmascript are gone: RFC 9239 made text/javascript canonical, which the text/* rule already covers. The inferred text encoding is now utf-8-sig, so a byte-order mark is dropped rather than surfacing as a leading U+FEFF that JSON parsers reject. A declared charset= is still used as-is. The encoding is validated with codecs.lookup at construction, so a typo fails at registration instead of on the first read. The extra="forbid" change gets its own migration section, since it applies to every Resource class, not just FileResource. --- docs/migration.md | 12 +++++-- src/mcp/server/mcpserver/resources/types.py | 36 +++++++++++-------- .../resources/test_file_resources.py | 29 ++++++++++++--- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 61656dcbeb..38d4fd408b 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -826,13 +826,19 @@ Reading a missing resource now returns JSON-RPC error code `-32602` (invalid par The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). +### `Resource` classes reject unknown keyword arguments + +The `Resource` base class now sets `extra="forbid"`, so every resource class — `TextResource`, `BinaryResource`, `FunctionResource`, `FileResource`, `HttpResource`, `DirectoryResource`, and your own subclasses — raises `ValidationError` on an unrecognised keyword argument instead of silently dropping it. Previously a typo'd or since-removed parameter (such as `FileResource(is_binary=...)`, below) was accepted and ignored. Remove any stray keyword arguments; if a subclass needs to accept arbitrary extras, set its own `model_config = ConfigDict(extra="allow")`. + ### `FileResource.is_binary` replaced by `encoding` `FileResource` used to take `is_binary: bool` and guess its default from `mime_type` (`text/*` → text, anything else → bytes). Two problems fell out of that: `is_binary=False` could not actually be set — `False` doubled as the "not given" sentinel, so `mime_type="application/json"` always came back as a base64 blob — and text reads used `Path.read_text()` with no encoding, i.e. the platform locale (cp1252 on Windows). -The field is now `encoding: str | None`. A string means "decode with this encoding and serve as text"; `None` means "read bytes and serve as a blob". When omitted it defaults to the `charset` declared in `mime_type` if there is one, otherwise `"utf-8"` for textual mime types (`text/*`, `application/json`, `application/xml`, `application/javascript`, and any `+json`/`+xml` suffix) and `None` for everything else, so JSON and XML files are now served as text without any configuration. +The field is now `encoding: str | None`. A string means "decode with this encoding and serve as text"; `None` means "read bytes and serve as a blob". When omitted it defaults to the `charset` declared in `mime_type` if there is one, otherwise `"utf-8-sig"` for textual mime types (`text/*`, `application/json`, `application/xml`, and any `+json`/`+xml` suffix) and `None` for everything else, so JSON and XML files are now served as text without any configuration. `utf-8-sig` is plain UTF-8 that also drops a byte-order mark if the file has one; a declared `charset=` is used as-is. + +Passing the removed `is_binary=` argument now raises a `ValidationError` at construction (see the section above) rather than being silently ignored. A misspelled `encoding` also fails at construction rather than on the first read. -Passing the removed `is_binary=` argument raises a `ValidationError` at construction rather than being silently ignored — all `Resource` classes now reject unknown keyword arguments. +One case flips the other way: a non-UTF-8 file with a textual mime type (say UTF-16 XML) previously shipped byte-exact as a blob and now fails to decode. Set `encoding` to the file's real encoding, or `encoding=None` to keep serving it as a blob. **Before (v1):** @@ -845,7 +851,7 @@ FileResource(uri="file:///notes.txt", path=notes) # text, decoded with the loca ```python FileResource(uri="file:///logo.png", path=logo, mime_type="image/png") # bytes, from mime_type -FileResource(uri="file:///notes.txt", path=notes) # text, decoded as UTF-8 +FileResource(uri="file:///notes.txt", path=notes) # text, decoded as UTF-8 (BOM tolerated) FileResource(uri="file:///data.json", path=data, mime_type="application/json") # now text, not a blob ``` diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index bc3b198c8d..910b8237c7 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -2,6 +2,7 @@ from __future__ import annotations +import codecs import json from collections.abc import Callable from functools import partial @@ -20,21 +21,17 @@ from mcp.shared._callable_inspection import is_async_callable from mcp.shared.exceptions import MCPError -_TEXTUAL_APPLICATION_TYPES = frozenset( - { - "application/json", - "application/xml", - "application/javascript", - "application/ecmascript", - } -) +# `application/*` types that are textual but predate the `+json`/`+xml` +# structured-syntax suffixes, so the suffix rule below can't catch them. +_TEXTUAL_APPLICATION_TYPES = frozenset({"application/json", "application/xml"}) def _default_file_encoding(mime_type: str) -> str | None: """The encoding a file of this mime type is decoded with by default. - A declared `charset=` parameter wins. Otherwise textual types (`text/*`, - JSON, XML, JavaScript) are UTF-8 and everything else is bytes (None). + A declared `charset=` parameter wins. Otherwise textual types (`text/*`, JSON, + XML) are `utf-8-sig` — UTF-8 that also tolerates a byte-order mark — and + everything else is bytes (None). """ essence, *params = (part.strip() for part in mime_type.split(";")) for param in params: @@ -43,7 +40,7 @@ def _default_file_encoding(mime_type: str) -> str | None: return value.strip().strip('"') essence = essence.lower() if essence.startswith("text/") or essence.endswith(("+json", "+xml")) or essence in _TEXTUAL_APPLICATION_TYPES: - return "utf-8" + return "utf-8-sig" return None @@ -151,9 +148,9 @@ class FileResource(Resource): The file is decoded with `encoding` and served as text, or read as bytes and served as a base64 blob when `encoding` is None. When `encoding` is omitted it - defaults to the `charset` declared in `mime_type`, else `"utf-8"` for textual - mime types (`text/*`, JSON, XML, JavaScript) and None for everything else; - pass it explicitly to override either way. + defaults to the `charset` declared in `mime_type`, else `"utf-8-sig"` for + textual mime types (`text/*`, JSON, XML) and None for everything else; pass + it explicitly to override either way. """ path: Path = Field(description="Path to the file") @@ -170,6 +167,17 @@ def validate_absolute_path(cls, path: Path) -> Path: raise ValueError("Path must be absolute") return path + @pydantic.field_validator("encoding") + @classmethod + def validate_known_encoding(cls, encoding: str | None) -> str | None: + """Ensure the encoding names a known codec, so a typo fails at construction not at read.""" + if encoding is not None: + try: + codecs.lookup(encoding) + except LookupError as e: + raise ValueError(f"unknown encoding: {encoding}") from e + return encoding + async def read(self) -> str | bytes: """Read the file content.""" try: diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index b3753dd2ed..d494f2f3e0 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -1,3 +1,4 @@ +import codecs import os from pathlib import Path from tempfile import NamedTemporaryFile @@ -37,7 +38,7 @@ def test_file_resource_creation(temp_file: Path): assert resource.description == "test file" assert resource.mime_type == "text/plain" assert resource.path == temp_file - assert resource.encoding == "utf-8" + assert resource.encoding == "utf-8-sig" def test_file_resource_str_path_conversion(temp_file: Path): @@ -79,17 +80,16 @@ async def test_encoding_none_reads_bytes(temp_file: Path): "mime_type", [ "text/plain", - "text/html; charset=utf-8", + "text/html", "application/json", "application/xml", "application/vnd.api+json", "image/svg+xml", - "application/javascript", ], ) -def test_textual_mime_types_default_to_utf8(temp_file: Path, mime_type: str): +def test_textual_mime_types_default_to_utf8_sig(temp_file: Path, mime_type: str): resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type) - assert resource.encoding == "utf-8" + assert resource.encoding == "utf-8-sig" @pytest.mark.parametrize("mime_type", ["image/png", "application/octet-stream", "application/pdf"]) @@ -113,6 +113,17 @@ def test_removed_is_binary_kwarg_is_rejected(temp_file: Path): FileResource.model_validate({"uri": temp_file.as_uri(), "path": temp_file, "is_binary": True}) +def test_unknown_encoding_is_rejected(temp_file: Path): + """A codec typo fails at construction rather than on the first read.""" + with pytest.raises(ValidationError, match="unknown encoding: not-a-codec"): + FileResource(uri=temp_file.as_uri(), path=temp_file, encoding="not-a-codec") + + +def test_unknown_declared_charset_is_rejected(temp_file: Path): + with pytest.raises(ValidationError, match="unknown encoding"): + FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="text/plain; charset=not-a-codec") + + @pytest.mark.anyio async def test_json_file_is_served_as_text_by_default(temp_file: Path): """The mime type that motivated the encoding field: JSON must not become a base64 blob.""" @@ -121,6 +132,14 @@ async def test_json_file_is_served_as_text_by_default(temp_file: Path): assert await resource.read() == '{"a": 1}' +@pytest.mark.anyio +async def test_utf8_bom_is_stripped_by_default(temp_file: Path): + """The default utf-8-sig decoding drops a byte-order mark that would otherwise break JSON parsers.""" + temp_file.write_bytes(codecs.BOM_UTF8 + b'{"a": 1}') + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="application/json") + assert await resource.read() == '{"a": 1}' + + @pytest.mark.anyio async def test_explicit_encoding_overrides_default(temp_file: Path): """An explicit encoding wins over the mime-type default and is what decodes the file.""" From 0c7c3339920b5be77f0559dffeeb26f5c202cc8b Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:25:45 +0000 Subject: [PATCH 3/3] Reject non-text codecs and clarify the migration edge cases Validate encoding by decoding a probe byte instead of codecs.lookup, so registered-but-not-text codecs (base64_codec, rot13, hex) are rejected at construction alongside unknown names, rather than passing validation and failing on read. Scope the migration guide's "flips the other way" example to a newly textual type (application/xml) - text/* files were never blobs in v1 - and call out separately that text/* files previously decoded with the platform locale rather than UTF-8. --- docs/migration.md | 2 +- src/mcp/server/mcpserver/resources/types.py | 13 ++++++++----- .../mcpserver/resources/test_file_resources.py | 12 ++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/docs/migration.md b/docs/migration.md index 38d4fd408b..408dcf57bd 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -838,7 +838,7 @@ The field is now `encoding: str | None`. A string means "decode with this encodi Passing the removed `is_binary=` argument now raises a `ValidationError` at construction (see the section above) rather than being silently ignored. A misspelled `encoding` also fails at construction rather than on the first read. -One case flips the other way: a non-UTF-8 file with a textual mime type (say UTF-16 XML) previously shipped byte-exact as a blob and now fails to decode. Set `encoding` to the file's real encoding, or `encoding=None` to keep serving it as a blob. +Two edge cases to check. A non-UTF-8 file with a *newly* textual mime type (say a UTF-16 `application/xml`) previously shipped byte-exact as a blob and now fails to decode. And an existing `text/*` file that was only readable through your platform's locale encoding (v1 decoded these with the locale, not UTF-8) now fails too. In both cases set `encoding` to the file's real encoding, or `encoding=None` to serve the bytes as a blob. **Before (v1):** diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index 910b8237c7..2edf342337 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -2,7 +2,6 @@ from __future__ import annotations -import codecs import json from collections.abc import Callable from functools import partial @@ -169,13 +168,17 @@ def validate_absolute_path(cls, path: Path) -> Path: @pydantic.field_validator("encoding") @classmethod - def validate_known_encoding(cls, encoding: str | None) -> str | None: - """Ensure the encoding names a known codec, so a typo fails at construction not at read.""" + def validate_text_encoding(cls, encoding: str | None) -> str | None: + """Ensure the encoding names a usable text codec, so a mistake fails at construction not at read.""" if encoding is not None: + # Decoding a probe byte rejects both unknown names and codecs that + # aren't text encodings (base64_codec, rot13, ...) via LookupError. try: - codecs.lookup(encoding) + b"x".decode(encoding) except LookupError as e: - raise ValueError(f"unknown encoding: {encoding}") from e + raise ValueError(str(e)) from e + except UnicodeError: + pass # a real text encoding; the probe byte just doesn't decode in it return encoding async def read(self) -> str | bytes: diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index d494f2f3e0..042ea422aa 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -124,6 +124,18 @@ def test_unknown_declared_charset_is_rejected(temp_file: Path): FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="text/plain; charset=not-a-codec") +def test_multibyte_encoding_is_accepted(temp_file: Path): + """UTF-16 can't decode a lone probe byte but is still a valid text encoding.""" + resource = FileResource(uri=temp_file.as_uri(), path=temp_file, encoding="utf-16") + assert resource.encoding == "utf-16" + + +def test_non_text_codec_is_rejected(temp_file: Path): + """A registered codec that isn't a text encoding (bytes-to-bytes) is not a usable encoding.""" + with pytest.raises(ValidationError, match="not a text encoding"): + FileResource(uri=temp_file.as_uri(), path=temp_file, encoding="base64_codec") + + @pytest.mark.anyio async def test_json_file_is_served_as_text_by_default(temp_file: Path): """The mime type that motivated the encoding field: JSON must not become a base64 blob."""