Merge pull request #563 from koic/validate_icon_src_and_sizes

Validate the src and sizes of an icon against the specification

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 @@ module MCP
     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

test/mcp/icon_test.rb

@@ -15,32 +15,112 @@ module MCP
       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