Skip to content

Commit cfa5c1d

Browse files
authored
mcp: check that UnmarshalJSON methods for Content don't panic on nil
Introduces a new test file content_nil_test.go which verifies that UnmarshalJSON methods for various Content types do not panic when unmarshaling onto nil pointers. Adds a nil check in contentFromWire function to guard against a nil wire.Content parameter. Tests cover different scenarios, including valid and invalid content types, as well as cases with empty or missing content fields. For #205
1 parent 16f2857 commit cfa5c1d

File tree

2 files changed

+227
-0
lines changed

2 files changed

+227
-0
lines changed

mcp/content.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ func contentsFromWire(wires []*wireContent, allow map[string]bool) ([]Content, e
252252
}
253253

254254
func contentFromWire(wire *wireContent, allow map[string]bool) (Content, error) {
255+
if wire == nil {
256+
return nil, fmt.Errorf("content wire is nil")
257+
}
255258
if allow != nil && !allow[wire.Type] {
256259
return nil, fmt.Errorf("invalid content type %q", wire.Type)
257260
}

mcp/content_nil_test.go

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright 2025 The Go MCP SDK Authors. All rights reserved.
2+
// Use of this source code is governed by an MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
// This file contains tests to verify that UnmarshalJSON methods for Content types
6+
// don't panic when unmarshaling onto nil pointers, as requested in GitHub issue #205.
7+
//
8+
// NOTE: The contentFromWire function has been fixed to handle nil wire.Content
9+
// gracefully by returning an error instead of panicking.
10+
11+
package mcp_test
12+
13+
import (
14+
"encoding/json"
15+
"testing"
16+
17+
"github.com/google/go-cmp/cmp"
18+
"github.com/modelcontextprotocol/go-sdk/mcp"
19+
)
20+
21+
func TestContentUnmarshalNil(t *testing.T) {
22+
tests := []struct {
23+
name string
24+
json string
25+
content interface{}
26+
want interface{}
27+
}{
28+
{
29+
name: "CallToolResult nil Content",
30+
json: `{"content":[{"type":"text","text":"hello"}]}`,
31+
content: &mcp.CallToolResult{},
32+
want: &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: "hello"}}},
33+
},
34+
{
35+
name: "CreateMessageResult nil Content",
36+
json: `{"content":{"type":"text","text":"hello"},"model":"test","role":"user"}`,
37+
content: &mcp.CreateMessageResult{},
38+
want: &mcp.CreateMessageResult{Content: &mcp.TextContent{Text: "hello"}, Model: "test", Role: "user"},
39+
},
40+
{
41+
name: "PromptMessage nil Content",
42+
json: `{"content":{"type":"text","text":"hello"},"role":"user"}`,
43+
content: &mcp.PromptMessage{},
44+
want: &mcp.PromptMessage{Content: &mcp.TextContent{Text: "hello"}, Role: "user"},
45+
},
46+
{
47+
name: "SamplingMessage nil Content",
48+
json: `{"content":{"type":"text","text":"hello"},"role":"user"}`,
49+
content: &mcp.SamplingMessage{},
50+
want: &mcp.SamplingMessage{Content: &mcp.TextContent{Text: "hello"}, Role: "user"},
51+
},
52+
{
53+
name: "CallToolResultFor nil Content",
54+
json: `{"content":[{"type":"text","text":"hello"}]}`,
55+
content: &mcp.CallToolResultFor[string]{},
56+
want: &mcp.CallToolResultFor[string]{Content: []mcp.Content{&mcp.TextContent{Text: "hello"}}},
57+
},
58+
}
59+
60+
for _, tt := range tests {
61+
t.Run(tt.name, func(t *testing.T) {
62+
// Test that unmarshaling doesn't panic on nil Content fields
63+
defer func() {
64+
if r := recover(); r != nil {
65+
t.Errorf("UnmarshalJSON panicked: %v", r)
66+
}
67+
}()
68+
69+
err := json.Unmarshal([]byte(tt.json), tt.content)
70+
if err != nil {
71+
t.Errorf("UnmarshalJSON failed: %v", err)
72+
}
73+
74+
// Verify that the Content field was properly populated
75+
if cmp.Diff(tt.want, tt.content) != "" {
76+
t.Errorf("Content is not equal: %v", cmp.Diff(tt.content, tt.content))
77+
}
78+
})
79+
}
80+
}
81+
82+
func TestContentUnmarshalNilWithDifferentTypes(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
json string
86+
content interface{}
87+
expectError bool
88+
}{
89+
{
90+
name: "ImageContent",
91+
json: `{"content":{"type":"image","mimeType":"image/png","data":"YTFiMmMz"}}`,
92+
content: &mcp.CreateMessageResult{},
93+
expectError: false,
94+
},
95+
{
96+
name: "AudioContent",
97+
json: `{"content":{"type":"audio","mimeType":"audio/wav","data":"YTFiMmMz"}}`,
98+
content: &mcp.CreateMessageResult{},
99+
expectError: false,
100+
},
101+
{
102+
name: "ResourceLink",
103+
json: `{"content":{"type":"resource_link","uri":"file:///test","name":"test"}}`,
104+
content: &mcp.CreateMessageResult{},
105+
expectError: true, // CreateMessageResult only allows text, image, audio
106+
},
107+
{
108+
name: "EmbeddedResource",
109+
json: `{"content":{"type":"resource","resource":{"uri":"file://test","text":"test"}}}`,
110+
content: &mcp.CreateMessageResult{},
111+
expectError: true, // CreateMessageResult only allows text, image, audio
112+
},
113+
}
114+
115+
for _, tt := range tests {
116+
t.Run(tt.name, func(t *testing.T) {
117+
// Test that unmarshaling doesn't panic on nil Content fields
118+
defer func() {
119+
if r := recover(); r != nil {
120+
t.Errorf("UnmarshalJSON panicked: %v", r)
121+
}
122+
}()
123+
124+
err := json.Unmarshal([]byte(tt.json), tt.content)
125+
if tt.expectError && err == nil {
126+
t.Error("Expected error but got none")
127+
}
128+
if !tt.expectError && err != nil {
129+
t.Errorf("Unexpected error: %v", err)
130+
}
131+
132+
// Verify that the Content field was properly populated for successful cases
133+
if !tt.expectError {
134+
if result, ok := tt.content.(*mcp.CreateMessageResult); ok {
135+
if result.Content == nil {
136+
t.Error("CreateMessageResult.Content was not populated")
137+
}
138+
}
139+
}
140+
})
141+
}
142+
}
143+
144+
func TestContentUnmarshalNilWithEmptyContent(t *testing.T) {
145+
tests := []struct {
146+
name string
147+
json string
148+
content interface{}
149+
expectError bool
150+
}{
151+
{
152+
name: "Empty Content array",
153+
json: `{"content":[]}`,
154+
content: &mcp.CallToolResult{},
155+
expectError: false,
156+
},
157+
{
158+
name: "Missing Content field",
159+
json: `{"model":"test","role":"user"}`,
160+
content: &mcp.CreateMessageResult{},
161+
expectError: true, // Content field is required for CreateMessageResult
162+
},
163+
}
164+
165+
for _, tt := range tests {
166+
t.Run(tt.name, func(t *testing.T) {
167+
// Test that unmarshaling doesn't panic on nil Content fields
168+
defer func() {
169+
if r := recover(); r != nil {
170+
t.Errorf("UnmarshalJSON panicked: %v", r)
171+
}
172+
}()
173+
174+
err := json.Unmarshal([]byte(tt.json), tt.content)
175+
if tt.expectError && err == nil {
176+
t.Error("Expected error but got none")
177+
}
178+
if !tt.expectError && err != nil {
179+
t.Errorf("Unexpected error: %v", err)
180+
}
181+
})
182+
}
183+
}
184+
185+
func TestContentUnmarshalNilWithInvalidContent(t *testing.T) {
186+
tests := []struct {
187+
name string
188+
json string
189+
content interface{}
190+
expectError bool
191+
}{
192+
{
193+
name: "Invalid content type",
194+
json: `{"content":{"type":"invalid","text":"hello"}}`,
195+
content: &mcp.CreateMessageResult{},
196+
expectError: true,
197+
},
198+
{
199+
name: "Missing type field",
200+
json: `{"content":{"text":"hello"}}`,
201+
content: &mcp.CreateMessageResult{},
202+
expectError: true,
203+
},
204+
}
205+
206+
for _, tt := range tests {
207+
t.Run(tt.name, func(t *testing.T) {
208+
// Test that unmarshaling doesn't panic on nil Content fields
209+
defer func() {
210+
if r := recover(); r != nil {
211+
t.Errorf("UnmarshalJSON panicked: %v", r)
212+
}
213+
}()
214+
215+
err := json.Unmarshal([]byte(tt.json), tt.content)
216+
if tt.expectError && err == nil {
217+
t.Error("Expected error but got none")
218+
}
219+
if !tt.expectError && err != nil {
220+
t.Errorf("Unexpected error: %v", err)
221+
}
222+
})
223+
}
224+
}

0 commit comments

Comments
 (0)