diff --git a/internal/jsonrpc2/invalid_request_id_test.go b/internal/jsonrpc2/invalid_request_id_test.go new file mode 100644 index 00000000..a9247df7 --- /dev/null +++ b/internal/jsonrpc2/invalid_request_id_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package jsonrpc2 + +import ( + "encoding/json" + "errors" + "testing" +) + +func TestInvalidRequestIDClassification(t *testing.T) { + for _, tt := range []struct { + name, wire string + rejectedRequest, rejected bool + }{ + {"request", `{"jsonrpc":"2.0","method":"ping","id":true}`, true, true}, + {"response", `{"jsonrpc":"2.0","result":{},"id":true}`, false, true}, + {"syntax", `{"jsonrpc":`, false, true}, + {"notification", `{"jsonrpc":"2.0","method":"ping"}`, false, false}, + {"null", `{"jsonrpc":"2.0","method":"ping","id":null}`, false, false}, + } { + t.Run(tt.name, func(t *testing.T) { + wire := []byte(tt.wire) + _, err := DecodeMessage(wire) + if (err != nil) != tt.rejected || errors.Is(err, ErrInvalidRequestID) != tt.rejectedRequest { + t.Fatalf("DecodeMessage error = %v", err) + } + }) + } +} + +func TestUnknownIDErrorEncoding(t *testing.T) { + response := &Response{Error: ErrInvalidRequest} + wire, err := EncodeMessage(response) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(wire, &fields); err != nil { + t.Fatal(err) + } + if string(fields["id"]) != "null" || fields["error"] == nil || fields["result"] != nil { + t.Fatalf("invalid error response: %s", wire) + } +} + +func TestNullIDErrorResponseRoundTrip(t *testing.T) { + original := &Response{Error: &WireError{Code: -32600, Message: "invalid request ID"}} + wire, err := EncodeMessage(original) + if err != nil { + t.Fatal(err) + } + decoded, err := DecodeMessage(wire) + if err != nil { + t.Fatal(err) + } + response, ok := decoded.(*Response) + if !ok { + t.Fatalf("decoded %T, want response", decoded) + } + var wireErr *WireError + if response.ID.IsValid() || !errors.As(response.Error, &wireErr) || wireErr.Code != -32600 { + t.Fatalf("response = %#v", response) + } +} + +func TestInvalidUnknownIDResponse(t *testing.T) { + for _, wire := range []string{ + `{"jsonrpc":"2.0","error":{"code":-32600,"message":"bad"}}`, + `{"jsonrpc":"2.0","id":null,"result":{}}`, + `{"jsonrpc":"2.0","id":null,"error":null}`, + `{"jsonrpc":"2.0","id":null,"result":null,"error":{"code":-32600,"message":"bad"}}`, + `{"jsonrpc":"2.0","ID":null,"error":{"code":-32600,"message":"bad"}}`, + } { + _, err := DecodeMessage([]byte(wire)) + if !errors.Is(err, ErrInvalidRequest) { + t.Fatalf("DecodeMessage(%s) error = %v", wire, err) + } + } +} diff --git a/internal/jsonrpc2/messages.go b/internal/jsonrpc2/messages.go index 8b967706..b3b517ca 100644 --- a/internal/jsonrpc2/messages.go +++ b/internal/jsonrpc2/messages.go @@ -122,6 +122,9 @@ func NewResponse(id ID, result any, rerr error) (*Response, error) { func (msg *Response) marshal(to *wireCombined) { to.ID = msg.ID.value + if !msg.ID.IsValid() && msg.Error != nil { + to.ID = json.RawMessage("null") + } to.Error = toWireError(msg.Error) to.Result = msg.Result } @@ -183,6 +186,9 @@ type wireDecode struct { Error *WireError `json:"error,omitempty"` } +// ErrInvalidRequestID identifies a request whose ID cannot be decoded. +var ErrInvalidRequestID = fmt.Errorf("%w: invalid request ID", ErrParse) + func DecodeMessage(data []byte) (Message, error) { msg := wireDecode{} if err := internaljson.Unmarshal(data, &msg); err != nil { @@ -193,6 +199,9 @@ func DecodeMessage(data []byte) (Message, error) { } id, err := MakeID(msg.ID) if err != nil { + if len(msg.Method) > 0 { + return nil, ErrInvalidRequestID + } return nil, err } if len(msg.Method) > 0 { @@ -209,7 +218,20 @@ func DecodeMessage(data []byte) (Message, error) { } // no method key, should be a response if !id.IsValid() { - return nil, ErrInvalidRequest + if msg.Error == nil || len(msg.Result) != 0 { + return nil, ErrInvalidRequest + } + // The interface-valued ID conflates absent and null. Check presence + // only on this error path, without adding work to ordinary messages. + var raw struct { + ID json.RawMessage `json:"id"` + } + if err := internaljson.Unmarshal(data, &raw); err != nil { + return nil, err + } + if !bytes.Equal(bytes.TrimSpace(raw.ID), []byte("null")) { + return nil, ErrInvalidRequest + } } resp := &Response{ ID: id, diff --git a/mcp/invalid_request_id_test.go b/mcp/invalid_request_id_test.go new file mode 100644 index 00000000..728dc732 --- /dev/null +++ b/mcp/invalid_request_id_test.go @@ -0,0 +1,107 @@ +// Copyright 2026 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package mcp + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/modelcontextprotocol/go-sdk/jsonrpc" +) + +type invalidIDWriter struct{ bytes.Buffer } + +func (*invalidIDWriter) Close() error { return nil } + +func TestInvalidRequestIDHTTP(t *testing.T) { + for _, id := range []string{"true", "{}", "[]"} { + calls := 0 + server := NewServer(&Implementation{Name: "synthetic", Version: "test"}, nil) + server.AddTool(&Tool{Name: "probe", InputSchema: map[string]any{"type": "object"}}, func(context.Context, *CallToolRequest) (*CallToolResult, error) { + calls++ + return &CallToolResult{}, nil + }) + handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{Stateless: true, JSONResponse: true}) + req := httptest.NewRequest(http.MethodPost, "http://localhost/mcp", strings.NewReader(`{"jsonrpc":"2.0","id":`+id+`,"method":"tools/call","params":{"name":"probe","arguments":{}}}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + result := httptest.NewRecorder() + handler.ServeHTTP(result, req) + var envelope struct { + ID json.RawMessage + Error struct{ Code int } + } + if err := json.Unmarshal(result.Body.Bytes(), &envelope); err != nil { + t.Fatal(err) + } + if result.Header().Get("Content-Type") != "application/json" || result.Code != 400 || string(envelope.ID) != "null" || envelope.Error.Code != -32600 || calls != 0 { + t.Fatalf("status%d calls%d body%s", result.Code, calls, result.Body) + } + } +} + +func TestInvalidRequestIDIORecovery(t *testing.T) { + data := strings.Repeat(`{"jsonrpc":"2.0","id":true,"method":"ping"}`+"\n", 1000) + `{"jsonrpc":"2.0","id":7,"method":"ping"}` + "\n" + writer := &invalidIDWriter{} + conn := newIOConn(rwc{rc: io.NopCloser(strings.NewReader(data)), wc: writer}) + t.Cleanup(func() { conn.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + message, err := conn.Read(ctx) + if err != nil { + t.Fatal(err) + } + if message.(*jsonrpc.Request).ID.Raw() != int64(7) { + t.Fatal("valid ID lost") + } + decoder := json.NewDecoder(bytes.NewReader(writer.Bytes())) + for range 1000 { + var envelope struct { + ID json.RawMessage + Error struct{ Code int } + } + if err := decoder.Decode(&envelope); err != nil { + t.Fatal(err) + } + if string(envelope.ID) != "null" || envelope.Error.Code != -32600 { + t.Fatal("invalid frame error") + } + } +} + +func TestIONullIDErrorResponse(t *testing.T) { + data := `{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"invalid request ID"}}` + "\n" + `{"jsonrpc":"2.0","id":7,"result":{}}` + "\n" + writer := &invalidIDWriter{} + conn := newIOConn(rwc{rc: io.NopCloser(strings.NewReader(data)), wc: writer}) + defer conn.Close() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + first, err := conn.Read(ctx) + if err != nil { + t.Fatal(err) + } + response, ok := first.(*jsonrpc.Response) + if !ok || response.ID.IsValid() || response.Error == nil { + t.Fatalf("first = %#v", first) + } + second, err := conn.Read(ctx) + if err != nil { + t.Fatal(err) + } + response, ok = second.(*jsonrpc.Response) + if !ok || response.ID.Raw() != int64(7) { + t.Fatalf("second = %#v", second) + } + if writer.Len() != 0 { + t.Fatalf("unexpected reply to error response: %s", writer.String()) + } +} diff --git a/mcp/streamable.go b/mcp/streamable.go index 813a3824..afaad469 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -1609,6 +1609,10 @@ func (c *streamableServerConn) servePOST(w http.ResponseWriter, req *http.Reques // logic. incoming, isBatch, err := readBatch(body) if err != nil { + if !isBatch && errors.Is(err, jsonrpc2.ErrInvalidRequestID) { + writeJSONRPCError(w, http.StatusBadRequest, jsonrpc.ID{}, &jsonrpc.Error{Code: jsonrpc.CodeInvalidRequest, Message: "invalid request ID"}) + return + } http.Error(w, fmt.Sprintf("malformed payload: %v", err), http.StatusBadRequest) return } diff --git a/mcp/streamable_client_test.go b/mcp/streamable_client_test.go index ae2319b2..81382476 100644 --- a/mcp/streamable_client_test.go +++ b/mcp/streamable_client_test.go @@ -1843,6 +1843,13 @@ func TestStreamableClientHandlerErrorPropagation(t *testing.T) { wantErrRejected bool // whether the returned error should match errors.Is(err, jsonrpc2.ErrRejected) wantSessionAlive bool // whether a subsequent call on the same session should succeed }{ + { + name: "400 with null-ID JSON-RPC error", + callStatus: http.StatusBadRequest, + callBody: `{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"invalid request ID"}}`, + wantErrRejected: true, + wantSessionAlive: true, + }, { // SEP-2575 ยง"Missing Required Capabilities": server returns // -32021 at HTTP 400. Session must survive. diff --git a/mcp/transport.go b/mcp/transport.go index 35c3e70b..e20c405d 100644 --- a/mcp/transport.go +++ b/mcp/transport.go @@ -671,6 +671,7 @@ type msgBatch struct { } func (t *ioConn) Read(ctx context.Context) (jsonrpc.Message, error) { +readNext: // As a matter of principle, enforce that reads on a closed context return an // error. select { @@ -701,6 +702,13 @@ func (t *ioConn) Read(ctx context.Context) (jsonrpc.Message, error) { msgs, batch, err := readBatch(raw) if err != nil { + if !batch && errors.Is(err, jsonrpc2.ErrInvalidRequestID) { + response := &jsonrpc.Response{Error: &jsonrpc.Error{Code: jsonrpc.CodeInvalidRequest, Message: "invalid request ID"}} + if err := t.Write(ctx, response); err != nil { + return nil, err + } + goto readNext + } return nil, err } var protocolVersion string