Skip to content

Commit daeb674

Browse files
authored
Fix positional optional params (#2421)
* Fix positional optional params * use required count instead of optional
1 parent c646d21 commit daeb674

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

jsonrpc/server.go

+18-1
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ type Method struct {
115115
// The method takes a context as its first parameter.
116116
// Set upon successful registration.
117117
needsContext bool
118+
119+
// The number of required parameters in the method.
120+
// Set upon successful registration.
121+
requiredParamCount int
118122
}
119123

120124
type Server struct {
@@ -198,6 +202,14 @@ func (s *Server) registerMethod(method Method) error {
198202
return errors.New("second return value must be a http.Header for 3 tuple handler")
199203
}
200204

205+
requiredParamCount := 0
206+
for _, param := range method.Params {
207+
if !param.Optional {
208+
requiredParamCount++
209+
}
210+
}
211+
method.requiredParamCount = requiredParamCount
212+
201213
// The method is valid. Mutate the appropriate fields and register on the server.
202214
s.methods[method.Name] = method
203215

@@ -534,7 +546,8 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
534546
case reflect.Slice:
535547
paramsList := params.([]any)
536548

537-
if len(paramsList) != numArgs-addContext {
549+
// Ensure that the number of provided parameters is between required and total parameters
550+
if len(paramsList) < method.requiredParamCount || len(paramsList) > len(method.Params) {
538551
return nil, errors.New("missing/unexpected params in list")
539552
}
540553

@@ -545,6 +558,10 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
545558
}
546559
args = append(args, v)
547560
}
561+
// Add remaining optional parameters if available
562+
for i := addContext + len(paramsList); i < numArgs; i++ {
563+
args = append(args, reflect.New(handlerType.In(i)).Elem())
564+
}
548565
case reflect.Map:
549566
paramsMap := params.(map[string]any)
550567

jsonrpc/server_test.go

+20-4
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,18 @@ func TestHandle(t *testing.T) {
181181
return 0, nil
182182
},
183183
},
184+
{
185+
Name: "multipleOptionalParams",
186+
Params: []jsonrpc.Parameter{
187+
{Name: "param1"},
188+
{Name: "param2"},
189+
{Name: "param3", Optional: true},
190+
{Name: "param4", Optional: true},
191+
},
192+
Handler: func(param1 *int, param2 []int, param3 *int, param4 []int) (int, *jsonrpc.Error) {
193+
return 0, nil
194+
},
195+
},
184196
}
185197

186198
listener := CountingEventListener{}
@@ -218,10 +230,6 @@ func TestHandle(t *testing.T) {
218230
req: `{"jsonrpc" : "2.0", "method" : "method", "id" : 5}`,
219231
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing non-optional param field"},"id":5}`,
220232
},
221-
"missing param(s)": {
222-
req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false] , "id" : 3}`,
223-
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
224-
},
225233
"too many params": {
226234
req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false, "error message", "too many"] , "id" : 3}`,
227235
res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
@@ -490,6 +498,14 @@ func TestHandle(t *testing.T) {
490498
req: `{"jsonrpc": "2.0", "method": "singleOptionalParam", "id": 1}`,
491499
res: `{"jsonrpc":"2.0","result":0,"id":1}`,
492500
},
501+
"empty multiple optional params": {
502+
req: `{"jsonrpc": "2.0", "method": "multipleOptionalParams", "params": {"param1": 1, "param2": [2, 3]}, "id": 1}`,
503+
res: `{"jsonrpc":"2.0","result":0,"id":1}`,
504+
},
505+
"empty multiple optional positional params": {
506+
req: `{"jsonrpc": "2.0", "method": "multipleOptionalParams", "params": [1, [2, 3]], "id": 1}`,
507+
res: `{"jsonrpc":"2.0","result":0,"id":1}`,
508+
},
493509
}
494510

495511
for desc, test := range tests {

0 commit comments

Comments
 (0)