Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mcp/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ func contentsFromWire(wires []*wireContent, allow map[string]bool) ([]Content, e
}

func contentFromWire(wire *wireContent, allow map[string]bool) (Content, error) {
if wire == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failed without this change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. One of the tests that I added failed without this change.

The test is unmarshaling {"model":"test","role":"user"} (missing "content"), which causes the *wireContent parameter here to be nil.

Below is the test output without this change:

$ go test ./mcp   
--- FAIL: TestContentUnmarshalNilWithEmptyContent (0.00s)
    --- FAIL: TestContentUnmarshalNilWithEmptyContent/Missing_Content_field (0.00s)
        content_nil_test.go:170: UnmarshalJSON panicked: runtime error: invalid memory address or nil pointer dereference
FAIL
FAIL    github.com/modelcontextprotocol/go-sdk/mcp      7.532s
FAIL

return nil, fmt.Errorf("content wire is nil")
}
if allow != nil && !allow[wire.Type] {
return nil, fmt.Errorf("invalid content type %q", wire.Type)
}
Expand Down
251 changes: 251 additions & 0 deletions mcp/content_nil_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
// Copyright 2025 The Go MCP SDK Authors. All rights reserved.
// Use of this source code is governed by an MIT-style
// license that can be found in the LICENSE file.

// This file contains tests to verify that UnmarshalJSON methods for Content types
// don't panic when unmarshaling onto nil pointers, as requested in GitHub issue #205.
//
// NOTE: The contentFromWire function has been fixed to handle nil wire.Content
// gracefully by returning an error instead of panicking.

package mcp_test

import (
"encoding/json"
"testing"

"github.com/modelcontextprotocol/go-sdk/mcp"
)

func TestContentUnmarshalNil(t *testing.T) {
tests := []struct {
name string
json string
content interface{}
}{
{
name: "CallToolResult nil Content",
json: `{"content":[{"type":"text","text":"hello"}]}`,
content: &mcp.CallToolResult{},
},
{
name: "CreateMessageResult nil Content",
json: `{"content":{"type":"text","text":"hello"},"model":"test","role":"user"}`,
content: &mcp.CreateMessageResult{},
},
{
name: "PromptMessage nil Content",
json: `{"content":{"type":"text","text":"hello"},"role":"user"}`,
content: &mcp.PromptMessage{},
},
{
name: "SamplingMessage nil Content",
json: `{"content":{"type":"text","text":"hello"},"role":"user"}`,
content: &mcp.SamplingMessage{},
},
{
name: "CallToolResultFor nil Content",
json: `{"content":[{"type":"text","text":"hello"}]}`,
content: &mcp.CallToolResultFor[string]{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test that unmarshaling doesn't panic on nil Content fields
defer func() {
if r := recover(); r != nil {
t.Errorf("UnmarshalJSON panicked: %v", r)
}
}()

err := json.Unmarshal([]byte(tt.json), tt.content)
if err != nil {
t.Errorf("UnmarshalJSON failed: %v", err)
}

// Verify that the Content field was properly populated
switch v := tt.content.(type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simplify this significantly using cmp.Diff: just compare the expected and actual value.
https://pkg.go.dev/github.com/google/go-cmp/cmp is already required by this module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a56a3cc

case *mcp.CallToolResult:
if len(v.Content) == 0 {
t.Error("CallToolResult.Content was not populated")
}
if _, ok := v.Content[0].(*mcp.TextContent); !ok {
t.Error("CallToolResult.Content[0] is not TextContent")
}
case *mcp.CallToolResultFor[string]:
if len(v.Content) == 0 {
t.Error("CallToolResultFor.Content was not populated")
}
if _, ok := v.Content[0].(*mcp.TextContent); !ok {
t.Error("CallToolResultFor.Content[0] is not TextContent")
}
case *mcp.CreateMessageResult:
if v.Content == nil {
t.Error("CreateMessageResult.Content was not populated")
}
if _, ok := v.Content.(*mcp.TextContent); !ok {
t.Error("CreateMessageResult.Content is not TextContent")
}
case *mcp.PromptMessage:
if v.Content == nil {
t.Error("PromptMessage.Content was not populated")
}
if _, ok := v.Content.(*mcp.TextContent); !ok {
t.Error("PromptMessage.Content is not TextContent")
}
case *mcp.SamplingMessage:
if v.Content == nil {
t.Error("SamplingMessage.Content was not populated")
}
if _, ok := v.Content.(*mcp.TextContent); !ok {
t.Error("SamplingMessage.Content is not TextContent")
}
}
})
}
}

func TestContentUnmarshalNilWithDifferentTypes(t *testing.T) {
tests := []struct {
name string
json string
content interface{}
expectError bool
}{
{
name: "ImageContent",
json: `{"content":{"type":"image","mimeType":"image/png","data":"YTFiMmMz"}}`,
content: &mcp.CreateMessageResult{},
expectError: false,
},
{
name: "AudioContent",
json: `{"content":{"type":"audio","mimeType":"audio/wav","data":"YTFiMmMz"}}`,
content: &mcp.CreateMessageResult{},
expectError: false,
},
{
name: "ResourceLink",
json: `{"content":{"type":"resource_link","uri":"file:///test","name":"test"}}`,
content: &mcp.CreateMessageResult{},
expectError: true, // CreateMessageResult only allows text, image, audio
},
{
name: "EmbeddedResource",
json: `{"content":{"type":"resource","resource":{"uri":"file://test","text":"test"}}}`,
content: &mcp.CreateMessageResult{},
expectError: true, // CreateMessageResult only allows text, image, audio
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test that unmarshaling doesn't panic on nil Content fields
defer func() {
if r := recover(); r != nil {
t.Errorf("UnmarshalJSON panicked: %v", r)
}
}()

err := json.Unmarshal([]byte(tt.json), tt.content)
if tt.expectError && err == nil {
t.Error("Expected error but got none")
}
if !tt.expectError && err != nil {
t.Errorf("Unexpected error: %v", err)
}

// Verify that the Content field was properly populated for successful cases
if !tt.expectError {
if result, ok := tt.content.(*mcp.CreateMessageResult); ok {
if result.Content == nil {
t.Error("CreateMessageResult.Content was not populated")
}
}
}
})
}
}

func TestContentUnmarshalNilWithEmptyContent(t *testing.T) {
tests := []struct {
name string
json string
content interface{}
expectError bool
}{
{
name: "Empty Content array",
json: `{"content":[]}`,
content: &mcp.CallToolResult{},
expectError: false,
},
{
name: "Missing Content field",
json: `{"model":"test","role":"user"}`,
content: &mcp.CreateMessageResult{},
expectError: true, // Content field is required for CreateMessageResult
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test that unmarshaling doesn't panic on nil Content fields
defer func() {
if r := recover(); r != nil {
t.Errorf("UnmarshalJSON panicked: %v", r)
}
}()

err := json.Unmarshal([]byte(tt.json), tt.content)
if tt.expectError && err == nil {
t.Error("Expected error but got none")
}
if !tt.expectError && err != nil {
t.Errorf("Unexpected error: %v", err)
}
})
}
}

func TestContentUnmarshalNilWithInvalidContent(t *testing.T) {
tests := []struct {
name string
json string
content interface{}
expectError bool
}{
{
name: "Invalid content type",
json: `{"content":{"type":"invalid","text":"hello"}}`,
content: &mcp.CreateMessageResult{},
expectError: true,
},
{
name: "Missing type field",
json: `{"content":{"text":"hello"}}`,
content: &mcp.CreateMessageResult{},
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test that unmarshaling doesn't panic on nil Content fields
defer func() {
if r := recover(); r != nil {
t.Errorf("UnmarshalJSON panicked: %v", r)
}
}()

err := json.Unmarshal([]byte(tt.json), tt.content)
if tt.expectError && err == nil {
t.Error("Expected error but got none")
}
if !tt.expectError && err != nil {
t.Errorf("Unexpected error: %v", err)
}
})
}
}