From bcbfe0ce3687dbec629c33756e95ce555b6d1b7b Mon Sep 17 00:00:00 2001 From: Piyush Bag Date: Tue, 16 Jun 2026 00:57:18 -0700 Subject: [PATCH] fix(jsonrpc2): decode requests when method key is present (#1000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #976. `DecodeMessage` used `msg.Method != ""` to classify messages. A wire payload with `"method":""` was decoded as a **response**, so stdio servers dropped it without a correlated JSON-RPC error. ## Approach Decode into a `wireDecode` struct whose `method` field is `json.RawMessage`. Treat the message as a **request** when the `method` key is **present on the wire** (including `""`), which matches JSON-RPC semantics. This revision: - Uses a **single JSON parse** per message (dedicated decode struct, no secondary fields-map unmarshal) - Documents why `json.RawMessage` is used for `method` - Adds regression coverage so normal responses and id-only payloads without a `method` key stay on the response path ## Test plan - [x] `go test ./internal/jsonrpc2/... -v` - [x] `go test ./mcp/... -run TestIOConnRead_EmptyMethod -v` - [x] `go test ./... -count=1` - [x] `go test -race ./internal/jsonrpc2/... -count=1` - [x] Server conformance (`npx @modelcontextprotocol/conformance server --url http://127.0.0.1:3000/mcp`) — 40 passed, 0 failed - [x] Client conformance (`./scripts/client-conformance.sh`) — 217 passed, 0 failed ## Scope note This PR addresses the primary repro in #976 (empty `method` with valid `id`). Other cases listed in the issue (`id: null`, structurally invalid envelopes) are unchanged. Co-authored-by: Guglielmo Colombo --- internal/jsonrpc2/messages.go | 26 ++++++++++++++++---- internal/jsonrpc2/wire_test.go | 45 ++++++++++++++++++++++++++++++++++ mcp/transport_test.go | 23 +++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/internal/jsonrpc2/messages.go b/internal/jsonrpc2/messages.go index 77c9531..8b96770 100644 --- a/internal/jsonrpc2/messages.go +++ b/internal/jsonrpc2/messages.go @@ -171,8 +171,20 @@ func EncodeIndent(msg Message, prefix, indent string) ([]byte, error) { return bytes.TrimRight(buf.Bytes(), "\n"), nil } +// wireDecode is the decode form of [wireCombined]. Method is a [json.RawMessage] +// so we can tell whether the "method" key was present on the wire, including +// when its value is the empty string (see go-sdk#976). +type wireDecode struct { + VersionTag string `json:"jsonrpc"` + ID any `json:"id,omitempty"` + Method json.RawMessage `json:"method"` + Params json.RawMessage `json:"params,omitempty"` + Result json.RawMessage `json:"result,omitempty"` + Error *WireError `json:"error,omitempty"` +} + func DecodeMessage(data []byte) (Message, error) { - msg := wireCombined{} + msg := wireDecode{} if err := internaljson.Unmarshal(data, &msg); err != nil { return nil, fmt.Errorf("unmarshaling jsonrpc message: %w", err) } @@ -183,15 +195,19 @@ func DecodeMessage(data []byte) (Message, error) { if err != nil { return nil, err } - if msg.Method != "" { - // has a method, must be a call + if len(msg.Method) > 0 { + // The "method" key was present. Decode its value (including ""). + var method string + if err := internaljson.Unmarshal(msg.Method, &method); err != nil { + return nil, fmt.Errorf("unmarshaling jsonrpc message: %w", err) + } return &Request{ - Method: msg.Method, + Method: method, ID: id, Params: msg.Params, }, nil } - // no method, should be a response + // no method key, should be a response if !id.IsValid() { return nil, ErrInvalidRequest } diff --git a/internal/jsonrpc2/wire_test.go b/internal/jsonrpc2/wire_test.go index 6f576f0..cf7e2b8 100644 --- a/internal/jsonrpc2/wire_test.go +++ b/internal/jsonrpc2/wire_test.go @@ -102,6 +102,51 @@ func newResponse(id any, result any, rerr error) jsonrpc2.Message { return msg } +// go-sdk#976: empty method must decode as a request (encode omits empty method). +func TestDecodeEmptyMethodRequest(t *testing.T) { + encoded := []byte(`{"jsonrpc":"2.0","id":5,"method":"","params":{}}`) + msg, err := jsonrpc2.DecodeMessage(encoded) + if err != nil { + t.Fatal(err) + } + req, ok := msg.(*jsonrpc2.Request) + if !ok { + t.Fatalf("message type = %T, want *jsonrpc2.Request", msg) + } + if req.Method != "" { + t.Errorf("Method = %q, want empty string", req.Method) + } + if req.ID != jsonrpc2.Int64ID(5) { + t.Errorf("ID = %v, want 5", req.ID.Raw()) + } + if !req.IsCall() { + t.Error("empty method with id=5 should be a call") + } +} + +func TestDecodeResponseUnchanged(t *testing.T) { + encoded := []byte(`{"jsonrpc":"2.0","id":2,"result":{}}`) + msg, err := jsonrpc2.DecodeMessage(encoded) + if err != nil { + t.Fatal(err) + } + if _, ok := msg.(*jsonrpc2.Response); !ok { + t.Fatalf("message type = %T, want *jsonrpc2.Response", msg) + } +} + +// Messages with an id but no "method" key are responses, not malformed requests. +func TestDecodeIDOnlyMessageIsResponse(t *testing.T) { + encoded := []byte(`{"jsonrpc":"2.0","id":5}`) + msg, err := jsonrpc2.DecodeMessage(encoded) + if err != nil { + t.Fatal(err) + } + if _, ok := msg.(*jsonrpc2.Response); !ok { + t.Fatalf("message type = %T, want *jsonrpc2.Response", msg) + } +} + func checkJSON(t *testing.T, got, want []byte) { // compare the compact form, to allow for formatting differences g := &bytes.Buffer{} diff --git a/mcp/transport_test.go b/mcp/transport_test.go index 515b8c1..730400e 100644 --- a/mcp/transport_test.go +++ b/mcp/transport_test.go @@ -124,3 +124,26 @@ func TestIOConnRead(t *testing.T) { }) } } + +// go-sdk#976: stdio must surface empty-method calls as requests, not responses. +func TestIOConnRead_EmptyMethod(t *testing.T) { + tr := newIOConn(rwc{ + rc: io.NopCloser(strings.NewReader(`{"jsonrpc":"2.0","id":5,"method":"","params":{}}`)), + }) + t.Cleanup(func() { tr.Close() }) + + msg, err := tr.Read(context.Background()) + if err != nil { + t.Fatalf("ioConn.Read() error = %v", err) + } + req, ok := msg.(*jsonrpc.Request) + if !ok { + t.Fatalf("message type = %T, want *jsonrpc.Request", msg) + } + if req.Method != "" { + t.Errorf("Method = %q, want empty string", req.Method) + } + if req.ID != jsonrpc2.Int64ID(5) { + t.Errorf("ID = %v, want 5", req.ID.Raw()) + } +}