Skip to content

Commit

Permalink
handler: add an option to disallow array-based parameters (#114)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
creachadair authored Apr 1, 2024
1 parent 241de22 commit a3d02fa
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 10 deletions.
21 changes: 15 additions & 6 deletions handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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")
}
Expand Down Expand Up @@ -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 {
Expand Down
36 changes: 36 additions & 0 deletions handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 6 additions & 4 deletions handler/positional.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
//
Expand Down

0 comments on commit a3d02fa

Please sign in to comment.