Skip to content

Commit 0b18d58

Browse files
twishabansalYuan325
twishabansal
and
Yuan325
authored
fix(server): return null id for batch request rejection (googleapis#3333)
## Description > Should include a concise description of the changes (bug or feature), it's > impact, along with a summary of the solution ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [ ] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/mcp-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/mcp-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes googleapis#3332 --------- Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
1 parent 72262d7 commit 0b18d58

2 files changed

Lines changed: 10 additions & 11 deletions

File tree

‎internal/server/mcp.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,11 @@ func httpHandler(s *Server, w http.ResponseWriter, r *http.Request) {
495495
// Read body first so we can extract trace context
496496
body, err := io.ReadAll(r.Body)
497497
if err != nil {
498-
// Generate a new uuid if unable to decode
499-
id := uuid.New().String()
498+
// The id cannot be determined from an unreadable body. Per JSON-RPC 2.0,
499+
// the response id MUST be null in that case.
500+
// See https://www.jsonrpc.org/specification#response_object
500501
s.logger.DebugContext(ctx, err.Error())
501-
render.JSON(w, r, jsonrpc.NewError(id, jsonrpc.PARSE_ERROR, err.Error(), nil))
502+
render.JSON(w, r, jsonrpc.NewError(nil, jsonrpc.PARSE_ERROR, err.Error(), nil))
502503
return
503504
}
504505

@@ -639,18 +640,19 @@ func processMcpMessage(ctx context.Context, body []byte, s *Server, protocolVers
639640
// Generic baseMessage could either be a JSONRPCNotification or JSONRPCRequest
640641
var baseMessage jsonrpc.BaseMessage
641642
if err = util.DecodeJSON(bytes.NewBuffer(body), &baseMessage); err != nil {
642-
// Generate a new uuid if unable to decode
643-
id := uuid.New().String()
643+
// The id cannot be determined from an undecodable body (batch or parse
644+
// error). Per JSON-RPC 2.0, the response id MUST be null in that case.
645+
// See https://www.jsonrpc.org/specification#response_object
644646

645647
// check if user is sending a batch request
646648
var a []any
647649
unmarshalErr := json.Unmarshal(body, &a)
648650
if unmarshalErr == nil {
649651
err = fmt.Errorf("not supporting batch requests")
650-
return "", jsonrpc.NewError(id, jsonrpc.INVALID_REQUEST, err.Error(), nil), err
652+
return "", jsonrpc.NewError(nil, jsonrpc.INVALID_REQUEST, err.Error(), nil), err
651653
}
652654

653-
return "", jsonrpc.NewError(id, jsonrpc.PARSE_ERROR, err.Error(), nil), err
655+
return "", jsonrpc.NewError(nil, jsonrpc.PARSE_ERROR, err.Error(), nil), err
654656
}
655657

656658
// Check if method is present

‎internal/server/mcp_test.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,7 @@ func TestMcpEndpoint(t *testing.T) {
788788
wantStatusCode: http.StatusOK,
789789
want: map[string]any{
790790
"jsonrpc": "2.0",
791+
"id": nil,
791792
"error": map[string]any{
792793
"code": -32600.0,
793794
"message": "not supporting batch requests",
@@ -899,10 +900,6 @@ func TestMcpEndpoint(t *testing.T) {
899900
if err := json.Unmarshal(body, &got); err != nil {
900901
t.Fatalf("unexpected error unmarshalling body: %s", err)
901902
}
902-
// for decode failure, a random uuid is generated in server
903-
if tc.want["id"] == nil {
904-
tc.want["id"] = got["id"]
905-
}
906903
if !reflect.DeepEqual(got, tc.want) {
907904
t.Fatalf("unexpected response: got %+v, want %+v", got, tc.want)
908905
}

0 commit comments

Comments
 (0)