Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
82 changes: 82 additions & 0 deletions internal/jsonrpc2/invalid_request_id_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
24 changes: 23 additions & 1 deletion internal/jsonrpc2/messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +125 to +126

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Teach DecodeMessage to accept the emitted null ID

This branch makes unknown-ID errors encode with "id":null, but DecodeMessage cannot consume that wire form: after MakeID(nil), the unchanged if !id.IsValid() branch at internal/jsonrpc2/messages.go:220 returns ErrInvalidRequest even when msg.Error is populated. Consequently the IO reader and the streamable client's non-2xx-body parser treat the new -32600 response as a transport/read failure (potentially tearing down the session) rather than a JSON-RPC error; accept explicit null-ID error responses and cover an encode/decode round trip.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed in 3e9420c. DecodeMessage now accepts an explicit null-ID error response while still rejecting missing response IDs, null-ID success responses, and null-ID responses with both result and error. Added encode/decode round-trip coverage, an IO response-followed-by-response regression, and an HTTP client regression proving the session survives the rejection and a subsequent call succeeds. All three regressions failed on the previous head; full tests, race for jsonrpc2/mcp, vet and build now pass. The presence check is confined to the unknown-ID error path; ordinary decoding and unmatched-response correlation are unchanged.

}
to.Error = toWireError(msg.Error)
to.Result = msg.Result
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
Expand Down
107 changes: 107 additions & 0 deletions mcp/invalid_request_id_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
}
4 changes: 4 additions & 0 deletions mcp/streamable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 7 additions & 0 deletions mcp/streamable_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions mcp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading