mcp: handle bad or missing params

internal/jsonrpc2/wire.go

@@ -13,30 +13,30 @@ import (
 
 var (
 	// ErrParse is used when invalid JSON was received by the server.
-	ErrParse = NewError(-32700, "JSON RPC parse error")
+	ErrParse = NewError(-32700, "parse error")
 	// ErrInvalidRequest is used when the JSON sent is not a valid Request object.
-	ErrInvalidRequest = NewError(-32600, "JSON RPC invalid request")
+	ErrInvalidRequest = NewError(-32600, "invalid request")
 	// ErrMethodNotFound should be returned by the handler when the method does
 	// not exist / is not available.
-	ErrMethodNotFound = NewError(-32601, "JSON RPC method not found")
+	ErrMethodNotFound = NewError(-32601, "method not found")
 	// ErrInvalidParams should be returned by the handler when method
 	// parameter(s) were invalid.
-	ErrInvalidParams = NewError(-32602, "JSON RPC invalid params")
+	ErrInvalidParams = NewError(-32602, "invalid params")
 	// ErrInternal indicates a failure to process a call correctly
-	ErrInternal = NewError(-32603, "JSON RPC internal error")
+	ErrInternal = NewError(-32603, "internal error")
 
 	// The following errors are not part of the json specification, but
 	// compliant extensions specific to this implementation.
 
 	// ErrServerOverloaded is returned when a message was refused due to a
 	// server being temporarily unable to accept any new messages.
-	ErrServerOverloaded = NewError(-32000, "JSON RPC overloaded")
+	ErrServerOverloaded = NewError(-32000, "overloaded")
 	// ErrUnknown should be used for all non coded errors.
-	ErrUnknown = NewError(-32001, "JSON RPC unknown error")
+	ErrUnknown = NewError(-32001, "unknown error")
 	// ErrServerClosing is returned for calls that arrive while the server is closing.
-	ErrServerClosing = NewError(-32004, "JSON RPC server is closing")
+	ErrServerClosing = NewError(-32004, "server is closing")
 	// ErrClientClosing is a dummy error returned for calls initiated while the client is closing.
-	ErrClientClosing = NewError(-32003, "JSON RPC client is closing")
+	ErrClientClosing = NewError(-32003, "client is closing")
 )
 
 const wireVersion = "2.0"

mcp/client.go

@@ -286,17 +286,21 @@ func (c *Client) AddReceivingMiddleware(middleware ...Middleware[*ClientSession]
 }
 
 // clientMethodInfos maps from the RPC method name to serverMethodInfos.
+//
+// The 'allowMissingParams' values are extracted from the protocol schema.
+// TODO(rfindley): actually load and validate the protocol schema, rather than
+// curating these method flags.
 var clientMethodInfos = map[string]methodInfo{
-	methodComplete:                  newMethodInfo(sessionMethod((*ClientSession).Complete), true),
-	methodPing:                      newMethodInfo(sessionMethod((*ClientSession).ping), true),
-	methodListRoots:                 newMethodInfo(clientMethod((*Client).listRoots), true),
-	methodCreateMessage:             newMethodInfo(clientMethod((*Client).createMessage), true),
-	notificationToolListChanged:     newMethodInfo(clientMethod((*Client).callToolChangedHandler), false),
-	notificationPromptListChanged:   newMethodInfo(clientMethod((*Client).callPromptChangedHandler), false),
-	notificationResourceListChanged: newMethodInfo(clientMethod((*Client).callResourceChangedHandler), false),
-	notificationResourceUpdated:     newMethodInfo(clientMethod((*Client).callResourceUpdatedHandler), false),
-	notificationLoggingMessage:      newMethodInfo(clientMethod((*Client).callLoggingHandler), false),
-	notificationProgress:            newMethodInfo(sessionMethod((*ClientSession).callProgressNotificationHandler), false),
+	methodComplete:                  newMethodInfo(sessionMethod((*ClientSession).Complete), 0),
+	methodPing:                      newMethodInfo(sessionMethod((*ClientSession).ping), missingParamsOK),
+	methodListRoots:                 newMethodInfo(clientMethod((*Client).listRoots), missingParamsOK),
+	methodCreateMessage:             newMethodInfo(clientMethod((*Client).createMessage), 0),
+	notificationToolListChanged:     newMethodInfo(clientMethod((*Client).callToolChangedHandler), notification|missingParamsOK),
+	notificationPromptListChanged:   newMethodInfo(clientMethod((*Client).callPromptChangedHandler), notification|missingParamsOK),
+	notificationResourceListChanged: newMethodInfo(clientMethod((*Client).callResourceChangedHandler), notification|missingParamsOK),
+	notificationResourceUpdated:     newMethodInfo(clientMethod((*Client).callResourceUpdatedHandler), notification|missingParamsOK),
+	notificationLoggingMessage:      newMethodInfo(clientMethod((*Client).callLoggingHandler), notification),
+	notificationProgress:            newMethodInfo(sessionMethod((*ClientSession).callProgressNotificationHandler), notification),
 }
 
 func (cs *ClientSession) sendingMethodInfos() map[string]methodInfo {

mcp/server.go

@@ -683,23 +683,27 @@ func (s *Server) AddReceivingMiddleware(middleware ...Middleware[*ServerSession]
 }
 
 // serverMethodInfos maps from the RPC method name to serverMethodInfos.
+//
+// The 'allowMissingParams' values are extracted from the protocol schema.
+// TODO(rfindley): actually load and validate the protocol schema, rather than
+// curating these method flags.
 var serverMethodInfos = map[string]methodInfo{
-	methodComplete:               newMethodInfo(serverMethod((*Server).complete), true),
-	methodInitialize:             newMethodInfo(sessionMethod((*ServerSession).initialize), true),
-	methodPing:                   newMethodInfo(sessionMethod((*ServerSession).ping), true),
-	methodListPrompts:            newMethodInfo(serverMethod((*Server).listPrompts), true),
-	methodGetPrompt:              newMethodInfo(serverMethod((*Server).getPrompt), true),
-	methodListTools:              newMethodInfo(serverMethod((*Server).listTools), true),
-	methodCallTool:               newMethodInfo(serverMethod((*Server).callTool), true),
-	methodListResources:          newMethodInfo(serverMethod((*Server).listResources), true),
-	methodListResourceTemplates:  newMethodInfo(serverMethod((*Server).listResourceTemplates), true),
-	methodReadResource:           newMethodInfo(serverMethod((*Server).readResource), true),
-	methodSetLevel:               newMethodInfo(sessionMethod((*ServerSession).setLevel), true),
-	methodSubscribe:              newMethodInfo(serverMethod((*Server).subscribe), true),
-	methodUnsubscribe:            newMethodInfo(serverMethod((*Server).unsubscribe), true),
-	notificationInitialized:      newMethodInfo(serverMethod((*Server).callInitializedHandler), false),
-	notificationRootsListChanged: newMethodInfo(serverMethod((*Server).callRootsListChangedHandler), false),
-	notificationProgress:         newMethodInfo(sessionMethod((*ServerSession).callProgressNotificationHandler), false),
+	methodComplete:               newMethodInfo(serverMethod((*Server).complete), 0),
+	methodInitialize:             newMethodInfo(sessionMethod((*ServerSession).initialize), 0),
+	methodPing:                   newMethodInfo(sessionMethod((*ServerSession).ping), missingParamsOK),
+	methodListPrompts:            newMethodInfo(serverMethod((*Server).listPrompts), missingParamsOK),
+	methodGetPrompt:              newMethodInfo(serverMethod((*Server).getPrompt), 0),
+	methodListTools:              newMethodInfo(serverMethod((*Server).listTools), missingParamsOK),
+	methodCallTool:               newMethodInfo(serverMethod((*Server).callTool), 0),
+	methodListResources:          newMethodInfo(serverMethod((*Server).listResources), missingParamsOK),
+	methodListResourceTemplates:  newMethodInfo(serverMethod((*Server).listResourceTemplates), missingParamsOK),
+	methodReadResource:           newMethodInfo(serverMethod((*Server).readResource), 0),
+	methodSetLevel:               newMethodInfo(sessionMethod((*ServerSession).setLevel), 0),
+	methodSubscribe:              newMethodInfo(serverMethod((*Server).subscribe), 0),
+	methodUnsubscribe:            newMethodInfo(serverMethod((*Server).unsubscribe), 0),
+	notificationInitialized:      newMethodInfo(serverMethod((*Server).callInitializedHandler), notification|missingParamsOK),
+	notificationRootsListChanged: newMethodInfo(serverMethod((*Server).callRootsListChangedHandler), notification|missingParamsOK),
+	notificationProgress:         newMethodInfo(sessionMethod((*ServerSession).callProgressNotificationHandler), notification),
 }
 
 func (ss *ServerSession) sendingMethodInfos() map[string]methodInfo { return clientMethodInfos }
@@ -744,6 +748,9 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any,
 }
 
 func (ss *ServerSession) initialize(ctx context.Context, params *InitializeParams) (*InitializeResult, error) {
+	if params == nil {
+		return nil, fmt.Errorf("%w: \"params\" must be be provided", jsonrpc2.ErrInvalidParams)
+	}
 	ss.mu.Lock()
 	ss.initializeParams = params
 	ss.mu.Unlock()

mcp/shared.go

@@ -129,7 +129,7 @@ func handleReceive[S Session](ctx context.Context, session S, req *jsonrpc.Reque
 	}
 	params, err := info.unmarshalParams(req.Params)
 	if err != nil {
-		return nil, fmt.Errorf("handleRequest %q: %w", req.Method, err)
+		return nil, fmt.Errorf("handling '%s': %w", req.Method, err)
 	}
 
 	mh := session.receivingMethodHandler().(MethodHandler[S])
@@ -154,20 +154,28 @@ func checkRequest(req *jsonrpc.Request, infos map[string]methodInfo) (methodInfo
 	if !ok {
 		return methodInfo{}, fmt.Errorf("%w: %q unsupported", jsonrpc2.ErrNotHandled, req.Method)
 	}
-	if info.isRequest && !req.ID.IsValid() {
-		return methodInfo{}, fmt.Errorf("%w: missing ID, %q", jsonrpc2.ErrInvalidRequest, req.Method)
-	}
-	if !info.isRequest && req.ID.IsValid() {
+	if info.flags&notification != 0 && req.ID.IsValid() {
 		return methodInfo{}, fmt.Errorf("%w: unexpected id for %q", jsonrpc2.ErrInvalidRequest, req.Method)
 	}
+	if info.flags&notification == 0 && !req.ID.IsValid() {
+		return methodInfo{}, fmt.Errorf("%w: missing id for %q", jsonrpc2.ErrInvalidRequest, req.Method)
+	}
+	// missingParamsOK is checked here to catch the common case where "params" is
+	// missing entirely.
+	//
+	// However, it's checked again after unmarshalling to catch the rare but
+	// possible case where "params" is JSON null (see https://go.dev/issue/33835).
+	if info.flags&missingParamsOK == 0 && len(req.Params) == 0 {
+		return methodInfo{}, fmt.Errorf("%w: missing required \"params\"", jsonrpc2.ErrInvalidRequest)
+	}
 	return info, nil
 }
 
 // methodInfo is information about sending and receiving a method.
 type methodInfo struct {
-	// isRequest reports whether the method is a JSON-RPC request.
-	// Otherwise, the method is treated as a notification.
-	isRequest bool
+	// flags is a collection of flags controlling how the JSONRPC method is
+	// handled. See individual flag values for documentation.
+	flags methodFlags
 	// Unmarshal params from the wire into a Params struct.
 	// Used on the receive side.
 	unmarshalParams func(json.RawMessage) (Params, error)
@@ -193,20 +201,37 @@ type paramsPtr[T any] interface {
 	Params
 }
 
+type methodFlags int
+
+const (
+	notification    methodFlags = 1 << iota // method is a notification, not request
+	missingParamsOK                         // params may be missing or null
+)
+
 // newMethodInfo creates a methodInfo from a typedMethodHandler.
 //
 // If isRequest is set, the method is treated as a request rather than a
 // notification.
-func newMethodInfo[S Session, P paramsPtr[T], R Result, T any](d typedMethodHandler[S, P, R], isRequest bool) methodInfo {
+func newMethodInfo[S Session, P paramsPtr[T], R Result, T any](d typedMethodHandler[S, P, R], flags methodFlags) methodInfo {
 	return methodInfo{
-		isRequest: isRequest,
+		flags: flags,
 		unmarshalParams: func(m json.RawMessage) (Params, error) {
 			var p P
 			if m != nil {
 				if err := json.Unmarshal(m, &p); err != nil {
 					return nil, fmt.Errorf("unmarshaling %q into a %T: %w", m, p, err)
 				}
 			}
+			// We must check missingParamsOK here, in addition to checkRequest, to
+			// catch the edge cases where "params" is set to JSON null.
+			// See also https://go.dev/issue/33835.
+			//
+			// We need to ensure that p is non-null to guard against crashes, as our
+			// internal code or externally provided handlers may assume that params
+			// is non-null.
+			if flags&missingParamsOK == 0 && p == nil {
+				return nil, fmt.Errorf("%w: missing required \"params\"", jsonrpc2.ErrInvalidRequest)
+			}
 			return orZero[Params](p), nil
 		},
 		handleMethod: MethodHandler[S](func(ctx context.Context, session S, _ string, params Params) (Result, error) {

mcp/sse_test.go

@@ -88,7 +88,7 @@ func TestSSEServer(t *testing.T) {
 					responseContains string
 				}{
 					{"not a method", `{"jsonrpc":"2.0", "method":"notamethod"}`, "not handled"},
-					{"missing ID", `{"jsonrpc":"2.0", "method":"ping"}`, "missing ID"},
+					{"missing ID", `{"jsonrpc":"2.0", "method":"ping"}`, "missing id"},
 				}
 				for _, r := range badRequests {
 					t.Run(r.name, func(t *testing.T) {

mcp/testdata/conformance/server/bad_requests.txtar

@@ -4,10 +4,11 @@ bad requests.
 Fixed bugs:
 - No id in 'initialize' should not panic (#197).
 - No id in 'ping' should not panic (#194).
-- Notifications with IDs should not be treated like requests.
-
-TODO:
 - No params in 'initialize' should not panic (#195).
+- Notifications with IDs should not be treated like requests. (#196)
+- No params in 'logging/setLevel' should not panic.
+- No params in 'completion/complete' should not panic.
+- JSON null params should also not panic in these cases.
 
 -- prompts --
 code_review
@@ -22,6 +23,11 @@ code_review
     "clientInfo": { "name": "ExampleClient", "version": "1.0.0" }
   }
 }
+{
+  "jsonrpc": "2.0",
+  "id": 1,
+  "method": "initialize"
+}
 {
   "jsonrpc": "2.0",
   "id": 2,
@@ -32,10 +38,21 @@ code_review
     "clientInfo": { "name": "ExampleClient", "version": "1.0.0" }
   }
 }
-{"jsonrpc":"2.0", "id": 3, "method":"notifications/initialized"}
+{"jsonrpc":"2.0", "id": 3, "method": "notifications/initialized"}
 {"jsonrpc":"2.0", "method":"ping"}
+{"jsonrpc":"2.0", "id": 4, "method": "logging/setLevel"}
+{"jsonrpc":"2.0", "id": 5, "method": "completion/complete"}
+{"jsonrpc":"2.0", "id": 4, "method": "logging/setLevel", "params": null}
 
 -- server --
+{
+	"jsonrpc": "2.0",
+	"id": 1,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}
 {
 	"jsonrpc": "2.0",
 	"id": 2,
@@ -59,6 +76,30 @@ code_review
 	"id": 3,
 	"error": {
 		"code": -32600,
-		"message": "JSON RPC invalid request: unexpected id for \"notifications/initialized\""
+		"message": "invalid request: unexpected id for \"notifications/initialized\""
+	}
+}
+{
+	"jsonrpc": "2.0",
+	"id": 4,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}
+{
+	"jsonrpc": "2.0",
+	"id": 5,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}
+{
+	"jsonrpc": "2.0",
+	"id": 4,
+	"error": {
+		"code": -32600,
+		"message": "handling 'logging/setLevel': invalid request: missing required \"params\""
 	}
 }

mcp/testdata/conformance/server/prompts.txtar

@@ -1,7 +1,8 @@
 Check behavior of a server with just prompts.
 
 Fixed bugs:
-- empty tools lists should not be returned as 'null'
+- Empty tools lists should not be returned as 'null'.
+- No params in 'prompts/get' should not panic.
 
 -- prompts --
 code_review
@@ -19,6 +20,7 @@ code_review
 }
 { "jsonrpc": "2.0", "id": 2, "method": "tools/list" }
 { "jsonrpc": "2.0", "id": 4, "method": "prompts/list" }
+{ "jsonrpc": "2.0", "id": 5, "method": "prompts/get" }
 -- server --
 {
 	"jsonrpc": "2.0",
@@ -63,3 +65,11 @@ code_review
 		]
 	}
 }
+{
+	"jsonrpc": "2.0",
+	"id": 5,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}

mcp/testdata/conformance/server/resources.txtar

@@ -2,6 +2,9 @@ Check behavior of a server with just resources.
 
 Fixed bugs:
 - A resource result holds a slice of contents, not just one.
+- No params in 'resource/read' should not panic.
+- No params in 'resources/subscribe' should not panic.
+- No params in 'resources/unsubscribe' should not panic.
 
 -- resources --
 info
@@ -39,6 +42,8 @@ info.txt
 	"roots": []
   }
 }
+{ "jsonrpc": "2.0", "id": 4, "method": "resources/read" }
+{ "jsonrpc": "2.0", "id": 5, "method": "resources/subscribe" }
 -- server --
 {
 	"jsonrpc": "2.0",
@@ -107,3 +112,19 @@ info.txt
 		]
 	}
 }
+{
+	"jsonrpc": "2.0",
+	"id": 4,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}
+{
+	"jsonrpc": "2.0",
+	"id": 5,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}

mcp/testdata/conformance/server/tools.txtar

@@ -4,6 +4,7 @@ Fixed bugs:
 - "tools/list" can have missing params
 - "_meta" should not be nil
 - empty resource or prompts should not be returned as 'null'
+- the server should not crash when params are passed to tools/call
 
 -- tools --
 greet
@@ -22,6 +23,7 @@ greet
 { "jsonrpc": "2.0", "id": 2, "method": "tools/list" }
 { "jsonrpc": "2.0", "id": 3, "method": "resources/list" }
 { "jsonrpc": "2.0", "id": 4, "method": "prompts/list" }
+{ "jsonrpc": "2.0", "id": 5, "method": "tools/call" }
 -- server --
 {
 	"jsonrpc": "2.0",
@@ -81,3 +83,11 @@ greet
 		"prompts": []
 	}
 }
+{
+	"jsonrpc": "2.0",
+	"id": 5,
+	"error": {
+		"code": -32600,
+		"message": "invalid request: missing required \"params\""
+	}
+}