Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the CheckRequest server option. #62

Merged
merged 1 commit into from
Nov 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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