From a32acd05196500e4ffa8bd4abf02aba40b7bd487 Mon Sep 17 00:00:00 2001 From: latent-9 <296084221+latent-9@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:42:22 +1200 Subject: [PATCH] mcp: avoid panicking when a resource handler returns nil readResource dereferenced the handler's result before the nil check that was meant to guard it, so a ResourceHandler returning (nil, nil) crashed the server with a nil pointer panic instead of returning an error. Request handlers run in their own goroutine with no recover, so that panic terminates the whole process and every concurrent session, not just the offending request. The sibling handlers callTool and getPrompt already guard their result with `if err == nil && res != nil` before touching it. Move the nil check in readResource ahead of handleMultiRoundTripResult and setDefaultCacheableValues so a nil result returns the existing descriptive error, and add a regression test. --- mcp/error_test.go | 26 ++++++++++++++++++++++++++ mcp/server.go | 5 ++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/mcp/error_test.go b/mcp/error_test.go index 2b68d75ec..898562cbc 100644 --- a/mcp/error_test.go +++ b/mcp/error_test.go @@ -127,6 +127,32 @@ func TestResourceNotFoundErrorCode(t *testing.T) { } } +// TestReadResourceNilResult verifies that a resource handler returning +// (nil, nil) yields a clean error to the client instead of a nil pointer +// panic that crashes the server. The sibling handlers callTool and getPrompt +// already tolerate a (nil, nil) return; readResource must do the same. +func TestReadResourceNilResult(t *testing.T) { + ctx := context.Background() + + cs, _, cleanup := basicConnection(t, func(s *Server) { + s.AddResource( + &Resource{URI: "file:///nil.txt", Name: "nil", MIMEType: "text/plain"}, + func(ctx context.Context, req *ReadResourceRequest) (*ReadResourceResult, error) { + return nil, nil + }, + ) + }) + defer cleanup() + + _, err := cs.ReadResource(ctx, &ReadResourceParams{URI: "file:///nil.txt"}) + if err == nil { + t.Fatal("got nil error, want non-nil error for nil resource result") + } + if !strings.Contains(err.Error(), "nil information") { + t.Errorf("got error %q, want it to mention 'nil information'", err.Error()) + } +} + // TestInputValidationToolError validates that input validation errors (missing // required params, wrong types) are returned as tool results with IsError=true, // not as JSON-RPC errors. This allows LLMs to see the error and self-correct. diff --git a/mcp/server.go b/mcp/server.go index c189a8ee4..c6d12707a 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -1035,6 +1035,9 @@ func (s *Server) readResource(ctx context.Context, req *ReadResourceRequest) (*R if err != nil { return nil, err } + if res == nil { + return nil, fmt.Errorf("reading resource %s: read handler returned nil information", uri) + } if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { return nil, err } @@ -1042,7 +1045,7 @@ func (s *Server) readResource(ctx context.Context, req *ReadResourceRequest) (*R if res.resultType == resultTypeInputRequired { return res, nil } - if res == nil || res.Contents == nil { + if res.Contents == nil { return nil, fmt.Errorf("reading resource %s: read handler returned nil information", uri) } // As a convenience, populate some fields.