From 2f592a7f5eb224952da856e82d53089f5ccf1aca Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Mon, 8 Jan 2024 09:26:01 -0800 Subject: [PATCH 1/3] 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 --- handler/handler.go | 12 ++++++++++-- handler/handler_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/handler/handler.go b/handler/handler.go index 5ab977a..b420029 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 whethe 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. @@ -274,7 +282,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 +373,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/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) { From 32a5d85c508894a1b42f434e2d00ddab02320741 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Mon, 8 Jan 2024 09:33:33 -0800 Subject: [PATCH 2/3] fix typo --- handler/handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handler/handler.go b/handler/handler.go index b420029..c3b31aa 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -119,7 +119,7 @@ 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 whethe the wrapper it +// 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 From 89f56f0b182726d2b7b3b72642c8a04d28c2e0c3 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Sat, 27 Jan 2024 16:40:13 -0800 Subject: [PATCH 3/3] breadcrumb array support in the constructor docs --- handler/handler.go | 9 +++++---- handler/positional.go | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/handler/handler.go b/handler/handler.go index c3b31aa..5658907 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -257,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 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} //