Repository: creachadair/jrpc2
Pull request #114 (2024-01-08 17:30:47)

handler: add an option to disallow array-based parameters
Add a new AllowArray option to handler.FuncInfo controlling whether the decoder
will allow array notation for struct arguments. The default is true, so that
existing use is not affected.

When true (the default), the decoder allows the arguments for a struct to be
passed as a JSON array and maps the corresponding values to the struct fields
in order of declaration. This is the existing behaviour.

When false, the decoder reports an error for a struct-valued argument when the
parameter value is an array.

Updates #113


2opremio: Any change of getting this merged soonish? I would like to be able to close https://github.com/stellar/soroban-rpc/issues/13
creachadair: > Any change of getting this merged soonish? I would like to be able to close [stellar/soroban-rpc#13](https://github.com/stellar/soroban-rpc/issues/13)

Sure, I'm glad to merge it anytime. I was leaving it open to make sure you had a chance to test it before I put a release number on it. If you're content with the current status, I'll merge and tag it.

Patch:
diff --git a/handler/handler.go b/handler/handler.go
index 5ab977a..5658907 100644
--- a/handler/handler.go
+++ b/handler/handler.go
@@ -106,6 +106,7 @@ type FuncInfo struct {
 	ReportsError bool         // true if the function reports an error
 
 	strictFields bool     // enforce strict field checking
+	allowArray   bool     // allow decoding from array format
 	posNames     []string // positional field names
 
 	fn any // the original function value
@@ -118,6 +119,13 @@ type FuncInfo struct {
 // for non-struct arguments.
 func (fi *FuncInfo) SetStrict(strict bool) *FuncInfo { fi.strictFields = strict; return fi }
 
+// AllowArray sets the flag on fi that determines whether the wrapper it
+// generates allows struct arguments to be sent in array notation.  If true, a
+// parameter array is decoded into corresponding fields of the struct argument
+// in declaration order; if false, array arguments report an error. The default
+// value is currently true. This option has no effect for non-struct arguments.
+func (fi *FuncInfo) AllowArray(ok bool) *FuncInfo { fi.allowArray = ok; return fi }
+
 // Wrap adapts the function represented by fi to a jrpc2.Handler.  The wrapped
 // function can obtain the *jrpc2.Request value from its context argument using
 // the jrpc2.InboundRequest helper.
@@ -249,10 +257,11 @@ func (fi *FuncInfo) Wrap() jrpc2.Handler {
 // If fn does not have one of these forms, Check reports an error.
 //
 // If the type of X is a struct or a pointer to a struct, the generated wrapper
-// accepts JSON parameters as either an object or an array.  Array parameters
-// are mapped to the fields of X in the order of field declaration, save that
-// unexported fields are skipped. If a field has a `json:"-"` tag, it is also
-// skipped. Anonymous fields are skipped unless they are tagged.
+// accepts JSON parameters as either an object or an array.  The caller may
+// disable array support by calling AllowArray(false).  When enabled, array
+// parameters are mapped to the fields of X in the order of field declaration,
+// save that unexported fields are skipped. If a field has a `json:"-"` tag, it
+// is also skipped. Anonymous fields are skipped unless they are tagged.
 //
 // For other (non-struct) argument types, the accepted format is whatever the
 // json.Unmarshal function can decode into the value.  Note, however, that the
@@ -274,7 +283,7 @@ func Check(fn any) (*FuncInfo, error) {
 		return nil, errors.New("nil function")
 	}
 
-	info := &FuncInfo{Type: reflect.TypeOf(fn), fn: fn}
+	info := &FuncInfo{Type: reflect.TypeOf(fn), fn: fn, allowArray: true}
 	if info.Type.Kind() != reflect.Func {
 		return nil, errors.New("not a function")
 	}
@@ -365,7 +374,7 @@ func (s *strictStub) UnmarshalJSON(data []byte) error {
 func (fi *FuncInfo) argWrapper() func(reflect.Value) any {
 	strict := fi.strictFields && fi.Argument != nil && !fi.Argument.Implements(strictType)
 	names := fi.posNames // capture so the wrapper does not pin fi
-	array := len(names) != 0
+	array := len(names) != 0 && fi.allowArray
 	switch {
 	case strict && array:
 		return func(v reflect.Value) any {
diff --git a/handler/positional.go b/handler/positional.go
index f04fb68..de27009 100644
--- a/handler/positional.go
+++ b/handler/positional.go
@@ -95,14 +95,16 @@ func structFieldNames(atype reflect.Type) (bool, []string) {
 //	// ...
 //	call := fi.Wrap()
 //
-// the resulting handler accepts a JSON array with with (exactly) the same
-// number of elements as the positional parameters:
+// the resulting handler by default accepts a JSON array with with (exactly)
+// the same number of elements as the positional parameters:
 //
 //	[17, 23]
 //
 // No arguments can be omitted in this format, but the caller can use a JSON
-// "null" in place of any argument.  The handler will also accept a parameter
-// object like:
+// "null" in place of any argument. The caller may also disable array support
+// by setting AllowArray(false) on the resulting FuncInfo.
+//
+// The handler will also accept a parameter object like:
 //
 //	{"first": 17, "second": 23}
 //


Test patch:
diff --git a/handler/handler_test.go b/handler/handler_test.go
index f1fd5e6..ee06de8 100644
--- a/handler/handler_test.go
+++ b/handler/handler_test.go
@@ -258,6 +258,42 @@ func TestFuncInfo_SetStrict(t *testing.T) {
 	}
 }
 
+func TestFuncInfo_AllowArray(t *testing.T) {
+	type arg struct {
+		A, B string
+	}
+	fi, err := handler.Check(func(ctx context.Context, arg *arg) error {
+		if arg.A != "x" || arg.B != "y" {
+			return fmt.Errorf("a=%q, b=%q", arg.A, arg.B)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("Check failed: %v", err)
+	}
+
+	req := testutil.MustParseRequest(t, `{
+   "jsonrpc": "2.0",
+   "id":      101,
+   "method":  "f",
+   "params":  ["x", "y"]
+   }`)
+
+	t.Run("true", func(t *testing.T) {
+		fn := fi.AllowArray(true).Wrap()
+		if _, err := fn(context.Background(), req); err != nil {
+			t.Fatalf("Handler unexpectedly failed: %v", err)
+		}
+	})
+	t.Run("false", func(t *testing.T) {
+		fn := fi.AllowArray(false).Wrap()
+		rsp, err := fn(context.Background(), req)
+		if got := jrpc2.ErrorCode(err); got != jrpc2.InvalidParams {
+			t.Errorf("Handler returned (%+v, %v), want InvalidParams", rsp, err)
+		}
+	})
+}
+
 // Verify that the handling of pointer-typed arguments does not incorrectly
 // introduce another pointer indirection.
 func TestNew_pointerRegression(t *testing.T) {