From 7ab9b3877d91cbd3c9205e0d8b9c3915fb5f8051 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 26 Nov 2021 08:57:14 -0800 Subject: [PATCH] Remove the CheckRequest server option. (#62) There is no strong reason to make the server handle this concern separately. Updates #46. --- jrpc2_test.go | 81 --------------------------------------------------- opts.go | 15 ---------- server.go | 8 ----- 3 files changed, 104 deletions(-) diff --git a/jrpc2_test.go b/jrpc2_test.go index f10b920..61fc8e4 100644 --- a/jrpc2_test.go +++ b/jrpc2_test.go @@ -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 }) @@ -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) { diff --git a/opts.go b/opts.go index d9cf062..cdb4109 100644 --- a/opts.go +++ b/opts.go @@ -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. @@ -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() diff --git a/server.go b/server.go index 61a3c9f..f133c7f 100644 --- a/server.go +++ b/server.go @@ -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 @@ -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(), @@ -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