Skip to content

Commit 85c15e0

Browse files
author
cl-ment
committed
fix(mcp): restore jsonRPCIDEchoable and validate request IDs before responding
1 parent 03df314 commit 85c15e0

2 files changed

Lines changed: 60 additions & 1 deletion

File tree

‎internal/mcp/client.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ func (client *Client) readLoop() {
388388
// A message with a Method is a server-initiated request or notification.
389389
// It must never be routed as a response to a pending client request.
390390
if message.Method != "" {
391-
if message.ID != nil {
391+
if message.ID != nil && jsonRPCIDEchoable(message.ID) {
392392
// Send the courtesy -32601 reply asynchronously off the read loop so
393393
// an undrained server stdin pipe never stalls readLoop or holds client.mu.
394394
id := message.ID
@@ -493,6 +493,27 @@ func rpcIDMatches(value any, id int) bool {
493493
}
494494
}
495495

496+
// jsonRPCIDEchoable reports whether id is a valid JSON-RPC 2.0 identifier type
497+
// (string, integer, or float with no fractional part) that is safe to echo back.
498+
func jsonRPCIDEchoable(id any) bool {
499+
if id == nil {
500+
return false
501+
}
502+
switch v := id.(type) {
503+
case string:
504+
return true
505+
case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64:
506+
return true
507+
case float64:
508+
return v == float64(int64(v))
509+
case json.Number:
510+
_, err := v.Int64()
511+
return err == nil
512+
default:
513+
return false
514+
}
515+
}
516+
496517
func (client *Client) notify(method string, params any) error {
497518
rawParams, err := json.Marshal(params)
498519
if err != nil {

‎internal/mcp/client_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,3 +949,41 @@ func TestStdioClientUndrainedServerDoesNotStallReadLoop(t *testing.T) {
949949
t.Fatal("readLoop stalled on undrained error write; pending response was not delivered")
950950
}
951951
}
952+
953+
func TestStdioClientDropsInvalidServerRequestIDs(t *testing.T) {
954+
inReader, inWriter := io.Pipe()
955+
outReader, outWriter := io.Pipe()
956+
957+
client := &Client{
958+
reader: newMessageReader(inReader),
959+
writer: newMessageWriter(outWriter),
960+
pending: make(map[int]chan dispatchResult),
961+
}
962+
defer func() {
963+
_ = inWriter.Close()
964+
_ = outReader.Close()
965+
}()
966+
967+
client.ensureReader()
968+
969+
// Server sends request with boolean ID (invalid JSON-RPC id)
970+
serverReq := `{"jsonrpc":"2.0","id":true,"method":"roots/list","params":{}}` + "\n"
971+
go func() {
972+
_, _ = inWriter.Write([]byte(serverReq))
973+
}()
974+
975+
// Read should not produce any output response for invalid ID
976+
buf := make([]byte, 1024)
977+
readChan := make(chan int, 1)
978+
go func() {
979+
n, _ := outReader.Read(buf)
980+
readChan <- n
981+
}()
982+
983+
select {
984+
case n := <-readChan:
985+
t.Fatalf("unexpected reply for invalid ID: %s", string(buf[:n]))
986+
case <-time.After(100 * time.Millisecond):
987+
// Expected: dropped cleanly
988+
}
989+
}

0 commit comments

Comments
 (0)