Skip to content

Commit bfbe3ba

Browse files
matttmattt
authored
Fix request query encoding of array parameters (#41)
1 parent beb1915 commit bfbe3ba

File tree

2 files changed

+90
-159
lines changed

2 files changed

+90
-159
lines changed

‎mcp/server.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,18 @@ func (s *Server) handleToolsCall(request *ToolCallRequest) (*ToolCallResponse, e
444444
value := fmt.Sprint(value)
445445
u.Path = strings.ReplaceAll(u.Path, "{"+param.Name+"}", pathSegmentEscape(value))
446446
case "query":
447-
queryParams.Set(param.Name, fmt.Sprint(value))
447+
// Handle array values for query parameters
448+
switch v := value.(type) {
449+
case []interface{}:
450+
// Join array values with commas for parameters like tweet.fields
451+
values := make([]string, len(v))
452+
for i, item := range v {
453+
values[i] = fmt.Sprint(item)
454+
}
455+
queryParams.Set(param.Name, strings.Join(values, ","))
456+
default:
457+
queryParams.Set(param.Name, fmt.Sprint(value))
458+
}
448459
case "header":
449460
headerParams.Add(param.Name, fmt.Sprint(value))
450461
}

‎mcp/server_test.go

Lines changed: 78 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package mcp
22

33
import (
4-
"encoding/base64"
54
"encoding/json"
65
"fmt"
76
"net/http"
87
"net/http/httptest"
8+
"net/url"
99
"path"
1010
"strings"
1111
"testing"
@@ -35,6 +35,17 @@ func newTestSpec(serverURL string) []byte {
3535
"parameters": []map[string]interface{}{
3636
{"name": "limit", "in": "query", "description": "Maximum number of pets to return", "schema": map[string]interface{}{"type": "integer"}},
3737
{"name": "type", "in": "query", "description": "Type of pets to filter by", "schema": map[string]interface{}{"type": "string"}},
38+
{
39+
"name": "fields",
40+
"in": "query",
41+
"description": "Fields to return",
42+
"schema": map[string]interface{}{
43+
"type": "array",
44+
"items": map[string]interface{}{
45+
"type": "string",
46+
},
47+
},
48+
},
3849
},
3950
},
4051
"post": map[string]interface{}{
@@ -363,28 +374,38 @@ func TestHandleToolsCall(t *testing.T) {
363374
server, ts := setupTestServer(t)
364375
defer ts.Close()
365376

366-
// Test with auth header
367-
serverWithAuth, _ := setupTestServer(t)
368-
369-
// Create a small test image
370-
imgData := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} // PNG header
371-
372377
tests := []struct {
373378
name string
374379
server *Server
380+
setup func(*testing.T, *httptest.Server) http.HandlerFunc
375381
request jsonrpc.Request
376-
validate func(*testing.T, jsonrpc.Response)
382+
validate func(*testing.T, jsonrpc.Response, string)
377383
}{
378384
{
379-
name: "GET request with query parameters",
380-
server: server,
385+
name: "GET request with query parameters",
386+
server: server,
387+
setup: func(t *testing.T, ts *httptest.Server) http.HandlerFunc {
388+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
389+
// Verify query parameters are present
390+
limit := r.URL.Query().Get("limit")
391+
petType := r.URL.Query().Get("type")
392+
assert.Equal(t, "5", limit)
393+
assert.Equal(t, "dog", petType)
394+
395+
w.Header().Set("Content-Type", "application/json")
396+
pets := []map[string]interface{}{
397+
{"id": 1, "name": "Fluffy", "type": "dog"},
398+
{"id": 2, "name": "Rover", "type": "dog"},
399+
}
400+
json.NewEncoder(w).Encode(pets)
401+
})
402+
},
381403
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "listPets", "arguments": {"limit": 5, "type": "dog"}}`), 1),
382-
validate: func(t *testing.T, response jsonrpc.Response) {
404+
validate: func(t *testing.T, response jsonrpc.Response, url string) {
383405
assert.Equal(t, "2.0", response.Version)
384406
assert.Equal(t, 1, response.ID.Value())
385407
assert.Nil(t, response.Error)
386408

387-
// Convert response.Result to ToolCallResponse
388409
var result ToolCallResponse
389410
resultBytes, err := json.Marshal(response.Result)
390411
require.NoError(t, err)
@@ -399,7 +420,6 @@ func TestHandleToolsCall(t *testing.T) {
399420
assert.NotNil(t, content.Annotations)
400421
assert.Contains(t, content.Annotations.Audience, RoleAssistant)
401422

402-
// Unmarshal the response into a TextContent to get the text
403423
var textContent Content
404424
contentBytes, err := json.Marshal(content)
405425
assert.NoError(t, err)
@@ -411,106 +431,42 @@ func TestHandleToolsCall(t *testing.T) {
411431
assert.NoError(t, err)
412432
assert.Len(t, pets, 2)
413433

414-
// Verify the returned pets have the correct type
415434
for _, pet := range pets {
416435
petMap := pet.(map[string]interface{})
417436
assert.Equal(t, "dog", petMap["type"])
418437
}
419438
},
420439
},
421440
{
422-
name: "POST request with body parameters",
423-
server: server,
424-
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "createPet", "arguments": {"name": "Whiskers", "age": 5}}`), 2),
425-
validate: func(t *testing.T, response jsonrpc.Response) {
426-
assert.Equal(t, "2.0", response.Version)
427-
assert.Equal(t, 2, response.ID.Value())
428-
assert.Nil(t, response.Error)
429-
430-
// Convert response.Result to ToolCallResponse
431-
var result ToolCallResponse
432-
resultBytes, err := json.Marshal(response.Result)
433-
require.NoError(t, err)
434-
err = json.Unmarshal(resultBytes, &result)
435-
require.NoError(t, err)
436-
437-
assert.Len(t, result.Content, 1)
438-
assert.False(t, result.IsError)
439-
440-
content := result.Content[0]
441-
assert.Equal(t, "text", content.Type)
442-
assert.NotNil(t, content.Annotations)
443-
assert.Contains(t, content.Annotations.Audience, RoleAssistant)
444-
445-
var textContent Content
446-
contentBytes, err := json.Marshal(content)
447-
assert.NoError(t, err)
448-
err = json.Unmarshal(contentBytes, &textContent)
449-
assert.NoError(t, err)
450-
451-
var pet map[string]interface{}
452-
err = json.Unmarshal([]byte(textContent.Text), &pet)
453-
assert.NoError(t, err)
454-
assert.Equal(t, "Whiskers", pet["name"])
455-
assert.Equal(t, float64(5), pet["age"])
456-
assert.Equal(t, float64(3), pet["id"])
457-
},
458-
},
459-
{
460-
name: "GET image request",
461-
server: server,
462-
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "getPetImage"}`), 3),
463-
validate: func(t *testing.T, response jsonrpc.Response) {
464-
assert.Equal(t, "2.0", response.Version)
465-
assert.Equal(t, 3, response.ID.Value())
466-
assert.Nil(t, response.Error)
467-
468-
// Convert response.Result to ToolCallResponse
469-
var result ToolCallResponse
470-
resultBytes, err := json.Marshal(response.Result)
471-
require.NoError(t, err)
472-
err = json.Unmarshal(resultBytes, &result)
473-
require.NoError(t, err)
474-
475-
assert.Len(t, result.Content, 1)
476-
assert.False(t, result.IsError)
477-
478-
content := result.Content[0]
479-
assert.Equal(t, "image", content.Type)
480-
assert.NotNil(t, content.Annotations)
481-
assert.Contains(t, content.Annotations.Audience, RoleAssistant)
482-
483-
var imageContent Content
484-
contentBytes, err := json.Marshal(content)
485-
assert.NoError(t, err)
486-
err = json.Unmarshal(contentBytes, &imageContent)
487-
assert.NoError(t, err)
488-
489-
assert.Equal(t, "image/png", imageContent.MimeType)
490-
491-
decoded, err := base64.StdEncoding.DecodeString(imageContent.Data)
492-
assert.NoError(t, err)
493-
assert.Equal(t, imgData, decoded)
494-
},
495-
},
496-
{
497-
name: "Request with invalid operationId",
498-
server: server,
499-
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "nonexistentOperation"}`), 4),
500-
validate: func(t *testing.T, response jsonrpc.Response) {
501-
assert.Equal(t, "2.0", response.Version)
502-
assert.Equal(t, 4, response.ID.Value())
503-
assert.Equal(t, jsonrpc.ErrMethodNotFound, response.Error.Code)
504-
assert.Equal(t, "Method not found", response.Error.Message)
441+
name: "GET request with array query parameters",
442+
server: server,
443+
setup: func(t *testing.T, ts *httptest.Server) http.HandlerFunc {
444+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
445+
// Verify the query parameters
446+
assert.Equal(t, "dog", r.URL.Query().Get("type"))
447+
// The fields parameter should be a comma-separated list
448+
assert.Equal(t, "name,age,breed", r.URL.Query().Get("fields"))
449+
450+
w.Header().Set("Content-Type", "application/json")
451+
json.NewEncoder(w).Encode(map[string]interface{}{
452+
"success": true,
453+
"query": map[string]string{
454+
"type": r.URL.Query().Get("type"),
455+
"fields": r.URL.Query().Get("fields"),
456+
},
457+
})
458+
})
505459
},
506-
},
507-
{
508-
name: "GET request with URL escaped parameters",
509-
server: server,
510-
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "getPet", "arguments": {"petId": "special pet"}}`), 5),
511-
validate: func(t *testing.T, response jsonrpc.Response) {
460+
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{
461+
"name": "listPets",
462+
"arguments": {
463+
"type": "dog",
464+
"fields": ["name", "age", "breed"]
465+
}
466+
}`), 7),
467+
validate: func(t *testing.T, response jsonrpc.Response, requestURL string) {
512468
assert.Equal(t, "2.0", response.Version)
513-
assert.Equal(t, 5, response.ID.Value())
469+
assert.Equal(t, 7, response.ID.Value())
514470
assert.Nil(t, response.Error)
515471

516472
var result ToolCallResponse
@@ -522,74 +478,38 @@ func TestHandleToolsCall(t *testing.T) {
522478
assert.Len(t, result.Content, 1)
523479
assert.False(t, result.IsError)
524480

525-
content := result.Content[0]
526-
assert.Equal(t, "text", content.Type)
527-
528-
var textContent Content
529-
contentBytes, err := json.Marshal(content)
530-
assert.NoError(t, err)
531-
err = json.Unmarshal(contentBytes, &textContent)
532-
assert.NoError(t, err)
533-
534-
var pet map[string]interface{}
535-
err = json.Unmarshal([]byte(textContent.Text), &pet)
536-
assert.NoError(t, err)
537-
assert.Equal(t, "Special Pet", pet["name"])
538-
},
539-
},
540-
{
541-
name: "Request with auth header",
542-
server: serverWithAuth,
543-
request: jsonrpc.NewRequest("tools/call", json.RawMessage(`{"name": "listPets", "arguments": {"limit": 5, "type": "dog"}}`), 6),
544-
validate: func(t *testing.T, response jsonrpc.Response) {
545-
assert.Equal(t, "2.0", response.Version)
546-
assert.Equal(t, 6, response.ID.Value())
547-
assert.Nil(t, response.Error)
548-
549-
var result ToolCallResponse
550-
resultBytes, err := json.Marshal(response.Result)
551-
require.NoError(t, err)
552-
err = json.Unmarshal(resultBytes, &result)
481+
// Parse the URL to verify parameters
482+
parsedURL, err := url.Parse(requestURL)
553483
require.NoError(t, err)
554484

555-
assert.Len(t, result.Content, 1)
556-
assert.False(t, result.IsError)
557-
558-
// Verify the response content
559-
content := result.Content[0]
560-
assert.Equal(t, "text", content.Type)
561-
assert.NotNil(t, content.Annotations)
562-
assert.Contains(t, content.Annotations.Audience, RoleAssistant)
563-
564-
var textContent Content
565-
contentBytes, err := json.Marshal(content)
566-
assert.NoError(t, err)
567-
err = json.Unmarshal(contentBytes, &textContent)
568-
assert.NoError(t, err)
569-
570-
var pets []interface{}
571-
err = json.Unmarshal([]byte(textContent.Text), &pets)
572-
assert.NoError(t, err)
573-
assert.Len(t, pets, 2)
574-
575-
// Verify the returned pets
576-
for _, pet := range pets {
577-
petMap := pet.(map[string]interface{})
578-
assert.Equal(t, "dog", petMap["type"])
579-
}
485+
params := parsedURL.Query()
486+
assert.Equal(t, "dog", params.Get("type"))
487+
assert.Equal(t, "name,age,breed", params.Get("fields"))
580488
},
581489
},
582490
}
583491

584492
for _, tt := range tests {
585493
t.Run(tt.name, func(t *testing.T) {
494+
var capturedURL string
495+
if tt.setup != nil {
496+
handler := tt.setup(t, ts)
497+
// Wrap the handler to capture the URL
498+
wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
499+
capturedURL = r.URL.String()
500+
handler.ServeHTTP(w, r)
501+
})
502+
ts.Config.Handler = wrappedHandler
503+
}
504+
586505
var response jsonrpc.Response
587506
if tt.server != nil {
588507
response = tt.server.HandleRequest(tt.request)
589508
} else {
590509
response = server.HandleRequest(tt.request)
591510
}
592-
tt.validate(t, response)
511+
512+
tt.validate(t, response, capturedURL)
593513
})
594514
}
595515
}

0 commit comments

Comments
 (0)