Skip to content

Commit a7a9ab5

Browse files
committed
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.
1 parent 0cb920f commit a7a9ab5

4 files changed

Lines changed: 194 additions & 100 deletions

File tree

docs/migration.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,31 @@ Reading a missing resource now returns JSON-RPC error code `-32602` (invalid par
826826

827827
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`).
828828

829+
### `FileResource.is_binary` replaced by `encoding`
830+
831+
`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).
832+
833+
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.
834+
835+
Passing the removed `is_binary=` argument raises a `ValidationError` at construction rather than being silently ignored — all `Resource` classes now reject unknown keyword arguments.
836+
837+
**Before (v1):**
838+
839+
```python
840+
FileResource(uri="file:///logo.png", path=logo, mime_type="image/png", is_binary=True)
841+
FileResource(uri="file:///notes.txt", path=notes) # text, decoded with the locale encoding
842+
```
843+
844+
**After (v2):**
845+
846+
```python
847+
FileResource(uri="file:///logo.png", path=logo, mime_type="image/png") # bytes, from mime_type
848+
FileResource(uri="file:///notes.txt", path=notes) # text, decoded as UTF-8
849+
FileResource(uri="file:///data.json", path=data, mime_type="application/json") # now text, not a blob
850+
```
851+
852+
Pass `encoding=None` to force a blob, or `encoding="latin-1"` (etc.) to decode a text file that isn't UTF-8.
853+
829854
### Resource templates: matching behavior changes
830855

831856
Resource template matching has been rewritten with [RFC 6570](https://datatracker.ietf.org/doc/html/rfc6570) support.

src/mcp/server/mcpserver/resources/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
class Resource(BaseModel, abc.ABC):
1717
"""Base class for all resources."""
1818

19-
model_config = ConfigDict(validate_default=True)
19+
model_config = ConfigDict(validate_default=True, extra="forbid")
2020

2121
uri: str = Field(default=..., description="URI of the resource")
2222
name: str | None = Field(description="Name of the resource", default=None)

src/mcp/server/mcpserver/resources/types.py

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import json
66
from collections.abc import Callable
7+
from functools import partial
78
from pathlib import Path
89
from typing import Any
910

@@ -13,12 +14,38 @@
1314
import pydantic
1415
import pydantic_core
1516
from mcp_types import Annotations, Icon, InputRequiredResult
16-
from pydantic import Field, ValidationInfo, validate_call
17+
from pydantic import Field, validate_call
1718

1819
from mcp.server.mcpserver.resources.base import Resource
1920
from mcp.shared._callable_inspection import is_async_callable
2021
from mcp.shared.exceptions import MCPError
2122

23+
_TEXTUAL_APPLICATION_TYPES = frozenset(
24+
{
25+
"application/json",
26+
"application/xml",
27+
"application/javascript",
28+
"application/ecmascript",
29+
}
30+
)
31+
32+
33+
def _default_file_encoding(mime_type: str) -> str | None:
34+
"""The encoding a file of this mime type is decoded with by default.
35+
36+
A declared `charset=` parameter wins. Otherwise textual types (`text/*`,
37+
JSON, XML, JavaScript) are UTF-8 and everything else is bytes (None).
38+
"""
39+
essence, *params = (part.strip() for part in mime_type.split(";"))
40+
for param in params:
41+
name, _, value = param.partition("=")
42+
if name.strip().lower() == "charset" and value:
43+
return value.strip().strip('"')
44+
essence = essence.lower()
45+
if essence.startswith("text/") or essence.endswith(("+json", "+xml")) or essence in _TEXTUAL_APPLICATION_TYPES:
46+
return "utf-8"
47+
return None
48+
2249

2350
class TextResource(Resource):
2451
"""A resource that reads from a string."""
@@ -122,17 +149,17 @@ def from_function(
122149
class FileResource(Resource):
123150
"""A resource that reads from a file.
124151
125-
Set is_binary=True to read the file as binary data instead of text.
152+
The file is decoded with `encoding` and served as text, or read as bytes and
153+
served as a base64 blob when `encoding` is None. When `encoding` is omitted it
154+
defaults to the `charset` declared in `mime_type`, else `"utf-8"` for textual
155+
mime types (`text/*`, JSON, XML, JavaScript) and None for everything else;
156+
pass it explicitly to override either way.
126157
"""
127158

128159
path: Path = Field(description="Path to the file")
129-
is_binary: bool = Field(
130-
default=False,
131-
description="Whether to read the file as binary data",
132-
)
133-
mime_type: str = Field(
134-
default="text/plain",
135-
description="MIME type of the resource content",
160+
encoding: str | None = Field(
161+
default_factory=lambda data: _default_file_encoding(data["mime_type"]),
162+
description="Text encoding used to decode the file, or None to serve its bytes as a blob",
136163
)
137164

138165
@pydantic.field_validator("path")
@@ -143,21 +170,12 @@ def validate_absolute_path(cls, path: Path) -> Path:
143170
raise ValueError("Path must be absolute")
144171
return path
145172

146-
@pydantic.field_validator("is_binary")
147-
@classmethod
148-
def set_binary_from_mime_type(cls, is_binary: bool, info: ValidationInfo) -> bool:
149-
"""Set is_binary based on mime_type if not explicitly set."""
150-
if is_binary:
151-
return True
152-
mime_type = info.data.get("mime_type", "text/plain")
153-
return not mime_type.startswith("text/")
154-
155173
async def read(self) -> str | bytes:
156174
"""Read the file content."""
157175
try:
158-
if self.is_binary:
176+
if self.encoding is None:
159177
return await anyio.to_thread.run_sync(self.path.read_bytes)
160-
return await anyio.to_thread.run_sync(self.path.read_text)
178+
return await anyio.to_thread.run_sync(partial(self.path.read_text, encoding=self.encoding))
161179
except Exception as e:
162180
raise ValueError(f"Error reading file {self.path}: {e}")
163181

tests/server/mcpserver/resources/test_file_resources.py

Lines changed: 130 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from tempfile import NamedTemporaryFile
44

55
import pytest
6+
from pydantic import ValidationError
67

78
from mcp.server.mcpserver.resources import FileResource
89

@@ -24,93 +25,143 @@ def temp_file():
2425
pass # File was already deleted by the test
2526

2627

27-
class TestFileResource:
28-
"""Test FileResource functionality."""
28+
def test_file_resource_creation(temp_file: Path):
29+
resource = FileResource(
30+
uri=temp_file.as_uri(),
31+
name="test",
32+
description="test file",
33+
path=temp_file,
34+
)
35+
assert str(resource.uri) == temp_file.as_uri()
36+
assert resource.name == "test"
37+
assert resource.description == "test file"
38+
assert resource.mime_type == "text/plain"
39+
assert resource.path == temp_file
40+
assert resource.encoding == "utf-8"
2941

30-
def test_file_resource_creation(self, temp_file: Path):
31-
"""Test creating a FileResource."""
32-
resource = FileResource(
33-
uri=temp_file.as_uri(),
34-
name="test",
35-
description="test file",
36-
path=temp_file,
37-
)
38-
assert str(resource.uri) == temp_file.as_uri()
39-
assert resource.name == "test"
40-
assert resource.description == "test file"
41-
assert resource.mime_type == "text/plain" # default
42-
assert resource.path == temp_file
43-
assert resource.is_binary is False # default
44-
45-
def test_file_resource_str_path_conversion(self, temp_file: Path):
46-
"""Test FileResource handles string paths."""
47-
resource = FileResource(
48-
uri=f"file://{temp_file}",
49-
name="test",
50-
path=Path(str(temp_file)),
51-
)
52-
assert isinstance(resource.path, Path)
53-
assert resource.path.is_absolute()
5442

55-
@pytest.mark.anyio
56-
async def test_read_text_file(self, temp_file: Path):
57-
"""Test reading a text file."""
58-
resource = FileResource(
59-
uri=f"file://{temp_file}",
43+
def test_file_resource_str_path_conversion(temp_file: Path):
44+
resource = FileResource(
45+
uri=f"file://{temp_file}",
46+
name="test",
47+
path=Path(str(temp_file)),
48+
)
49+
assert isinstance(resource.path, Path)
50+
assert resource.path.is_absolute()
51+
52+
53+
@pytest.mark.anyio
54+
async def test_read_text_file(temp_file: Path):
55+
resource = FileResource(
56+
uri=f"file://{temp_file}",
57+
name="test",
58+
path=temp_file,
59+
)
60+
content = await resource.read()
61+
assert content == "test content"
62+
assert resource.mime_type == "text/plain"
63+
64+
65+
@pytest.mark.anyio
66+
async def test_encoding_none_reads_bytes(temp_file: Path):
67+
resource = FileResource(
68+
uri=f"file://{temp_file}",
69+
name="test",
70+
path=temp_file,
71+
encoding=None,
72+
)
73+
content = await resource.read()
74+
assert isinstance(content, bytes)
75+
assert content == b"test content"
76+
77+
78+
@pytest.mark.parametrize(
79+
"mime_type",
80+
[
81+
"text/plain",
82+
"text/html; charset=utf-8",
83+
"application/json",
84+
"application/xml",
85+
"application/vnd.api+json",
86+
"image/svg+xml",
87+
"application/javascript",
88+
],
89+
)
90+
def test_textual_mime_types_default_to_utf8(temp_file: Path, mime_type: str):
91+
resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type)
92+
assert resource.encoding == "utf-8"
93+
94+
95+
@pytest.mark.parametrize("mime_type", ["image/png", "application/octet-stream", "application/pdf"])
96+
def test_binary_mime_types_default_to_no_encoding(temp_file: Path, mime_type: str):
97+
resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type)
98+
assert resource.encoding is None
99+
100+
101+
@pytest.mark.parametrize(
102+
"mime_type",
103+
["text/plain; charset=iso-8859-1", 'text/plain; format=flowed; charset="iso-8859-1"'],
104+
)
105+
def test_declared_charset_becomes_default_encoding(temp_file: Path, mime_type: str):
106+
resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type=mime_type)
107+
assert resource.encoding == "iso-8859-1"
108+
109+
110+
def test_removed_is_binary_kwarg_is_rejected(temp_file: Path):
111+
"""The v1 `is_binary` parameter fails loudly at construction rather than being ignored."""
112+
with pytest.raises(ValidationError, match="is_binary"):
113+
FileResource.model_validate({"uri": temp_file.as_uri(), "path": temp_file, "is_binary": True})
114+
115+
116+
@pytest.mark.anyio
117+
async def test_json_file_is_served_as_text_by_default(temp_file: Path):
118+
"""The mime type that motivated the encoding field: JSON must not become a base64 blob."""
119+
temp_file.write_text('{"a": 1}', encoding="utf-8")
120+
resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="application/json")
121+
assert await resource.read() == '{"a": 1}'
122+
123+
124+
@pytest.mark.anyio
125+
async def test_explicit_encoding_overrides_default(temp_file: Path):
126+
"""An explicit encoding wins over the mime-type default and is what decodes the file."""
127+
temp_file.write_bytes("naïve".encode("latin-1"))
128+
resource = FileResource(uri=temp_file.as_uri(), path=temp_file, mime_type="image/png", encoding="latin-1")
129+
assert resource.encoding == "latin-1"
130+
assert await resource.read() == "naïve"
131+
132+
133+
def test_relative_path_error():
134+
with pytest.raises(ValueError, match="Path must be absolute"):
135+
FileResource(
136+
uri="file:///test.txt",
60137
name="test",
61-
path=temp_file,
138+
path=Path("test.txt"),
62139
)
63-
content = await resource.read()
64-
assert content == "test content"
65-
assert resource.mime_type == "text/plain"
66140

67-
@pytest.mark.anyio
68-
async def test_read_binary_file(self, temp_file: Path):
69-
"""Test reading a file as binary."""
141+
142+
@pytest.mark.anyio
143+
async def test_missing_file_error(temp_file: Path):
144+
missing = temp_file.parent / "missing.txt"
145+
resource = FileResource(
146+
uri="file:///missing.txt",
147+
name="test",
148+
path=missing,
149+
)
150+
with pytest.raises(ValueError, match="Error reading file"):
151+
await resource.read()
152+
153+
154+
@pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows")
155+
@pytest.mark.anyio
156+
async def test_permission_error(temp_file: Path): # pragma: lax no cover
157+
temp_file.chmod(0o000) # Remove all permissions
158+
try:
70159
resource = FileResource(
71-
uri=f"file://{temp_file}",
160+
uri=temp_file.as_uri(),
72161
name="test",
73162
path=temp_file,
74-
is_binary=True,
75-
)
76-
content = await resource.read()
77-
assert isinstance(content, bytes)
78-
assert content == b"test content"
79-
80-
def test_relative_path_error(self):
81-
"""Test error on relative path."""
82-
with pytest.raises(ValueError, match="Path must be absolute"):
83-
FileResource(
84-
uri="file:///test.txt",
85-
name="test",
86-
path=Path("test.txt"),
87-
)
88-
89-
@pytest.mark.anyio
90-
async def test_missing_file_error(self, temp_file: Path):
91-
"""Test error when file doesn't exist."""
92-
# Create path to non-existent file
93-
missing = temp_file.parent / "missing.txt"
94-
resource = FileResource(
95-
uri="file:///missing.txt",
96-
name="test",
97-
path=missing,
98163
)
99164
with pytest.raises(ValueError, match="Error reading file"):
100165
await resource.read()
101-
102-
@pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows")
103-
@pytest.mark.anyio
104-
async def test_permission_error(self, temp_file: Path): # pragma: lax no cover
105-
"""Test reading a file without permissions."""
106-
temp_file.chmod(0o000) # Remove all permissions
107-
try:
108-
resource = FileResource(
109-
uri=temp_file.as_uri(),
110-
name="test",
111-
path=temp_file,
112-
)
113-
with pytest.raises(ValueError, match="Error reading file"):
114-
await resource.read()
115-
finally:
116-
temp_file.chmod(0o644) # Restore permissions
166+
finally:
167+
temp_file.chmod(0o644) # Restore permissions

0 commit comments

Comments
 (0)