[v1.x] fix: reject trailing newline in tool-name validation (#3086)
Co-authored-by: otiscuilei <otiscui@icloud.com>
This commit is contained in:
parent
5f0b6af9f2
commit
e8283746d0
4 changed files with 22 additions and 2 deletions
|
|
@ -86,7 +86,7 @@ class ResourceTemplate(BaseModel):
|
||||||
"""Check if URI matches template and extract parameters."""
|
"""Check if URI matches template and extract parameters."""
|
||||||
# Convert template to regex pattern
|
# Convert template to regex pattern
|
||||||
pattern = self.uri_template.replace("{", "(?P<").replace("}", ">[^/]+)")
|
pattern = self.uri_template.replace("{", "(?P<").replace("}", ">[^/]+)")
|
||||||
match = re.match(f"^{pattern}$", uri)
|
match = re.fullmatch(pattern, uri)
|
||||||
if match:
|
if match:
|
||||||
return match.groupdict()
|
return match.groupdict()
|
||||||
return None
|
return None
|
||||||
|
|
|
||||||
|
|
@ -77,7 +77,7 @@ def validate_tool_name(name: str) -> ToolNameValidationResult:
|
||||||
warnings.append("Tool name starts or ends with a dot, which may cause parsing issues in some contexts")
|
warnings.append("Tool name starts or ends with a dot, which may cause parsing issues in some contexts")
|
||||||
|
|
||||||
# Check for invalid characters
|
# Check for invalid characters
|
||||||
if not TOOL_NAME_REGEX.match(name):
|
if not TOOL_NAME_REGEX.fullmatch(name):
|
||||||
# Find all invalid characters (unique, preserving order)
|
# Find all invalid characters (unique, preserving order)
|
||||||
invalid_chars: list[str] = []
|
invalid_chars: list[str] = []
|
||||||
seen: set[str] = set()
|
seen: set[str] = set()
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,21 @@ class TestResourceTemplate:
|
||||||
assert template.matches("test://foo") is None
|
assert template.matches("test://foo") is None
|
||||||
assert template.matches("other://foo/123") is None
|
assert template.matches("other://foo/123") is None
|
||||||
|
|
||||||
|
def test_template_matches_rejects_trailing_newline_after_literal(self):
|
||||||
|
"""A trailing newline after a literal segment slipped past `$` with re.match."""
|
||||||
|
|
||||||
|
def my_func(key: str) -> str: # pragma: no cover
|
||||||
|
return key
|
||||||
|
|
||||||
|
template = ResourceTemplate.from_function(
|
||||||
|
fn=my_func,
|
||||||
|
uri_template="test://{key}/data",
|
||||||
|
name="test",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert template.matches("test://foo/data") == {"key": "foo"}
|
||||||
|
assert template.matches("test://foo/data\n") is None
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_create_resource(self):
|
async def test_create_resource(self):
|
||||||
"""Test creating a resource from a template."""
|
"""Test creating a resource from a template."""
|
||||||
|
|
|
||||||
|
|
@ -66,12 +66,17 @@ class TestValidateToolName:
|
||||||
("get,user,profile", "','"),
|
("get,user,profile", "','"),
|
||||||
("user/profile/update", "'/'"),
|
("user/profile/update", "'/'"),
|
||||||
("user@domain.com", "'@'"),
|
("user@domain.com", "'@'"),
|
||||||
|
# a single trailing newline slipped past `$` with re.match
|
||||||
|
("valid_name\n", "'\\n'"),
|
||||||
|
("a" * 127 + "\n", "'\\n'"),
|
||||||
],
|
],
|
||||||
ids=[
|
ids=[
|
||||||
"with_spaces",
|
"with_spaces",
|
||||||
"with_commas",
|
"with_commas",
|
||||||
"with_slashes",
|
"with_slashes",
|
||||||
"with_at_symbol",
|
"with_at_symbol",
|
||||||
|
"with_trailing_newline",
|
||||||
|
"max_length_with_trailing_newline",
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
def test_rejects_invalid_characters(self, tool_name: str, expected_char: str) -> None:
|
def test_rejects_invalid_characters(self, tool_name: str, expected_char: str) -> None:
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue