Skip to content

Commit 1656060

Browse files
committed
fix(jsonrpc2): decode requests when method key is present
Use json.RawMessage for the method field so DecodeMessage can distinguish a missing method key from an explicitly empty method string. Fixes the silent stdio drop in #976 with a single JSON parse (no second fields-map unmarshal). Fixes #976
1 parent dd97816 commit 1656060

3 files changed

Lines changed: 89 additions & 5 deletions

File tree

internal/jsonrpc2/messages.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,20 @@ func EncodeIndent(msg Message, prefix, indent string) ([]byte, error) {
171171
return bytes.TrimRight(buf.Bytes(), "\n"), nil
172172
}
173173

174+
// wireDecode is the decode form of [wireCombined]. Method is a [json.RawMessage]
175+
// so we can tell whether the "method" key was present on the wire, including
176+
// when its value is the empty string (see go-sdk#976).
177+
type wireDecode struct {
178+
VersionTag string `json:"jsonrpc"`
179+
ID any `json:"id,omitempty"`
180+
Method json.RawMessage `json:"method"`
181+
Params json.RawMessage `json:"params,omitempty"`
182+
Result json.RawMessage `json:"result,omitempty"`
183+
Error *WireError `json:"error,omitempty"`
184+
}
185+
174186
func DecodeMessage(data []byte) (Message, error) {
175-
msg := wireCombined{}
187+
msg := wireDecode{}
176188
if err := internaljson.Unmarshal(data, &msg); err != nil {
177189
return nil, fmt.Errorf("unmarshaling jsonrpc message: %w", err)
178190
}
@@ -183,15 +195,19 @@ func DecodeMessage(data []byte) (Message, error) {
183195
if err != nil {
184196
return nil, err
185197
}
186-
if msg.Method != "" {
187-
// has a method, must be a call
198+
if len(msg.Method) > 0 {
199+
// The "method" key was present. Decode its value (including "").
200+
var method string
201+
if err := internaljson.Unmarshal(msg.Method, &method); err != nil {
202+
return nil, fmt.Errorf("unmarshaling jsonrpc message: %w", err)
203+
}
188204
return &Request{
189-
Method: msg.Method,
205+
Method: method,
190206
ID: id,
191207
Params: msg.Params,
192208
}, nil
193209
}
194-
// no method, should be a response
210+
// no method key, should be a response
195211
if !id.IsValid() {
196212
return nil, ErrInvalidRequest
197213
}

internal/jsonrpc2/wire_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,51 @@ func newResponse(id any, result any, rerr error) jsonrpc2.Message {
102102
return msg
103103
}
104104

105+
// go-sdk#976: empty method must decode as a request (encode omits empty method).
106+
func TestDecodeEmptyMethodRequest(t *testing.T) {
107+
encoded := []byte(`{"jsonrpc":"2.0","id":5,"method":"","params":{}}`)
108+
msg, err := jsonrpc2.DecodeMessage(encoded)
109+
if err != nil {
110+
t.Fatal(err)
111+
}
112+
req, ok := msg.(*jsonrpc2.Request)
113+
if !ok {
114+
t.Fatalf("message type = %T, want *jsonrpc2.Request", msg)
115+
}
116+
if req.Method != "" {
117+
t.Errorf("Method = %q, want empty string", req.Method)
118+
}
119+
if req.ID != jsonrpc2.Int64ID(5) {
120+
t.Errorf("ID = %v, want 5", req.ID.Raw())
121+
}
122+
if !req.IsCall() {
123+
t.Error("empty method with id=5 should be a call")
124+
}
125+
}
126+
127+
func TestDecodeResponseUnchanged(t *testing.T) {
128+
encoded := []byte(`{"jsonrpc":"2.0","id":2,"result":{}}`)
129+
msg, err := jsonrpc2.DecodeMessage(encoded)
130+
if err != nil {
131+
t.Fatal(err)
132+
}
133+
if _, ok := msg.(*jsonrpc2.Response); !ok {
134+
t.Fatalf("message type = %T, want *jsonrpc2.Response", msg)
135+
}
136+
}
137+
138+
// Messages with an id but no "method" key are responses, not malformed requests.
139+
func TestDecodeIDOnlyMessageIsResponse(t *testing.T) {
140+
encoded := []byte(`{"jsonrpc":"2.0","id":5}`)
141+
msg, err := jsonrpc2.DecodeMessage(encoded)
142+
if err != nil {
143+
t.Fatal(err)
144+
}
145+
if _, ok := msg.(*jsonrpc2.Response); !ok {
146+
t.Fatalf("message type = %T, want *jsonrpc2.Response", msg)
147+
}
148+
}
149+
105150
func checkJSON(t *testing.T, got, want []byte) {
106151
// compare the compact form, to allow for formatting differences
107152
g := &bytes.Buffer{}

mcp/transport_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,26 @@ func TestIOConnRead(t *testing.T) {
124124
})
125125
}
126126
}
127+
128+
// go-sdk#976: stdio must surface empty-method calls as requests, not responses.
129+
func TestIOConnRead_EmptyMethod(t *testing.T) {
130+
tr := newIOConn(rwc{
131+
rc: io.NopCloser(strings.NewReader(`{"jsonrpc":"2.0","id":5,"method":"","params":{}}`)),
132+
})
133+
t.Cleanup(func() { tr.Close() })
134+
135+
msg, err := tr.Read(context.Background())
136+
if err != nil {
137+
t.Fatalf("ioConn.Read() error = %v", err)
138+
}
139+
req, ok := msg.(*jsonrpc.Request)
140+
if !ok {
141+
t.Fatalf("message type = %T, want *jsonrpc.Request", msg)
142+
}
143+
if req.Method != "" {
144+
t.Errorf("Method = %q, want empty string", req.Method)
145+
}
146+
if req.ID != jsonrpc2.Int64ID(5) {
147+
t.Errorf("ID = %v, want 5", req.ID.Raw())
148+
}
149+
}

0 commit comments

Comments
 (0)