From 8173a38280b0c7c31b2ef5bc3e7dfd1f4ce7fa8b Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 19 Sep 2026 17:08:22 +0900 Subject: [PATCH] Validate the src and sizes of an icon against the specification ## Motivation and Context `MCP::Icon` validated `theme` but accepted anything for `src` and `sizes`: `src:` defaulted to `nil`, so `MCP::Icon.new.to_h` was `{}`, an icon with no source, and a `sizes` given as a String reached `to_h` unchanged and serialized as a JSON string where the specification's `Icon` type requires an array of strings. Nothing on the server side reported either; the server answered every request with 200 and the failure surfaced in a client, with no reference to the field responsible. The TypeScript SDK types `sizes` as `string[]` and `src` as required, and the Python SDK's model refuses such an icon at construction, so this SDK was the one that let it through. `src` is now a required keyword and must be a non-empty String, `sizes` must be `nil` or an Array of Strings, and `mime_type` must be `nil` or a String, each refused with `ArgumentError` at construction the way `theme` already was. The checks stop at the schema's types, as the reference SDKs do: the scheme of a URL and the `WxH` form of a size stay the caller's to get right. A message names the class of the rejected value, never the value, since `src` may carry a `data:` URI of any length. Coercing a String into a one-element Array was considered and rejected: it would hide the caller's mistake that the class exists to surface. The `theme` check let `false` through its truthiness test and serialized it; it now refuses everything but `nil`, `"light"`, and `"dark"`. Fixes #562. ## How Has This Been Tested? New tests in `test/mcp/icon_test.rb`. Against the previous library, an icon without `src` and an icon whose `sizes` is a String are both constructed and serialized. ## Breaking Changes `MCP::Icon.new` now raises `ArgumentError` where earlier releases produced an icon the specification's schema rejects: `src:` is required and must be a non-empty String, so a call without it, or with `nil`, an empty String, or a non-String, fails; `sizes:` must be `nil` or an Array of Strings, so a String, or an Array holding `nil` or a non-String, fails; `mime_type:` must be `nil` or a String; `theme:` must be `nil`, `"light"`, or `"dark"`, so `false` fails where it used to be serialized. Leaving the optional keywords out, or passing `nil` for them, is unchanged. --- lib/mcp/icon.rb | 37 ++++++++++++++++- test/mcp/icon_test.rb | 94 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/lib/mcp/icon.rb b/lib/mcp/icon.rb index 3397c33f..56ac6adb 100644 --- a/lib/mcp/icon.rb +++ b/lib/mcp/icon.rb @@ -1,13 +1,35 @@ # frozen_string_literal: true module MCP + # An icon attached to a server, tool, prompt, or resource, per the specification's `Icon` type: + # https://modelcontextprotocol.io/specification/2026-07-28/basic#icons + # + # Each argument is checked against the schema's type, as the TypeScript and Python SDKs check theirs, + # so an icon that would serialize into something a client rejects fails here instead. What a value means + # is not judged: `src` may be any non-empty `String` (the schema types it as a URI, which an empty `String` + # is not, and the specification allows an HTTP/HTTPS URL or a `data:` URI), and a size may be any `String` + # (the specification expects `WxH` or `"any"`). class Icon SUPPORTED_THEMES = ["light", "dark"].freeze attr_reader :mime_type, :sizes, :src, :theme - def initialize(mime_type: nil, sizes: nil, src: nil, theme: nil) - raise ArgumentError, 'The value of theme must specify "light" or "dark".' if theme && !SUPPORTED_THEMES.include?(theme) + def initialize(mime_type: nil, sizes: nil, src:, theme: nil) + unless src.is_a?(String) && !src.empty? + raise ArgumentError, "The value of src must be a non-empty String (got #{src.class})." + end + + unless mime_type.nil? || mime_type.is_a?(String) + raise ArgumentError, "The value of mime_type must be a String (got #{mime_type.class})." + end + + if (problem = sizes_problem(sizes)) + raise ArgumentError, "The value of sizes must be an Array of Strings such as [\"48x48\"] or [\"any\"] (#{problem})." + end + + unless theme.nil? || SUPPORTED_THEMES.include?(theme) + raise ArgumentError, 'The value of theme must specify "light" or "dark".' + end @mime_type = mime_type @sizes = sizes @@ -18,5 +40,16 @@ def initialize(mime_type: nil, sizes: nil, src: nil, theme: nil) def to_h { mimeType: mime_type, sizes: sizes, src: src, theme: theme }.compact end + + private + + def sizes_problem(sizes) + return if sizes.nil? + return "got #{sizes.class}" unless sizes.is_a?(Array) + + index = sizes.index { |size| !size.is_a?(String) } + + "got #{sizes[index].class} inside the Array" if index + end end end diff --git a/test/mcp/icon_test.rb b/test/mcp/icon_test.rb index e8c837cd..f2fbc9a2 100644 --- a/test/mcp/icon_test.rb +++ b/test/mcp/icon_test.rb @@ -15,32 +15,112 @@ def test_initialization assert_equal({ mimeType: "image/png", sizes: ["48x48", "96x96"], src: "https://example.com", theme: "light" }, icon.to_h) end - def test_initialization_by_default - icon = Icon.new + def test_initialization_with_only_src + icon = Icon.new(src: "https://example.com/icon.png") assert_nil(icon.mime_type) assert_nil(icon.sizes) - assert_nil(icon.src) + assert_equal("https://example.com/icon.png", icon.src) assert_nil(icon.theme) - assert_equal({}, icon.to_h) + assert_equal({ src: "https://example.com/icon.png" }, icon.to_h) + end + + def test_src_is_required + exception = assert_raises(ArgumentError) do + Icon.new + end + assert_equal("missing keyword: :src", exception.message) + end + + def test_src_rejects_nil_and_an_empty_string + [nil, ""].each do |src| + exception = assert_raises(ArgumentError) do + Icon.new(src: src) + end + assert_equal("The value of src must be a non-empty String (got #{src.class}).", exception.message) + end + end + + def test_src_rejects_a_non_string + exception = assert_raises(ArgumentError) do + Icon.new(src: :icon) + end + assert_equal("The value of src must be a non-empty String (got Symbol).", exception.message) + end + + def test_sizes_accepts_an_array_of_strings + [["48x48", "96x96"], ["any"], []].each do |sizes| + icon = Icon.new(sizes: sizes, src: "https://example.com/icon.png") + + assert_equal(sizes, icon.to_h[:sizes]) + end + end + + # https://github.com/modelcontextprotocol/ruby-sdk/issues/562 + def test_sizes_rejects_a_string + exception = assert_raises(ArgumentError) do + Icon.new(mime_type: "image/png", sizes: "51x51", src: "https://example.com/icon.png") + end + assert_equal( + 'The value of sizes must be an Array of Strings such as ["48x48"] or ["any"] (got String).', + exception.message, + ) + end + + def test_sizes_rejects_an_array_holding_a_non_string + { + ["48x48", 96] => "Integer", + [nil] => "NilClass", + ["48x48", nil] => "NilClass", + [nil, 96] => "NilClass", + }.each do |sizes, offender| + exception = assert_raises(ArgumentError) do + Icon.new(sizes: sizes, src: "https://example.com/icon.png") + end + assert_equal( + "The value of sizes must be an Array of Strings such as [\"48x48\"] or [\"any\"] (got #{offender} inside the Array).", + exception.message, + ) + end + end + + def test_src_accepts_any_scheme_and_sizes_accept_any_string + string_subclass = Class.new(String) + icon = Icon.new(sizes: ["unconventional", +"48x48", string_subclass.new("any")], src: "custom:icon") + + assert_equal({ sizes: ["unconventional", "48x48", "any"], src: "custom:icon" }, icon.to_h) + end + + def test_mime_type_rejects_a_non_string + exception = assert_raises(ArgumentError) do + Icon.new(mime_type: :png, src: "https://example.com/icon.png") + end + assert_equal("The value of mime_type must be a String (got Symbol).", exception.message) end def test_valid_theme_for_light assert_nothing_raised do - Icon.new(theme: "light") + Icon.new(src: "https://example.com/icon.png", theme: "light") end end def test_valid_theme_for_dark assert_nothing_raised do - Icon.new(theme: "dark") + Icon.new(src: "https://example.com/icon.png", theme: "dark") end end def test_invalid_theme exception = assert_raises(ArgumentError) do - Icon.new(theme: "unexpected") + Icon.new(src: "https://example.com/icon.png", theme: "unexpected") + end + assert_equal('The value of theme must specify "light" or "dark".', exception.message) + end + + def test_theme_rejects_false + exception = assert_raises(ArgumentError) do + Icon.new(src: "https://example.com/icon.png", theme: false) end assert_equal('The value of theme must specify "light" or "dark".', exception.message) end