From daeb67487768678266c2c2ffb6449642a847005a Mon Sep 17 00:00:00 2001
From: Ng Wei Han <47109095+weiihann@users.noreply.github.com>
Date: Wed, 5 Feb 2025 20:38:18 +0800
Subject: [PATCH] Fix positional optional params (#2421)

* Fix positional optional params

* use required count instead of optional
---
 jsonrpc/server.go      | 19 ++++++++++++++++++-
 jsonrpc/server_test.go | 24 ++++++++++++++++++++----
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/jsonrpc/server.go b/jsonrpc/server.go
index 863b1594f5..06302ac2f2 100644
--- a/jsonrpc/server.go
+++ b/jsonrpc/server.go
@@ -115,6 +115,10 @@ type Method struct {
 	// The method takes a context as its first parameter.
 	// Set upon successful registration.
 	needsContext bool
+
+	// The number of required parameters in the method.
+	// Set upon successful registration.
+	requiredParamCount int
 }
 
 type Server struct {
@@ -198,6 +202,14 @@ func (s *Server) registerMethod(method Method) error {
 		return errors.New("second return value must be a http.Header for 3 tuple handler")
 	}
 
+	requiredParamCount := 0
+	for _, param := range method.Params {
+		if !param.Optional {
+			requiredParamCount++
+		}
+	}
+	method.requiredParamCount = requiredParamCount
+
 	// The method is valid. Mutate the appropriate fields and register on the server.
 	s.methods[method.Name] = method
 
@@ -534,7 +546,8 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
 	case reflect.Slice:
 		paramsList := params.([]any)
 
-		if len(paramsList) != numArgs-addContext {
+		// Ensure that the number of provided parameters is between required and total parameters
+		if len(paramsList) < method.requiredParamCount || len(paramsList) > len(method.Params) {
 			return nil, errors.New("missing/unexpected params in list")
 		}
 
@@ -545,6 +558,10 @@ func (s *Server) buildArguments(ctx context.Context, params any, method Method)
 			}
 			args = append(args, v)
 		}
+		// Add remaining optional parameters if available
+		for i := addContext + len(paramsList); i < numArgs; i++ {
+			args = append(args, reflect.New(handlerType.In(i)).Elem())
+		}
 	case reflect.Map:
 		paramsMap := params.(map[string]any)
 
diff --git a/jsonrpc/server_test.go b/jsonrpc/server_test.go
index 71218a429d..862ba7e29a 100644
--- a/jsonrpc/server_test.go
+++ b/jsonrpc/server_test.go
@@ -181,6 +181,18 @@ func TestHandle(t *testing.T) {
 				return 0, nil
 			},
 		},
+		{
+			Name: "multipleOptionalParams",
+			Params: []jsonrpc.Parameter{
+				{Name: "param1"},
+				{Name: "param2"},
+				{Name: "param3", Optional: true},
+				{Name: "param4", Optional: true},
+			},
+			Handler: func(param1 *int, param2 []int, param3 *int, param4 []int) (int, *jsonrpc.Error) {
+				return 0, nil
+			},
+		},
 	}
 
 	listener := CountingEventListener{}
@@ -218,10 +230,6 @@ func TestHandle(t *testing.T) {
 			req: `{"jsonrpc" : "2.0", "method" : "method", "id" : 5}`,
 			res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing non-optional param field"},"id":5}`,
 		},
-		"missing param(s)": {
-			req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false] , "id" : 3}`,
-			res: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"missing/unexpected params in list"},"id":3}`,
-		},
 		"too many params": {
 			req: `{"jsonrpc" : "2.0", "method" : "method", "params" : [3, false, "error message", "too many"] , "id" : 3}`,
 			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) {
 			req: `{"jsonrpc": "2.0", "method": "singleOptionalParam", "id": 1}`,
 			res: `{"jsonrpc":"2.0","result":0,"id":1}`,
 		},
+		"empty multiple optional params": {
+			req: `{"jsonrpc": "2.0", "method": "multipleOptionalParams", "params": {"param1": 1, "param2": [2, 3]}, "id": 1}`,
+			res: `{"jsonrpc":"2.0","result":0,"id":1}`,
+		},
+		"empty multiple optional positional params": {
+			req: `{"jsonrpc": "2.0", "method": "multipleOptionalParams", "params": [1, [2, 3]], "id": 1}`,
+			res: `{"jsonrpc":"2.0","result":0,"id":1}`,
+		},
 	}
 
 	for desc, test := range tests {