diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java index b58f1c552..3b3f16a94 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java @@ -1325,6 +1325,15 @@ public record ListToolsResult( // @formatter:off @JsonProperty("nextCursor") String nextCursor, @JsonProperty("_meta") Map meta) implements Result { // @formatter:on + /** + * Compact constructor that validates tool names on deserialization (warns only). + */ + public ListToolsResult { + if (tools != null) { + tools.forEach(tool -> ToolNameValidator.validate(tool.name(), false)); + } + } + public ListToolsResult(List tools, String nextCursor) { this(tools, nextCursor, null); } @@ -1466,7 +1475,7 @@ public Builder meta(Map meta) { } public Tool build() { - Assert.hasText(name, "name must not be empty"); + ToolNameValidator.validate(name, true); return new Tool(name, title, description, inputSchema, outputSchema, annotations, meta); } @@ -1508,6 +1517,13 @@ public record CallToolRequest( // @formatter:off @JsonProperty("arguments") Map arguments, @JsonProperty("_meta") Map meta) implements Request { // @formatter:on + /** + * Compact constructor that validates tool name on deserialization (warns only). + */ + public CallToolRequest { + ToolNameValidator.validate(name, false); + } + public CallToolRequest(McpJsonMapper jsonMapper, String name, String jsonArguments) { this(name, parseJsonArguments(jsonMapper, jsonArguments), null); } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/ToolNameValidator.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/ToolNameValidator.java new file mode 100644 index 000000000..2bc27f06a --- /dev/null +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/ToolNameValidator.java @@ -0,0 +1,68 @@ +/* + * Copyright 2024-2024 the original author or authors. + */ + +package io.modelcontextprotocol.spec; + +import java.util.regex.Pattern; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Validates tool names according to the MCP specification. + * + *

+ * Tool names must conform to the following rules: + *

    + *
  • Must be between 1 and 128 characters in length
  • + *
  • May only contain: A-Z, a-z, 0-9, underscore (_), hyphen (-), and dot (.)
  • + *
  • Must not contain spaces, commas, or other special characters
  • + *
+ * + * @see MCP + * Specification - Tool Names + */ +public final class ToolNameValidator { + + private static final Logger logger = LoggerFactory.getLogger(ToolNameValidator.class); + + private static final int MAX_LENGTH = 128; + + private static final Pattern VALID_NAME_PATTERN = Pattern.compile("^[A-Za-z0-9_\\-.]+$"); + + private ToolNameValidator() { + } + + /** + * Validates a tool name according to MCP specification. + * @param name the tool name to validate + * @param strict if true, throws exception on invalid name; if false, logs warning + * @throws IllegalArgumentException if strict is true and name is invalid + */ + public static void validate(String name, boolean strict) { + if (name == null || name.isEmpty()) { + handleError("Tool name must not be null or empty", name, strict); + return; + } + if (name.length() > MAX_LENGTH) { + handleError("Tool name must not exceed 128 characters", name, strict); + return; + } + if (!VALID_NAME_PATTERN.matcher(name).matches()) { + handleError("Tool name contains invalid characters (allowed: A-Z, a-z, 0-9, _, -, .)", name, strict); + } + } + + private static void handleError(String message, String name, boolean strict) { + String fullMessage = message + ": '" + name + "'"; + if (strict) { + throw new IllegalArgumentException(fullMessage); + } + else { + logger.warn("{}. Processing continues, but tool name should be fixed.", fullMessage); + } + } + +} diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java index 82ffe9ede..6bfda18ed 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java @@ -1765,4 +1765,33 @@ void testProgressNotificationWithoutMessage() throws Exception { {"progressToken":"progress-token-789","progress":0.25}""")); } + // Tool Name Validation Tests + + @Test + void testToolBuilderWithValidName() { + McpSchema.Tool tool = McpSchema.Tool.builder().name("valid_tool-name.v1").description("A test tool").build(); + + assertThat(tool.name()).isEqualTo("valid_tool-name.v1"); + assertThat(tool.description()).isEqualTo("A test tool"); + } + + @Test + void testToolBuilderWithInvalidNameThrowsException() { + assertThatThrownBy(() -> McpSchema.Tool.builder().name("invalid tool name").build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("invalid characters"); + } + + @Test + void testListToolsResultDeserializationWithInvalidToolName() throws Exception { + // Deserialization should not throw, just warn + String json = """ + {"tools":[{"name":"invalid tool name","description":"test"}],"nextCursor":null}"""; + + McpSchema.ListToolsResult result = JSON_MAPPER.readValue(json, McpSchema.ListToolsResult.class); + + assertThat(result.tools()).hasSize(1); + assertThat(result.tools().get(0).name()).isEqualTo("invalid tool name"); + } + } diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/spec/ToolNameValidatorTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/spec/ToolNameValidatorTests.java new file mode 100644 index 000000000..38d05fb62 --- /dev/null +++ b/mcp-core/src/test/java/io/modelcontextprotocol/spec/ToolNameValidatorTests.java @@ -0,0 +1,110 @@ +/* + * Copyright 2024-2024 the original author or authors. + */ + +package io.modelcontextprotocol.spec; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Tests for {@link ToolNameValidator}. + */ +class ToolNameValidatorTests { + + @ParameterizedTest + @ValueSource(strings = { "getUser", "DATA_EXPORT_v2", "admin.tools.list", "my-tool", "Tool123", "a", "A", + "_private", "tool_name", "tool-name", "tool.name", "UPPERCASE", "lowercase", "MixedCase123" }) + void validToolNames_strictMode(String name) { + assertThatCode(() -> ToolNameValidator.validate(name, true)).doesNotThrowAnyException(); + } + + @Test + void validToolName_maxLength() { + String name = "a".repeat(128); + assertThatCode(() -> ToolNameValidator.validate(name, true)).doesNotThrowAnyException(); + } + + @Test + void invalidToolName_null_strictMode() { + assertThatThrownBy(() -> ToolNameValidator.validate(null, true)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("null or empty"); + } + + @Test + void invalidToolName_empty_strictMode() { + assertThatThrownBy(() -> ToolNameValidator.validate("", true)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("null or empty"); + } + + @Test + void invalidToolName_tooLong_strictMode() { + String name = "a".repeat(129); + assertThatThrownBy(() -> ToolNameValidator.validate(name, true)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("128 characters"); + } + + @ParameterizedTest + @ValueSource(strings = { "tool name", // space + "tool,name", // comma + "tool@name", // at sign + "tool#name", // hash + "tool$name", // dollar + "tool%name", // percent + "tool&name", // ampersand + "tool*name", // asterisk + "tool+name", // plus + "tool=name", // equals + "tool/name", // slash + "tool\\name", // backslash + "tool:name", // colon + "tool;name", // semicolon + "tool'name", // single quote + "tool\"name", // double quote + "toolname", // greater than + "tool?name", // question mark + "tool!name", // exclamation + "tool(name)", // parentheses + "tool[name]", // brackets + "tool{name}", // braces + "tool|name", // pipe + "tool~name", // tilde + "tool`name", // backtick + "tool^name", // caret + "tööl", // non-ASCII + "工具" // unicode + }) + void invalidToolNames_specialCharacters_strictMode(String name) { + assertThatThrownBy(() -> ToolNameValidator.validate(name, true)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("invalid characters"); + } + + @Test + void invalidToolName_nonStrictMode_doesNotThrow() { + // Non-strict mode should not throw, just warn + assertThatCode(() -> ToolNameValidator.validate("invalid name", false)).doesNotThrowAnyException(); + assertThatCode(() -> ToolNameValidator.validate(null, false)).doesNotThrowAnyException(); + assertThatCode(() -> ToolNameValidator.validate("", false)).doesNotThrowAnyException(); + assertThatCode(() -> ToolNameValidator.validate("a".repeat(129), false)).doesNotThrowAnyException(); + } + + @Test + void toolBuilder_validatesName_strictMode() { + assertThatThrownBy(() -> McpSchema.Tool.builder().name("invalid name with space").build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("invalid characters"); + } + + @Test + void toolBuilder_validName() { + McpSchema.Tool tool = McpSchema.Tool.builder().name("valid_tool-name.v1").build(); + assertThat(tool.name()).isEqualTo("valid_tool-name.v1"); + } + +}