Skip to content

Commit

Permalink
Remove the CheckRequest server option. (#62)
Browse files Browse the repository at this point in the history
There is no strong reason to make the server handle this concern separately.

Updates #46.
  • Loading branch information
creachadair authored Nov 26, 2021
1 parent 63a5fca commit 7ab9b38
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 104 deletions.
81 changes: 0 additions & 81 deletions jrpc2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ var (
_ code.ErrCoder = (*jrpc2.Error)(nil)
)

var notAuthorized = code.Register(-32095, "request not authorized")

var testOK = handler.New(func(ctx context.Context) (string, error) {
return "OK", nil
})
Expand Down Expand Up @@ -1038,85 +1036,6 @@ func TestContextPlumbing(t *testing.T) {
}
}

// Verify that the request-checking hook works.
func TestServer_checkRequestHook(t *testing.T) {
const wantResponse = "Hey girl"
const wantToken = "OK"

loc := server.NewLocal(handler.Map{
"Test": handler.New(func(ctx context.Context) (string, error) {
return wantResponse, nil
}),
}, &server.LocalOptions{
// Enable auth checking and context decoding for the server.
Server: &jrpc2.ServerOptions{
DecodeContext: jctx.Decode,
CheckRequest: func(ctx context.Context, req *jrpc2.Request) error {
var token []byte
switch err := jctx.UnmarshalMetadata(ctx, &token); err {
case nil:
t.Logf("Metadata present: value=%q", string(token))
case jctx.ErrNoMetadata:
t.Log("Metadata not set")
default:
return err
}
if s := string(token); s != wantToken {
return jrpc2.Errorf(notAuthorized, "not authorized")
}
return nil
},
},

// Enable context encoding for the client.
Client: &jrpc2.ClientOptions{
EncodeContext: jctx.Encode,
},
})
defer loc.Close()
c := loc.Client

// Call without a token and verify that we get an error.
t.Run("NoToken", func(t *testing.T) {
var rsp string
err := c.CallResult(context.Background(), "Test", nil, &rsp)
if err == nil {
t.Errorf("Call(Test): got %q, wanted error", rsp)
} else if ec := code.FromError(err); ec != notAuthorized {
t.Errorf("Call(Test): got code %v, want %v", ec, notAuthorized)
}
})

// Call with a valid token and verify that we get a response.
t.Run("GoodToken", func(t *testing.T) {
ctx, err := jctx.WithMetadata(context.Background(), []byte(wantToken))
if err != nil {
t.Fatalf("Call(Test): attaching metadata: %v", err)
}
var rsp string
if err := c.CallResult(ctx, "Test", nil, &rsp); err != nil {
t.Errorf("Call(Test): unexpected error: %v", err)
}
if rsp != wantResponse {
t.Errorf("Call(Test): got %q, want %q", rsp, wantResponse)
}
})

// Call with an invalid token and verify that we get an error.
t.Run("BadToken", func(t *testing.T) {
ctx, err := jctx.WithMetadata(context.Background(), []byte("BAD"))
if err != nil {
t.Fatalf("Call(Test): attaching metadata: %v", err)
}
var rsp string
if err := c.CallResult(ctx, "Test", nil, &rsp); err == nil {
t.Errorf("Call(Test): got %q, wanted error", rsp)
} else if ec := code.FromError(err); ec != notAuthorized {
t.Errorf("Call(Test): got code %v, want %v", ec, notAuthorized)
}
})
}

// Verify that calling a wrapped method which takes no parameters, but in which
// the caller provided parameters, will correctly report an error.
func TestHandler_noParams(t *testing.T) {
Expand Down
15 changes: 0 additions & 15 deletions opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ type ServerOptions struct {
// If unset, context and parameters are used as given.
DecodeContext func(context.Context, string, json.RawMessage) (context.Context, json.RawMessage, error)

// If set, this function is called with the context and client request
// (after decoding, if DecodeContext is set) that are to be delivered to the
// handler. If CheckRequest reports a non-nil error, the request fails with
// that error without invoking the handler.
CheckRequest func(ctx context.Context, req *Request) error

// If set, use this value to record server metrics. All servers created
// from the same options will share the same metrics collector. If none is
// set, an empty collector will be created for each new server.
Expand Down Expand Up @@ -110,15 +104,6 @@ func (s *ServerOptions) decodeContext() (decoder, bool) {
return s.DecodeContext, true
}

type verifier = func(context.Context, *Request) error

func (s *ServerOptions) checkRequest() verifier {
if s == nil || s.CheckRequest == nil {
return func(context.Context, *Request) error { return nil }
}
return s.CheckRequest
}

func (s *ServerOptions) metrics() *metrics.M {
if s == nil || s.Metrics == nil {
return metrics.New()
Expand Down
8 changes: 0 additions & 8 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type Server struct {
rpcLog RPCLogger // log RPC requests and responses here
newctx func() context.Context // create a new base request context
dectx decoder // decode context from request
ckreq verifier // request checking hook
expctx bool // whether to expect request context
metrics *metrics.M // metrics collected during execution
start time.Time // when Start was called
Expand Down Expand Up @@ -75,7 +74,6 @@ func NewServer(mux Assigner, opts *ServerOptions) *Server {
rpcLog: opts.rpcLog(),
newctx: opts.newContext(),
dectx: dc,
ckreq: opts.checkRequest(),
expctx: exp,
mu: new(sync.Mutex),
metrics: opts.metrics(),
Expand Down Expand Up @@ -317,12 +315,6 @@ func (s *Server) setContext(t *task, id string) bool {
return false
}

// Check request.
if err := s.ckreq(base, t.hreq); err != nil {
t.err = err
return false
}

t.ctx = context.WithValue(base, inboundRequestKey{}, t.hreq)

// Store the cancellation for a request that needs a reply, so that we can
Expand Down

0 comments on commit 7ab9b38

Please sign in to comment.