Skip to content

Commit a7de1dc

Browse files
wklkenwklken
committed
fix(mcp-proxy): serialize query arrays as repeated params
Why this change was needed: MCP proxy query array values were being converted through the scalar parameter path, causing values such as related=[fields storages] to be sent upstream as a single encoded slice string. What changed: - Added a query-specific multi-value parameter type with UseNumber-preserving JSON decoding - Passed query value slices through SetQueryParam as repeated values - Added focused tests for query arrays, URL encoding, large numbers, and header/path isolation Problem solved: Array query parameters now proxy as repeated query entries while scalar headers, paths, and numeric string preservation remain unchanged.
1 parent 3316838 commit a7de1dc

4 files changed

Lines changed: 242 additions & 14 deletions

File tree

‎src/mcp-proxy/pkg/infra/proxy/proxy.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,9 @@ func setHandlerRequestParams(
626626
}
627627
}
628628
if handlerRequest.QueryParam != nil {
629-
for k, value := range handlerRequest.QueryParam {
630-
if err := req.SetQueryParam(k, value); err != nil {
631-
auditLog.Error("set query param err", zap.String(k, value), zap.Error(err))
629+
for k, values := range handlerRequest.QueryParam {
630+
if err := req.SetQueryParam(k, values...); err != nil {
631+
auditLog.Error("set query param err", zap.Strings(k, values), zap.Error(err))
632632
return err
633633
}
634634
}
@@ -917,7 +917,7 @@ func genToolHandler(toolApiConfig *ToolConfig, serverName string, rawResponseEna
917917
auditStatus = "success"
918918
auditUpstreamReqID string
919919
auditHeaderInfo map[string]string
920-
auditQueryParam StringParamMap
920+
auditQueryParam QueryParam
921921
auditPathParam StringParamMap
922922
auditBodyParam any
923923
)

‎src/mcp-proxy/pkg/infra/proxy/proxy_test.go‎

Lines changed: 140 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ import (
2424
"encoding/json"
2525
"net/http"
2626
"net/http/httptest"
27+
"net/url"
2728
"strings"
2829
"sync"
2930
"time"
3031

3132
"github.com/gin-gonic/gin"
33+
"github.com/go-openapi/runtime"
3234
"github.com/modelcontextprotocol/go-sdk/mcp"
3335
. "github.com/onsi/ginkgo/v2"
3436
. "github.com/onsi/gomega"
@@ -1046,12 +1048,63 @@ var _ = Describe("MCPProxy", func() {
10461048
})
10471049
})
10481050

1051+
Describe("setHandlerRequestParams", func() {
1052+
It("should set array query values as repeated query params", func() {
1053+
req := newRecordingClientRequest()
1054+
handlerRequest := &HandlerRequest{
1055+
QueryParam: QueryParam{
1056+
"related": {"fields", "storages"},
1057+
},
1058+
}
1059+
1060+
err := setHandlerRequestParams(req, handlerRequest, map[string]string{}, zap.NewNop())
1061+
Expect(err).NotTo(HaveOccurred())
1062+
Expect(req.GetQueryParams()["related"]).To(Equal([]string{"fields", "storages"}))
1063+
Expect(req.GetQueryParams().Encode()).To(Equal("related=fields&related=storages"))
1064+
Expect(req.GetQueryParams().Encode()).NotTo(ContainSubstring("%5Bfields+storages%5D"))
1065+
})
1066+
1067+
It("should keep scalar query values unchanged", func() {
1068+
req := newRecordingClientRequest()
1069+
handlerRequest := &HandlerRequest{
1070+
QueryParam: QueryParam{
1071+
"extra": {"true"},
1072+
"ratio": {"1.25"},
1073+
},
1074+
}
1075+
1076+
err := setHandlerRequestParams(req, handlerRequest, map[string]string{}, zap.NewNop())
1077+
Expect(err).NotTo(HaveOccurred())
1078+
Expect(req.GetQueryParams()["extra"]).To(Equal([]string{"true"}))
1079+
Expect(req.GetQueryParams()["ratio"]).To(Equal([]string{"1.25"}))
1080+
})
1081+
1082+
It("should keep header and path params scalar", func() {
1083+
req := newRecordingClientRequest()
1084+
headerInfo := map[string]string{}
1085+
handlerRequest := &HandlerRequest{
1086+
HeaderParam: StringParamMap{"X-Trace-ID": "2005000002"},
1087+
PathParam: StringParamMap{"bk_biz_id": "2005000002"},
1088+
}
1089+
1090+
err := setHandlerRequestParams(req, handlerRequest, headerInfo, zap.NewNop())
1091+
Expect(err).NotTo(HaveOccurred())
1092+
Expect(req.GetHeaderParams().Values("X-Trace-ID")).To(Equal([]string{"2005000002"}))
1093+
Expect(req.pathParams["bk_biz_id"]).To(Equal("2005000002"))
1094+
Expect(headerInfo["X-Trace-ID"]).To(Equal("2005000002"))
1095+
})
1096+
})
1097+
10491098
Describe("decodeHandlerRequest with UseNumber", func() {
10501099
It("should preserve numbers for all request parameter groups", func() {
10511100
arguments := map[string]any{
10521101
"header_param": map[string]any{"X-Trace-ID": 2005000002},
1053-
"query_param": map[string]any{"bk_biz_id": 2005000002, "ratio": 1.25},
1054-
"path_param": map[string]any{"bk_biz_id": 2005000002},
1102+
"query_param": map[string]any{
1103+
"bk_biz_id": 2005000002,
1104+
"ratio": 1.25,
1105+
"ids": []any{2005000002, json.Number("9007199254740992")},
1106+
},
1107+
"path_param": map[string]any{"bk_biz_id": 2005000002},
10551108
"body_param": map[string]any{
10561109
"bk_biz_id": 2005000002,
10571110
"nested": map[string]any{"id": 2005000003},
@@ -1068,8 +1121,9 @@ var _ = Describe("MCPProxy", func() {
10681121
Expect(err).NotTo(HaveOccurred())
10691122

10701123
Expect(handlerRequest.HeaderParam["X-Trace-ID"]).To(Equal("2005000002"))
1071-
Expect(handlerRequest.QueryParam["bk_biz_id"]).To(Equal("2005000002"))
1072-
Expect(handlerRequest.QueryParam["ratio"]).To(Equal("1.25"))
1124+
Expect(handlerRequest.QueryParam["bk_biz_id"]).To(Equal([]string{"2005000002"}))
1125+
Expect(handlerRequest.QueryParam["ratio"]).To(Equal([]string{"1.25"}))
1126+
Expect(handlerRequest.QueryParam["ids"]).To(Equal([]string{"2005000002", "9007199254740992"}))
10731127
Expect(handlerRequest.PathParam["bk_biz_id"]).To(Equal("2005000002"))
10741128

10751129
body := handlerRequest.BodyParam.(map[string]any)
@@ -1078,3 +1132,85 @@ var _ = Describe("MCPProxy", func() {
10781132
})
10791133
})
10801134
})
1135+
1136+
type recordingClientRequest struct {
1137+
headers http.Header
1138+
query url.Values
1139+
pathParams map[string]string
1140+
bodyParam any
1141+
timeout time.Duration
1142+
}
1143+
1144+
func newRecordingClientRequest() *recordingClientRequest {
1145+
return &recordingClientRequest{
1146+
headers: http.Header{},
1147+
query: url.Values{},
1148+
pathParams: map[string]string{},
1149+
}
1150+
}
1151+
1152+
var _ runtime.ClientRequest = (*recordingClientRequest)(nil)
1153+
1154+
func (r *recordingClientRequest) SetHeaderParam(name string, values ...string) error {
1155+
r.headers.Del(name)
1156+
for _, value := range values {
1157+
r.headers.Add(name, value)
1158+
}
1159+
return nil
1160+
}
1161+
1162+
func (r *recordingClientRequest) GetHeaderParams() http.Header {
1163+
return r.headers.Clone()
1164+
}
1165+
1166+
func (r *recordingClientRequest) SetQueryParam(name string, values ...string) error {
1167+
r.query[name] = values
1168+
return nil
1169+
}
1170+
1171+
func (r *recordingClientRequest) SetFormParam(string, ...string) error {
1172+
return nil
1173+
}
1174+
1175+
func (r *recordingClientRequest) SetPathParam(name, value string) error {
1176+
r.pathParams[name] = value
1177+
return nil
1178+
}
1179+
1180+
func (r *recordingClientRequest) GetQueryParams() url.Values {
1181+
return r.query
1182+
}
1183+
1184+
func (r *recordingClientRequest) SetFileParam(string, ...runtime.NamedReadCloser) error {
1185+
return nil
1186+
}
1187+
1188+
func (r *recordingClientRequest) SetBodyParam(body any) error {
1189+
r.bodyParam = body
1190+
return nil
1191+
}
1192+
1193+
func (r *recordingClientRequest) SetTimeout(timeout time.Duration) error {
1194+
r.timeout = timeout
1195+
return nil
1196+
}
1197+
1198+
func (r *recordingClientRequest) GetMethod() string {
1199+
return ""
1200+
}
1201+
1202+
func (r *recordingClientRequest) GetPath() string {
1203+
return ""
1204+
}
1205+
1206+
func (r *recordingClientRequest) GetBody() []byte {
1207+
return nil
1208+
}
1209+
1210+
func (r *recordingClientRequest) GetBodyParam() any {
1211+
return r.bodyParam
1212+
}
1213+
1214+
func (r *recordingClientRequest) GetFileParam() map[string][]runtime.NamedReadCloser {
1215+
return nil
1216+
}

‎src/mcp-proxy/pkg/infra/proxy/types.go‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,48 @@ func (m *StringParamMap) UnmarshalJSON(data []byte) error {
7171
return nil
7272
}
7373

74+
// QueryParam stores HTTP query parameters as multiple string values per key.
75+
type QueryParam map[string][]string
76+
77+
// UnmarshalJSON decodes JSON query values into strings, preserving numeric precision and arrays.
78+
func (m *QueryParam) UnmarshalJSON(data []byte) error {
79+
if string(data) == "null" {
80+
*m = nil
81+
return nil
82+
}
83+
84+
var values map[string]any
85+
decoder := json.NewDecoder(bytes.NewReader(data))
86+
decoder.UseNumber()
87+
if err := decoder.Decode(&values); err != nil {
88+
return err
89+
}
90+
91+
result := make(QueryParam, len(values))
92+
for key, value := range values {
93+
if items, ok := value.([]any); ok {
94+
if len(items) == 0 {
95+
continue
96+
}
97+
98+
queryValues := make([]string, 0, len(items))
99+
for _, item := range items {
100+
queryValues = append(queryValues, stringifyRequestParamValue(item))
101+
}
102+
result[key] = queryValues
103+
continue
104+
}
105+
106+
result[key] = []string{stringifyRequestParamValue(value)}
107+
}
108+
*m = result
109+
return nil
110+
}
111+
74112
// HandlerRequest ...
75113
type HandlerRequest struct {
76114
HeaderParam StringParamMap `json:"header_param,omitempty"`
77-
QueryParam StringParamMap `json:"query_param,omitempty"`
115+
QueryParam QueryParam `json:"query_param,omitempty"`
78116
PathParam StringParamMap `json:"path_param,omitempty"`
79117
BodyParam any `json:"body_param,omitempty"`
80118
}

‎src/mcp-proxy/pkg/infra/proxy/types_test.go‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ var _ = Describe("Types", func() {
3333
It("should marshal and unmarshal correctly", func() {
3434
request := proxy.HandlerRequest{
3535
HeaderParam: proxy.StringParamMap{"Content-Type": "application/json"},
36-
QueryParam: proxy.StringParamMap{"limit": "10", "offset": "0"},
36+
QueryParam: proxy.QueryParam{"limit": {"10"}, "offset": {"0"}},
3737
PathParam: proxy.StringParamMap{"id": "123"},
3838
BodyParam: map[string]any{"name": "test"},
3939
}
@@ -46,7 +46,7 @@ var _ = Describe("Types", func() {
4646
err = json.Unmarshal(data, &result)
4747
Expect(err).NotTo(HaveOccurred())
4848
Expect(result.HeaderParam["Content-Type"]).To(Equal("application/json"))
49-
Expect(result.QueryParam["limit"]).To(Equal("10"))
49+
Expect(result.QueryParam["limit"]).To(Equal([]string{"10"}))
5050
Expect(result.PathParam["id"]).To(Equal("123"))
5151
Expect(result.BodyParam.(map[string]any)["name"]).To(Equal("test"))
5252
})
@@ -60,7 +60,7 @@ var _ = Describe("Types", func() {
6060

6161
It("should handle partial fields", func() {
6262
request := proxy.HandlerRequest{
63-
QueryParam: proxy.StringParamMap{"search": "test"},
63+
QueryParam: proxy.QueryParam{"search": {"test"}},
6464
}
6565

6666
data, err := json.Marshal(request)
@@ -102,7 +102,7 @@ var _ = Describe("Types", func() {
102102
It("should unmarshal from JSON string", func() {
103103
jsonStr := `{
104104
"header_param": {"Authorization": "Bearer token"},
105-
"query_param": {"page": 1},
105+
"query_param": {"page": ["1"]},
106106
"path_param": {"userId": "abc123"},
107107
"body_param": {"data": "test"}
108108
}`
@@ -112,13 +112,67 @@ var _ = Describe("Types", func() {
112112
Expect(err).NotTo(HaveOccurred())
113113

114114
Expect(request.HeaderParam["Authorization"]).To(Equal("Bearer token"))
115-
Expect(request.QueryParam["page"]).To(Equal("1"))
115+
Expect(request.QueryParam["page"]).To(Equal([]string{"1"}))
116116
Expect(request.PathParam["userId"]).To(Equal("abc123"))
117117
Expect(request.BodyParam.(map[string]any)["data"]).To(Equal("test"))
118118
})
119119
})
120120
})
121121

122+
Describe("QueryParam", func() {
123+
Describe("UnmarshalJSON", func() {
124+
It("should decode array values as multiple query values", func() {
125+
jsonStr := `{"related": ["fields", "storages"]}`
126+
var m proxy.QueryParam
127+
err := json.Unmarshal([]byte(jsonStr), &m)
128+
Expect(err).NotTo(HaveOccurred())
129+
Expect(m["related"]).To(Equal([]string{"fields", "storages"}))
130+
})
131+
132+
It("should decode single-item arrays as one query value", func() {
133+
jsonStr := `{"related": ["fields"]}`
134+
var m proxy.QueryParam
135+
err := json.Unmarshal([]byte(jsonStr), &m)
136+
Expect(err).NotTo(HaveOccurred())
137+
Expect(m["related"]).To(Equal([]string{"fields"}))
138+
})
139+
140+
It("should skip empty arrays instead of stringifying them", func() {
141+
jsonStr := `{"related": []}`
142+
var m proxy.QueryParam
143+
err := json.Unmarshal([]byte(jsonStr), &m)
144+
Expect(err).NotTo(HaveOccurred())
145+
Expect(m).NotTo(HaveKey("related"))
146+
})
147+
148+
It("should decode scalar values using existing string conversion semantics", func() {
149+
jsonStr := `{"extra": true, "ratio": 1.25, "bk_biz_id": 2005000002}`
150+
var m proxy.QueryParam
151+
err := json.Unmarshal([]byte(jsonStr), &m)
152+
Expect(err).NotTo(HaveOccurred())
153+
Expect(m["extra"]).To(Equal([]string{"true"}))
154+
Expect(m["ratio"]).To(Equal([]string{"1.25"}))
155+
Expect(m["bk_biz_id"]).To(Equal([]string{"2005000002"}))
156+
})
157+
158+
It("should preserve large integers in arrays without scientific notation", func() {
159+
jsonStr := `{"ids": [2005000002, 9007199254740992]}`
160+
var m proxy.QueryParam
161+
err := json.Unmarshal([]byte(jsonStr), &m)
162+
Expect(err).NotTo(HaveOccurred())
163+
Expect(m["ids"]).To(Equal([]string{"2005000002", "9007199254740992"}))
164+
})
165+
166+
It("should handle null input", func() {
167+
jsonStr := `null`
168+
var m proxy.QueryParam
169+
err := json.Unmarshal([]byte(jsonStr), &m)
170+
Expect(err).NotTo(HaveOccurred())
171+
Expect(m).To(BeNil())
172+
})
173+
})
174+
})
175+
122176
Describe("StringParamMap", func() {
123177
Describe("UnmarshalJSON", func() {
124178
It("should convert large integers to decimal strings without scientific notation", func() {

0 commit comments

Comments
 (0)