From 5cdc7d6ccd61a0241a49639cc4747b405886e4c2 Mon Sep 17 00:00:00 2001 From: amyxia Date: Wed, 26 May 2021 20:52:51 +0800 Subject: [PATCH 01/29] fix typo in jsonrpc2/stream.go (#47) Co-authored-by: xiarui.xr --- stream.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stream.go b/stream.go index e7a9025..7023c65 100644 --- a/stream.go +++ b/stream.go @@ -68,7 +68,7 @@ func (t *bufferedObjectStream) Close() error { return t.conn.Close() } -// An ObjectCodec specifies how to encoed and decode a JSON-RPC 2.0 +// An ObjectCodec specifies how to encode and decode a JSON-RPC 2.0 // object in a stream. type ObjectCodec interface { // WriteObject writes a JSON-RPC 2.0 object to the stream. From d6ac66e24f6a88557db47fd1f04fec6a63cd444c Mon Sep 17 00:00:00 2001 From: lhchavez Date: Wed, 4 Aug 2021 05:45:59 -0700 Subject: [PATCH 02/29] Add a way to specify more non-standard-compliant fields to Request (#50) This change introduces `ExtraField`, a `CallOption` that can add arbitrary fields to the top-level JSON-RPC Request message. --- call_opt.go | 9 ++++ call_opt_test.go | 50 ++++++++++++++++++++ jsonrpc2.go | 120 ++++++++++++++++++++++++++++++++++++----------- jsonrpc2_test.go | 2 +- object_test.go | 10 ++-- 5 files changed, 159 insertions(+), 32 deletions(-) diff --git a/call_opt.go b/call_opt.go index 73fe9c2..851baa5 100644 --- a/call_opt.go +++ b/call_opt.go @@ -19,6 +19,15 @@ func Meta(meta interface{}) CallOption { }) } +// ExtraField returns a call option which attaches the given name/value pair to +// the JSON-RPC 2.0 request. This can be used to add arbitrary extensions to +// JSON RPC 2.0. +func ExtraField(name string, value interface{}) CallOption { + return callOptionFunc(func(r *Request) error { + return r.SetExtraField(name, value) + }) +} + // PickID returns a call option which sets the ID on a request. Care must be // taken to ensure there are no conflicts with any previously picked ID, nor // with the default sequence ID. diff --git a/call_opt_test.go b/call_opt_test.go index 82b05ca..b64f661 100644 --- a/call_opt_test.go +++ b/call_opt_test.go @@ -90,3 +90,53 @@ func TestStringID(t *testing.T) { t.Fatal(err) } } + +func TestExtraField(t *testing.T) { + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + a, b := inMemoryPeerConns() + defer a.Close() + defer b.Close() + + handler := handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) { + replyWithError := func(msg string) { + respErr := &jsonrpc2.Error{Code: jsonrpc2.CodeInvalidRequest, Message: msg} + if err := conn.ReplyWithError(ctx, req.ID, respErr); err != nil { + t.Error(err) + } + } + var sessionID string + for _, field := range req.ExtraFields { + if field.Name != "sessionId" { + continue + } + var ok bool + sessionID, ok = field.Value.(string) + if !ok { + t.Errorf("\"sessionId\" is not a string: %v", field.Value) + } + } + if sessionID == "" { + replyWithError("sessionId must be set") + return + } + if sessionID != "session" { + replyWithError("sessionId has the wrong value") + return + } + if err := conn.Reply(ctx, req.ID, "ok"); err != nil { + t.Error(err) + } + }) + connA := jsonrpc2.NewConn(ctx, jsonrpc2.NewBufferedStream(a, jsonrpc2.VSCodeObjectCodec{}), handler) + connB := jsonrpc2.NewConn(ctx, jsonrpc2.NewBufferedStream(b, jsonrpc2.VSCodeObjectCodec{}), noopHandler{}) + defer connA.Close() + defer connB.Close() + + var res string + if err := connB.Call(ctx, "f", nil, &res, jsonrpc2.ExtraField("sessionId", "session")); err != nil { + t.Fatal(err) + } +} diff --git a/jsonrpc2.go b/jsonrpc2.go index 005b65c..e6e2251 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -30,6 +30,12 @@ type JSONRPC2 interface { Close() error } +// RequestField is a top-level field that can be added to the JSON-RPC request. +type RequestField struct { + Name string + Value interface{} +} + // Request represents a JSON-RPC request or // notification. See // http://www.jsonrpc.org/specification#request_object and @@ -45,61 +51,104 @@ type Request struct { // NOTE: It is not part of spec. However, it is useful for propogating // tracing context, etc. Meta *json.RawMessage `json:"meta,omitempty"` + + // ExtraFields optionally adds fields to the root of the JSON-RPC request. + // + // NOTE: It is not part of the spec, but there are other protocols based on + // JSON-RPC 2 that require it. + ExtraFields []RequestField `json:"-"` } // MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" // property. func (r Request) MarshalJSON() ([]byte, error) { - r2 := struct { - Method string `json:"method"` - Params *json.RawMessage `json:"params,omitempty"` - ID *ID `json:"id,omitempty"` - Meta *json.RawMessage `json:"meta,omitempty"` - JSONRPC string `json:"jsonrpc"` - }{ - Method: r.Method, - Params: r.Params, - Meta: r.Meta, - JSONRPC: "2.0", + r2 := map[string]interface{}{ + "jsonrpc": "2.0", + "method": r.Method, + } + for _, field := range r.ExtraFields { + r2[field.Name] = field.Value } if !r.Notif { - r2.ID = &r.ID + r2["id"] = &r.ID + } + if r.Params != nil { + r2["params"] = r.Params + } + if r.Meta != nil { + r2["meta"] = r.Meta } return json.Marshal(r2) } // UnmarshalJSON implements json.Unmarshaler. func (r *Request) UnmarshalJSON(data []byte) error { - var r2 struct { - Method string `json:"method"` - Params *json.RawMessage `json:"params,omitempty"` - Meta *json.RawMessage `json:"meta,omitempty"` - ID *ID `json:"id"` - } + r2 := make(map[string]interface{}) // Detect if the "params" field is JSON "null" or just not present // by seeing if the field gets overwritten to nil. - r2.Params = &json.RawMessage{} + emptyParams := &json.RawMessage{} + r2["params"] = emptyParams - if err := json.Unmarshal(data, &r2); err != nil { + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.UseNumber() + if err := decoder.Decode(&r2); err != nil { return err } - r.Method = r2.Method + var ok bool + r.Method, ok = r2["method"].(string) + if !ok { + return errors.New("missing method field") + } switch { - case r2.Params == nil: + case r2["params"] == nil: r.Params = &jsonNull - case len(*r2.Params) == 0: + case r2["params"] == emptyParams: r.Params = nil default: - r.Params = r2.Params + b, err := json.Marshal(r2["params"]) + if err != nil { + return fmt.Errorf("failed to marshal params: %w", err) + } + r.Params = (*json.RawMessage)(&b) } - r.Meta = r2.Meta - if r2.ID == nil { + meta, ok := r2["meta"] + if ok { + b, err := json.Marshal(meta) + if err != nil { + return fmt.Errorf("failed to marshal Meta: %w", err) + } + r.Meta = (*json.RawMessage)(&b) + } + switch rawID := r2["id"].(type) { + case nil: r.ID = ID{} r.Notif = true - } else { - r.ID = *r2.ID + case string: + r.ID = ID{Str: rawID, IsString: true} r.Notif = false + case json.Number: + id, err := rawID.Int64() + if err != nil { + return fmt.Errorf("failed to unmarshal ID: %w", err) + } + r.ID = ID{Num: uint64(id)} + r.Notif = false + default: + return fmt.Errorf("unexpected ID type: %T", rawID) + } + + // Clear the extra fields before populating them again. + r.ExtraFields = nil + for name, value := range r2 { + switch name { + case "id", "jsonrpc", "meta", "method", "params": + continue + } + r.ExtraFields = append(r.ExtraFields, RequestField{ + Name: name, + Value: value, + }) } return nil } @@ -126,6 +175,21 @@ func (r *Request) SetMeta(v interface{}) error { return nil } +// SetExtraField adds an entry to r.ExtraFields, so that it is added to the +// JSON representation of the request, as a way to add arbitrary extensions to +// JSON RPC 2.0. If JSON marshaling fails, it returns an error. +func (r *Request) SetExtraField(name string, v interface{}) error { + switch name { + case "id", "jsonrpc", "meta", "method", "params": + return fmt.Errorf("invalid extra field %q", name) + } + r.ExtraFields = append(r.ExtraFields, RequestField{ + Name: name, + Value: v, + }) + return nil +} + // Response represents a JSON-RPC response. See // http://www.jsonrpc.org/specification#response_object. type Response struct { diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index c319a1b..f9dd950 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -24,7 +24,7 @@ func TestRequest_MarshalJSON_jsonrpc(t *testing.T) { if err != nil { t.Fatal(err) } - if want := `{"method":"","id":0,"jsonrpc":"2.0"}`; string(b) != want { + if want := `{"id":0,"jsonrpc":"2.0","method":""}`; string(b) != want { t.Errorf("got %q, want %q", b, want) } } diff --git a/object_test.go b/object_test.go index 2430e3b..cfa5b00 100644 --- a/object_test.go +++ b/object_test.go @@ -44,17 +44,21 @@ func TestRequest_MarshalUnmarshalJSON(t *testing.T) { want Request }{ { - data: []byte(`{"method":"m","params":{"foo":"bar"},"id":123,"jsonrpc":"2.0"}`), + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":{"foo":"bar"}}`), want: Request{ID: ID{Num: 123}, Method: "m", Params: &obj}, }, { - data: []byte(`{"method":"m","params":null,"id":123,"jsonrpc":"2.0"}`), + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":null}`), want: Request{ID: ID{Num: 123}, Method: "m", Params: &null}, }, { - data: []byte(`{"method":"m","id":123,"jsonrpc":"2.0"}`), + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m"}`), want: Request{ID: ID{Num: 123}, Method: "m", Params: nil}, }, + { + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","sessionId":"session"}`), + want: Request{ID: ID{Num: 123}, Method: "m", Params: nil, ExtraFields: []RequestField{{Name: "sessionId", Value: "session"}}}, + }, } for _, test := range tests { var got Request From 120d461fd18e58e769449b4720a3ffcbf0468b54 Mon Sep 17 00:00:00 2001 From: lhchavez Date: Wed, 4 Aug 2021 05:46:32 -0700 Subject: [PATCH 03/29] Add GitHub actions (#51) That extra green checkmark does wonders for peace of mind. --- .github/workflows/ci.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..32d86a7 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,30 @@ +name: CI +on: + pull_request: {} + push: + branches: + - master +jobs: + test: + strategy: + fail-fast: false + matrix: + go: + - 1.16 + name: Go ${{ matrix.go }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go }} + id: go + - name: Get dependencies + run: go get -t -v ./... + - name: Install staticcheck + run: go install honnef.co/go/tools/cmd/staticcheck@latest + - name: Lint + run: staticcheck -checks=all ./... + - name: Test + run: go test -v -race ./... From 5f298fe6a1f608093138e403dd15fe9a3c72757b Mon Sep 17 00:00:00 2001 From: lhchavez Date: Wed, 4 Aug 2021 09:55:02 -0700 Subject: [PATCH 04/29] Homogenize treatment of params and meta in UnmarshalJSON (#52) This change makes the treatment of params and meta the same, by assigning a well-known pointer at first to detect if the unmarshaling process overwrites it with an explicit nil, or it stays the same in which it means that it was unset from the beginning. --- jsonrpc2.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/jsonrpc2.go b/jsonrpc2.go index e6e2251..7815c84 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -85,10 +85,12 @@ func (r Request) MarshalJSON() ([]byte, error) { func (r *Request) UnmarshalJSON(data []byte) error { r2 := make(map[string]interface{}) - // Detect if the "params" field is JSON "null" or just not present - // by seeing if the field gets overwritten to nil. + // Detect if the "params" or "meta" fields are JSON "null" or just not + // present by seeing if the field gets overwritten to nil. emptyParams := &json.RawMessage{} r2["params"] = emptyParams + emptyMeta := &json.RawMessage{} + r2["meta"] = emptyMeta decoder := json.NewDecoder(bytes.NewReader(data)) decoder.UseNumber() @@ -112,9 +114,13 @@ func (r *Request) UnmarshalJSON(data []byte) error { } r.Params = (*json.RawMessage)(&b) } - meta, ok := r2["meta"] - if ok { - b, err := json.Marshal(meta) + switch { + case r2["meta"] == nil: + r.Meta = &jsonNull + case r2["meta"] == emptyMeta: + r.Meta = nil + default: + b, err := json.Marshal(r2["meta"]) if err != nil { return fmt.Errorf("failed to marshal Meta: %w", err) } From c9c77b6bb9be655af1a5870e54930aa2fab012c2 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Fri, 19 Nov 2021 03:30:03 -0500 Subject: [PATCH 05/29] Add ability to set custom logger (#48) Before this commit, a custom logger could be set with the LogMessages function. However, by using LogMessages not only is a custom logger set but also all received and sent messages are logged. Use cases exist where a custom logger is desired to log errors but not all messages. --- conn_opt.go | 7 +++++++ conn_opt_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 conn_opt_test.go diff --git a/conn_opt.go b/conn_opt.go index 3779d7f..423cf80 100644 --- a/conn_opt.go +++ b/conn_opt.go @@ -102,3 +102,10 @@ func LogMessages(logger Logger) ConnOpt { })(c) } } + +// SetLogger sets the logger for the connection. +func SetLogger(logger Logger) ConnOpt { + return func(c *Conn) { + c.logger = logger + } +} diff --git a/conn_opt_test.go b/conn_opt_test.go new file mode 100644 index 0000000..df53a1a --- /dev/null +++ b/conn_opt_test.go @@ -0,0 +1,53 @@ +package jsonrpc2_test + +import ( + "bufio" + "context" + "io" + "log" + "net" + "testing" + + "github.com/sourcegraph/jsonrpc2" +) + +func TestSetLogger(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + rd, wr := io.Pipe() + defer rd.Close() + defer wr.Close() + + buf := bufio.NewReader(rd) + logger := log.New(wr, "", log.Lmsgprefix) + + a, b := net.Pipe() + connA := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewBufferedStream(a, jsonrpc2.VSCodeObjectCodec{}), + noopHandler{}, + jsonrpc2.SetLogger(logger), + ) + connB := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewBufferedStream(b, jsonrpc2.VSCodeObjectCodec{}), + noopHandler{}, + ) + defer connA.Close() + defer connB.Close() + + // Write a response with no corresponding request. + if err := connB.Reply(ctx, jsonrpc2.ID{Num: 0}, nil); err != nil { + t.Fatal(err) + } + + want := "jsonrpc2: ignoring response #0 with no corresponding request\n" + got, err := buf.ReadString('\n') + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} From a896fc3eac98195e9f144050320d8d485564de45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Nowotnik?= Date: Mon, 11 Jul 2022 15:43:39 +0200 Subject: [PATCH 06/29] [#57] Fix and deprecate PlainObjectCodec (#58) This change fixes a bug that causes PlainObjectCodec to lose additional messages from stream. json.Decoder has an internal buffer that reads more than one message, but is discarded after every use. Now PlainObjectCodec reuses encoder and decoder within a buffered stream, however using it directly in your code retains the old, incorrect behaviour. A user should now use plainObjectStream if he needs plain JSON-RPC 2.0 stream without headers. `NewPlainObjectStream` method has been added for this reason. --- jsonrpc2_test.go | 101 +++++++++++++++++++++++++++++------------------ stream.go | 55 ++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 42 deletions(-) diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index f9dd950..ca600db 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -112,44 +112,67 @@ func (h *testHandlerB) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jso h.t.Errorf("testHandlerB got unexpected request %+v", req) } +type streamMaker func(conn io.ReadWriteCloser) jsonrpc2.ObjectStream + +func testClientServerForCodec(t *testing.T, streamMaker streamMaker) { + ctx := context.Background() + done := make(chan struct{}) + + lis, err := net.Listen("tcp", "127.0.0.1:0") // any available address + if err != nil { + t.Fatal("Listen:", err) + } + defer func() { + if lis == nil { + return // already closed + } + if err = lis.Close(); err != nil { + if !strings.HasSuffix(err.Error(), "use of closed network connection") { + t.Fatal(err) + } + } + }() + + ha := testHandlerA{t: t} + go func() { + if err = serve(ctx, lis, &ha, streamMaker); err != nil { + if !strings.HasSuffix(err.Error(), "use of closed network connection") { + t.Error(err) + } + } + close(done) + }() + + conn, err := net.Dial("tcp", lis.Addr().String()) + if err != nil { + t.Fatal("Dial:", err) + } + testClientServer(ctx, t, streamMaker(conn)) + + lis.Close() + <-done // ensure Serve's error return (if any) is caught by this test +} + func TestClientServer(t *testing.T) { - t.Run("tcp", func(t *testing.T) { - ctx := context.Background() - done := make(chan struct{}) - - lis, err := net.Listen("tcp", "127.0.0.1:0") // any available address - if err != nil { - t.Fatal("Listen:", err) - } - defer func() { - if lis == nil { - return // already closed - } - if err = lis.Close(); err != nil { - if !strings.HasSuffix(err.Error(), "use of closed network connection") { - t.Fatal(err) - } - } - }() - - ha := testHandlerA{t: t} - go func() { - if err = serve(ctx, lis, &ha); err != nil { - if !strings.HasSuffix(err.Error(), "use of closed network connection") { - t.Error(err) - } - } - close(done) - }() - - conn, err := net.Dial("tcp", lis.Addr().String()) - if err != nil { - t.Fatal("Dial:", err) - } - testClientServer(ctx, t, jsonrpc2.NewBufferedStream(conn, jsonrpc2.VarintObjectCodec{})) - - lis.Close() - <-done // ensure Serve's error return (if any) is caught by this test + t.Run("tcp-varint-object-codec", func(t *testing.T) { + testClientServerForCodec(t, func(conn io.ReadWriteCloser) jsonrpc2.ObjectStream { + return jsonrpc2.NewBufferedStream(conn, jsonrpc2.VarintObjectCodec{}) + }) + }) + t.Run("tcp-vscode-object-codec", func(t *testing.T) { + testClientServerForCodec(t, func(conn io.ReadWriteCloser) jsonrpc2.ObjectStream { + return jsonrpc2.NewBufferedStream(conn, jsonrpc2.VSCodeObjectCodec{}) + }) + }) + t.Run("tcp-plain-object-codec", func(t *testing.T) { + testClientServerForCodec(t, func(conn io.ReadWriteCloser) jsonrpc2.ObjectStream { + return jsonrpc2.NewBufferedStream(conn, jsonrpc2.PlainObjectCodec{}) + }) + }) + t.Run("tcp-plain-object-stream", func(t *testing.T) { + testClientServerForCodec(t, func(conn io.ReadWriteCloser) jsonrpc2.ObjectStream { + return jsonrpc2.NewPlainObjectStream(conn) + }) }) t.Run("websocket", func(t *testing.T) { ctx := context.Background() @@ -367,12 +390,12 @@ func TestConn_Close_waitingForResponse(t *testing.T) { <-done } -func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, opts ...jsonrpc2.ConnOpt) error { +func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMaker streamMaker, opts ...jsonrpc2.ConnOpt) error { for { conn, err := lis.Accept() if err != nil { return err } - jsonrpc2.NewConn(ctx, jsonrpc2.NewBufferedStream(conn, jsonrpc2.VarintObjectCodec{}), h, opts...) + jsonrpc2.NewConn(ctx, streamMaker(conn), h, opts...) } } diff --git a/stream.go b/stream.go index 7023c65..ff24d0f 100644 --- a/stream.go +++ b/stream.go @@ -40,6 +40,12 @@ type bufferedObjectStream struct { // objectStream is used to produce the bytes to write to the stream // for the JSON-RPC 2.0 objects. func NewBufferedStream(conn io.ReadWriteCloser, codec ObjectCodec) ObjectStream { + switch v := codec.(type) { + case PlainObjectCodec: + v.decoder = json.NewDecoder(conn) + v.encoder = json.NewEncoder(conn) + codec = v + } return &bufferedObjectStream{ conn: conn, w: bufio.NewWriter(conn), @@ -164,14 +170,57 @@ func (VSCodeObjectCodec) ReadObject(stream *bufio.Reader, v interface{}) error { } // PlainObjectCodec reads/writes plain JSON-RPC 2.0 objects without a header. -type PlainObjectCodec struct{} +// +// Deprecated: use NewPlainObjectStream +type PlainObjectCodec struct { + decoder *json.Decoder + encoder *json.Encoder +} // WriteObject implements ObjectCodec. -func (PlainObjectCodec) WriteObject(stream io.Writer, v interface{}) error { +func (c PlainObjectCodec) WriteObject(stream io.Writer, v interface{}) error { + if c.encoder != nil { + return c.encoder.Encode(v) + } return json.NewEncoder(stream).Encode(v) } // ReadObject implements ObjectCodec. -func (PlainObjectCodec) ReadObject(stream *bufio.Reader, v interface{}) error { +func (c PlainObjectCodec) ReadObject(stream *bufio.Reader, v interface{}) error { + if c.decoder != nil { + return c.decoder.Decode(v) + } return json.NewDecoder(stream).Decode(v) } + +// plainObjectStream reads/writes plain JSON-RPC 2.0 objects without a header. +type plainObjectStream struct { + conn io.Closer + decoder *json.Decoder + encoder *json.Encoder +} + +// NewPlainObjectStream creates a buffered stream from a network +// connection (or other similar interface). The underlying +// objectStream produces plain JSON-RPC 2.0 objects without a header. +func NewPlainObjectStream(conn io.ReadWriteCloser) ObjectStream { + return &plainObjectStream{ + conn: conn, + encoder: json.NewEncoder(conn), + decoder: json.NewDecoder(conn), + } +} + +func (os *plainObjectStream) ReadObject(v interface{}) error { + return os.decoder.Decode(v) +} + +// WriteObject serializes a value to JSON and writes it to a stream. +// Not thread-safe, a user must synchronize writes in a multithreaded environment. +func (os *plainObjectStream) WriteObject(v interface{}) error { + return os.encoder.Encode(v) +} + +func (os *plainObjectStream) Close() error { + return os.conn.Close() +} From 7f448843acda50846570df25a0cdc12b6b579588 Mon Sep 17 00:00:00 2001 From: Sourcegraph Date: Tue, 3 Jan 2023 22:03:16 +0000 Subject: [PATCH 07/29] batch changes - update checkout v2 to v3 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32d86a7..f807c82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Go uses: actions/setup-go@v2 with: From e1f9fdf1bbdbf9afb90fd78aeaf3d384fd2102f2 Mon Sep 17 00:00:00 2001 From: Semesse Date: Sat, 14 Jan 2023 02:33:58 +0800 Subject: [PATCH 08/29] fix no corresponding request if req.ID is modified by onSend (#60) --- jsonrpc2.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/jsonrpc2.go b/jsonrpc2.go index 7815c84..0bfcc71 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -432,8 +432,7 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err return nil, ErrClosed } - // Store requests so we can later associate them with incoming - // responses. + // Assign a default id if not set if m.request != nil && wait { cc = &call{request: m.request, seq: c.seq, done: make(chan error, 1)} @@ -445,8 +444,6 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err m.request.ID.Num = c.seq } } - id = m.request.ID - c.pending[id] = cc c.seq++ } c.mu.Unlock() @@ -467,6 +464,15 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err } } + // Store requests so we can later associate them with incoming + // responses. + if m.request != nil && wait { + c.mu.Lock() + id = m.request.ID + c.pending[id] = cc + c.mu.Unlock() + } + // From here on, if we fail to send this, then we need to remove // this from the pending map so we don't block on it or pile up // pending entries for unsent messages. From 8012d49686b17aaf77d91d92e42c998070c69afa Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 24 Jan 2023 01:47:36 -0500 Subject: [PATCH 09/29] Add ability to omit params member from request (#62) The JSON-RPC 2.0 specification allows the params member of a request to be omitted [1]. Before this commit, this library did not allow the params member to be omitted when sending a request. When the params argument of the Conn.Call/Conn.DispatchCall or Conn.Notify method was set to nil, then Request.Params was set to the JSON encoding of nil which is null. This commit adds a CallOption named OmitNilParams. If OmitNilParams is used when sending a request with params set to nil, then the params member in the JSON encoding of Request is omitted. If the OmitNilParams option is not used then the previous behavior is maintained. In other words, the changes in this commit are backwards compatible. References [1]: https://www.jsonrpc.org/specification#request_object --- call_opt.go | 9 ++++ call_opt_test.go | 111 +++++++++++++++++++++++++++++++++++++++++++++++ jsonrpc2.go | 25 +++++++---- 3 files changed, 137 insertions(+), 8 deletions(-) diff --git a/call_opt.go b/call_opt.go index 851baa5..84a04c2 100644 --- a/call_opt.go +++ b/call_opt.go @@ -46,3 +46,12 @@ func StringID() CallOption { return nil }) } + +// OmitNilParams returns a call option that instructs requests to omit params +// values of nil instead of JSON encoding them to null. +func OmitNilParams() CallOption { + return callOptionFunc(func(r *Request) error { + r.OmitNilParams = true + return nil + }) +} diff --git a/call_opt_test.go b/call_opt_test.go index b64f661..aff8003 100644 --- a/call_opt_test.go +++ b/call_opt_test.go @@ -2,7 +2,10 @@ package jsonrpc2_test import ( "context" + "encoding/json" "fmt" + "net" + "sync" "testing" "github.com/sourcegraph/jsonrpc2" @@ -140,3 +143,111 @@ func TestExtraField(t *testing.T) { t.Fatal(err) } } + +func TestOmitNilParams(t *testing.T) { + rawJSONMessage := func(v string) *json.RawMessage { + b := []byte(v) + return (*json.RawMessage)(&b) + } + + type testCase struct { + callOpt jsonrpc2.CallOption + sendParams interface{} + wantParams *json.RawMessage + } + + testCases := []testCase{ + { + sendParams: nil, + wantParams: rawJSONMessage("null"), + }, + { + sendParams: rawJSONMessage("null"), + wantParams: rawJSONMessage("null"), + }, + { + callOpt: jsonrpc2.OmitNilParams(), + sendParams: nil, + wantParams: nil, + }, + { + callOpt: jsonrpc2.OmitNilParams(), + sendParams: rawJSONMessage("null"), + wantParams: rawJSONMessage("null"), + }, + } + + assert := func(got *json.RawMessage, want *json.RawMessage) error { + // Assert pointers. + if got == nil || want == nil { + if got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + } + { + // If pointers are not nil, then assert values. + got := string(*got) + want := string(*want) + if got != want { + return fmt.Errorf("got %q, want %q", got, want) + } + } + return nil + } + + newClientServer := func(handler jsonrpc2.Handler) (client *jsonrpc2.Conn, server *jsonrpc2.Conn) { + ctx := context.Background() + connA, connB := net.Pipe() + client = jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connA), + noopHandler{}, + ) + server = jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connB), + handler, + ) + return client, server + } + + for i, tc := range testCases { + + t.Run(fmt.Sprintf("test case %v", i), func(t *testing.T) { + t.Run("call", func(t *testing.T) { + handler := jsonrpc2.HandlerWithError(func(ctx context.Context, c *jsonrpc2.Conn, r *jsonrpc2.Request) (result interface{}, err error) { + return nil, assert(r.Params, tc.wantParams) + }) + + client, server := newClientServer(handler) + defer client.Close() + defer server.Close() + + if err := client.Call(context.Background(), "f", tc.sendParams, nil, tc.callOpt); err != nil { + t.Fatal(err) + } + }) + t.Run("notify", func(t *testing.T) { + wg := &sync.WaitGroup{} + handler := handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) { + err := assert(req.Params, tc.wantParams) + if err != nil { + t.Error(err) + } + wg.Done() + }) + + client, server := newClientServer(handler) + defer client.Close() + defer server.Close() + + wg.Add(1) + if err := client.Notify(context.Background(), "f", tc.sendParams, tc.callOpt); err != nil { + t.Fatal(err) + } + wg.Wait() + }) + }) + } +} diff --git a/jsonrpc2.go b/jsonrpc2.go index 0bfcc71..4885b05 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -57,6 +57,9 @@ type Request struct { // NOTE: It is not part of the spec, but there are other protocols based on // JSON-RPC 2 that require it. ExtraFields []RequestField `json:"-"` + // OmitNilParams instructs the SetParams method to not JSON encode a nil + // value and set Params to nil instead. + OmitNilParams bool `json:"-"` } // MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" @@ -159,9 +162,15 @@ func (r *Request) UnmarshalJSON(data []byte) error { return nil } -// SetParams sets r.Params to the JSON representation of v. If JSON -// marshaling fails, it returns an error. +// SetParams sets r.Params to the JSON representation of v. If JSON marshaling +// fails, it returns an error. Beware that the JSON encoding of nil is null. If +// r.OmitNilParams is true and v is nil, then r.Params is set to nil and +// therefore omitted from the JSON-RPC request. func (r *Request) SetParams(v interface{}) error { + if r.OmitNilParams && v == nil { + r.Params = nil + return nil + } b, err := json.Marshal(v) if err != nil { return err @@ -511,9 +520,6 @@ func (c *Conn) Call(ctx context.Context, method string, params, result interface // otherwise use Call. func (c *Conn) DispatchCall(ctx context.Context, method string, params interface{}, opts ...CallOption) (Waiter, error) { req := &Request{Method: method} - if err := req.SetParams(params); err != nil { - return Waiter{}, err - } for _, opt := range opts { if opt == nil { continue @@ -522,6 +528,9 @@ func (c *Conn) DispatchCall(ctx context.Context, method string, params interface return Waiter{}, err } } + if err := req.SetParams(params); err != nil { + return Waiter{}, err + } call, err := c.send(ctx, &anyMessage{request: req}, true) if err != nil { return Waiter{}, err @@ -569,9 +578,6 @@ var jsonNull = json.RawMessage("null") // notifications do not have responses). func (c *Conn) Notify(ctx context.Context, method string, params interface{}, opts ...CallOption) error { req := &Request{Method: method, Notif: true} - if err := req.SetParams(params); err != nil { - return err - } for _, opt := range opts { if opt == nil { continue @@ -580,6 +586,9 @@ func (c *Conn) Notify(ctx context.Context, method string, params interface{}, op return err } } + if err := req.SetParams(params); err != nil { + return err + } _, err := c.send(ctx, &anyMessage{request: req}, false) return err } From 78a3d790f32417c0693d60afe55135983594deb0 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Fri, 3 Feb 2023 03:51:22 -0500 Subject: [PATCH 10/29] Pin staticcheck version to v0.2.2 (#63) The CI pipeline has been broken in this project for some time because the latest version of staticcheck is not compatible with every version of Go. Through trial and error, it was discovered that staticcheck v0.2.2 is the latest version that is compatible with Go 1.16. The authors of staticcheck also recommend pinning the version in CI pipelines to prevent unintentional breakage of the build [1]. References [1]: https://staticcheck.io/docs/running-staticcheck/ci/github-actions/#version --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f807c82..9bf8e75 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: - name: Get dependencies run: go get -t -v ./... - name: Install staticcheck - run: go install honnef.co/go/tools/cmd/staticcheck@latest + run: go install honnef.co/go/tools/cmd/staticcheck@v0.2.2 - name: Lint run: staticcheck -checks=all ./... - name: Test From 028a50bb3999aa8fd8ed27d22d40169b60dfd7a5 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 7 Feb 2023 15:46:05 -0500 Subject: [PATCH 11/29] Fix underlying connection not being closed on protocol error (#64) Before this commit, the underlying connection of `Conn` was not being closed when a protocol error was encountered. This behavior contradicted with `Conn.DisconnectNotify()` because it reported that the underlying connection was being closed. Additionally, the underlying connection was now orphaned because `Conn` was no longer processing any of the subsequent requests. With this commit, the underlying connection is now being closed when a protocol error is encountered, matching what `Conn.DisconnectNotify()` has already been reporting. --- jsonrpc2.go | 58 ++++++++---------- jsonrpc2_test.go | 152 ++++++++++++++++++++++++++--------------------- 2 files changed, 110 insertions(+), 100 deletions(-) diff --git a/jsonrpc2.go b/jsonrpc2.go index 4885b05..32bc98c 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -366,11 +366,10 @@ type Conn struct { h Handler - mu sync.Mutex - shutdown bool - closing bool - seq uint64 - pending map[ID]*call + mu sync.Mutex + closed bool + seq uint64 + pending map[ID]*call sending sync.Mutex @@ -417,13 +416,29 @@ func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOp // Close closes the JSON-RPC connection. The connection may not be // used after it has been closed. func (c *Conn) Close() error { + return c.close(nil) +} + +func (c *Conn) close(cause error) error { + c.sending.Lock() c.mu.Lock() - if c.shutdown || c.closing { - c.mu.Unlock() + defer c.sending.Unlock() + defer c.mu.Unlock() + + if c.closed { return ErrClosed } - c.closing = true - c.mu.Unlock() + + for _, call := range c.pending { + close(call.done) + } + + if cause != nil && cause != io.EOF && cause != io.ErrUnexpectedEOF { + c.logger.Printf("jsonrpc2: protocol error: %v\n", cause) + } + + close(c.disconnect) + c.closed = true return c.stream.Close() } @@ -436,7 +451,7 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err var id ID c.mu.Lock() - if c.shutdown || c.closing { + if c.closed { c.mu.Unlock() return nil, ErrClosed } @@ -675,28 +690,7 @@ func (c *Conn) readMessages(ctx context.Context) { } } } - - c.sending.Lock() - c.mu.Lock() - c.shutdown = true - closing := c.closing - if err == io.EOF { - if closing { - err = ErrClosed - } else { - err = io.ErrUnexpectedEOF - } - } - for _, call := range c.pending { - call.done <- err - close(call.done) - } - c.mu.Unlock() - c.sending.Unlock() - if err != io.ErrUnexpectedEOF && !closing { - c.logger.Printf("jsonrpc2: protocol error: %v\n", err) - } - close(c.disconnect) + c.close(err) } // call represents a JSON-RPC call over its entire lifecycle. diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index ca600db..b68f3c1 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -314,80 +314,82 @@ type noopHandler struct{} func (noopHandler) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) {} -type readWriteCloser struct { - read, write func(p []byte) (n int, err error) -} +func TestConn_DisconnectNotify(t *testing.T) { -func (x readWriteCloser) Read(p []byte) (n int, err error) { - return x.read(p) -} - -func (x readWriteCloser) Write(p []byte) (n int, err error) { - return x.write(p) -} - -func (readWriteCloser) Close() error { return nil } - -func eof(p []byte) (n int, err error) { - return 0, io.EOF -} - -func TestConn_DisconnectNotify_EOF(t *testing.T) { - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewBufferedStream(&readWriteCloser{eof, eof}, jsonrpc2.VarintObjectCodec{}), nil) - select { - case <-c.DisconnectNotify(): - case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") - } -} - -func TestConn_DisconnectNotify_Close(t *testing.T) { - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewBufferedStream(&readWriteCloser{eof, eof}, jsonrpc2.VarintObjectCodec{}), nil) - if err := c.Close(); err != nil { - t.Error(err) - } - select { - case <-c.DisconnectNotify(): - case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") - } -} - -func TestConn_DisconnectNotify_Close_async(t *testing.T) { - done := make(chan struct{}) - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewBufferedStream(&readWriteCloser{eof, eof}, jsonrpc2.VarintObjectCodec{}), nil) - go func() { - if err := c.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Run("EOF", func(t *testing.T) { + connA, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + // By closing connA, connB receives io.EOF + if err := connA.Close(); err != nil { t.Error(err) } - close(done) - }() - select { - case <-c.DisconnectNotify(): - case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") - } - <-done + assertDisconnect(t, c, connB) + }) + + t.Run("Close", func(t *testing.T) { + _, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + if err := c.Close(); err != nil { + t.Error(err) + } + assertDisconnect(t, c, connB) + }) + + t.Run("Close async", func(t *testing.T) { + done := make(chan struct{}) + _, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + go func() { + if err := c.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Error(err) + } + close(done) + }() + assertDisconnect(t, c, connB) + <-done + }) + + t.Run("protocol error", func(t *testing.T) { + connA, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + connA.Write([]byte("invalid json")) + assertDisconnect(t, c, connB) + }) } -func TestConn_Close_waitingForResponse(t *testing.T) { - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewBufferedStream(&readWriteCloser{eof, eof}, jsonrpc2.VarintObjectCodec{}), noopHandler{}) - done := make(chan struct{}) - go func() { - if err := c.Call(context.Background(), "m", nil, nil); err != jsonrpc2.ErrClosed { - t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) +func TestConn_Close(t *testing.T) { + t.Run("waiting for response", func(t *testing.T) { + connA, connB := net.Pipe() + nodeA := jsonrpc2.NewConn( + context.Background(), + jsonrpc2.NewPlainObjectStream(connA), noopHandler{}, + ) + defer nodeA.Close() + nodeB := jsonrpc2.NewConn( + context.Background(), + jsonrpc2.NewPlainObjectStream(connB), + noopHandler{}, + ) + defer nodeB.Close() + + ready := make(chan struct{}) + done := make(chan struct{}) + go func() { + close(ready) + err := nodeB.Call(context.Background(), "m", nil, nil) + if err != jsonrpc2.ErrClosed { + t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) + } + close(done) + }() + // Wait for the request to be sent before we close the connection. + <-ready + if err := nodeB.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Error(err) } - close(done) - }() - if err := c.Close(); err != nil && err != jsonrpc2.ErrClosed { - t.Error(err) - } - select { - case <-c.DisconnectNotify(): - case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") - } - <-done + assertDisconnect(t, nodeB, connB) + <-done + }) } func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMaker streamMaker, opts ...jsonrpc2.ConnOpt) error { @@ -399,3 +401,17 @@ func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMake jsonrpc2.NewConn(ctx, streamMaker(conn), h, opts...) } } + +func assertDisconnect(t *testing.T, c *jsonrpc2.Conn, conn io.Writer) { + select { + case <-c.DisconnectNotify(): + case <-time.After(200 * time.Millisecond): + t.Fatal("no disconnect notification") + } + // Assert that conn is closed by trying to write to it. + _, got := conn.Write(nil) + want := io.ErrClosedPipe + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} From 846c29e96d716812f25f9b439f59a81d888c60c3 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Thu, 9 Feb 2023 01:56:42 -0500 Subject: [PATCH 12/29] Split jsonrpc2.go file into multiple files (#65) This merge request moves some of the contents from the jsonrpc2.go file into their own designated file. The new files being introduced (excluding test files) are as follows: * conn.go * request.go * response.go The motive of this change is to make it easier to navigate the code. Without this change, the jsonrpc2.go file is 813 lines of code. --- conn.go | 460 +++++++++++++++++++++++++++++++ conn_test.go | 103 +++++++ internal_test.go | 35 +++ jsonrpc2.go | 693 ----------------------------------------------- jsonrpc2_test.go | 153 ----------- object_test.go | 132 --------- request.go | 183 +++++++++++++ request_test.go | 65 +++++ response.go | 72 +++++ response_test.go | 110 ++++++++ 10 files changed, 1028 insertions(+), 978 deletions(-) create mode 100644 conn.go create mode 100644 conn_test.go create mode 100644 internal_test.go delete mode 100644 object_test.go create mode 100644 request.go create mode 100644 request_test.go create mode 100644 response.go create mode 100644 response_test.go diff --git a/conn.go b/conn.go new file mode 100644 index 0000000..c2016bf --- /dev/null +++ b/conn.go @@ -0,0 +1,460 @@ +package jsonrpc2 + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "log" + "os" + "strconv" + "sync" +) + +// Conn is a JSON-RPC client/server connection. The JSON-RPC protocol +// is symmetric, so a Conn runs on both ends of a client-server +// connection. +type Conn struct { + stream ObjectStream + + h Handler + + mu sync.Mutex + closed bool + seq uint64 + pending map[ID]*call + + sending sync.Mutex + + disconnect chan struct{} + + logger Logger + + // Set by ConnOpt funcs. + onRecv []func(*Request, *Response) + onSend []func(*Request, *Response) +} + +var _ JSONRPC2 = (*Conn)(nil) + +// NewConn creates a new JSON-RPC client/server connection using the +// given ReadWriteCloser (typically a TCP connection or stdio). The +// JSON-RPC protocol is symmetric, so a Conn runs on both ends of a +// client-server connection. +// +// NewClient consumes conn, so you should call Close on the returned +// client not on the given conn. +func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOpt) *Conn { + c := &Conn{ + stream: stream, + h: h, + pending: map[ID]*call{}, + disconnect: make(chan struct{}), + logger: log.New(os.Stderr, "", log.LstdFlags), + } + for _, opt := range opts { + if opt == nil { + continue + } + opt(c) + } + go c.readMessages(ctx) + return c +} + +// Close closes the JSON-RPC connection. The connection may not be +// used after it has been closed. +func (c *Conn) Close() error { + return c.close(nil) +} + +// Call initiates a JSON-RPC call using the specified method and +// params, and waits for the response. If the response is successful, +// its result is stored in result (a pointer to a value that can be +// JSON-unmarshaled into); otherwise, a non-nil error is returned. +func (c *Conn) Call(ctx context.Context, method string, params, result interface{}, opts ...CallOption) error { + call, err := c.DispatchCall(ctx, method, params, opts...) + if err != nil { + return err + } + return call.Wait(ctx, result) +} + +// DisconnectNotify returns a channel that is closed when the +// underlying connection is disconnected. +func (c *Conn) DisconnectNotify() <-chan struct{} { + return c.disconnect +} + +// DispatchCall dispatches a JSON-RPC call using the specified method +// and params, and returns a call proxy or an error. Call Wait() +// on the returned proxy to receive the response. Only use this +// function if you need to do work after dispatching the request, +// otherwise use Call. +func (c *Conn) DispatchCall(ctx context.Context, method string, params interface{}, opts ...CallOption) (Waiter, error) { + req := &Request{Method: method} + for _, opt := range opts { + if opt == nil { + continue + } + if err := opt.apply(req); err != nil { + return Waiter{}, err + } + } + if err := req.SetParams(params); err != nil { + return Waiter{}, err + } + call, err := c.send(ctx, &anyMessage{request: req}, true) + if err != nil { + return Waiter{}, err + } + return Waiter{call: call}, nil +} + +// Notify is like Call, but it returns when the notification request +// is sent (without waiting for a response, because JSON-RPC +// notifications do not have responses). +func (c *Conn) Notify(ctx context.Context, method string, params interface{}, opts ...CallOption) error { + req := &Request{Method: method, Notif: true} + for _, opt := range opts { + if opt == nil { + continue + } + if err := opt.apply(req); err != nil { + return err + } + } + if err := req.SetParams(params); err != nil { + return err + } + _, err := c.send(ctx, &anyMessage{request: req}, false) + return err +} + +// Reply sends a successful response with a result. +func (c *Conn) Reply(ctx context.Context, id ID, result interface{}) error { + resp := &Response{ID: id} + if err := resp.SetResult(result); err != nil { + return err + } + _, err := c.send(ctx, &anyMessage{response: resp}, false) + return err +} + +// ReplyWithError sends a response with an error. +func (c *Conn) ReplyWithError(ctx context.Context, id ID, respErr *Error) error { + _, err := c.send(ctx, &anyMessage{response: &Response{ID: id, Error: respErr}}, false) + return err +} + +// SendResponse sends resp to the peer. It is lower level than (*Conn).Reply. +func (c *Conn) SendResponse(ctx context.Context, resp *Response) error { + _, err := c.send(ctx, &anyMessage{response: resp}, false) + return err +} + +func (c *Conn) close(cause error) error { + c.sending.Lock() + c.mu.Lock() + defer c.sending.Unlock() + defer c.mu.Unlock() + + if c.closed { + return ErrClosed + } + + for _, call := range c.pending { + close(call.done) + } + + if cause != nil && cause != io.EOF && cause != io.ErrUnexpectedEOF { + c.logger.Printf("jsonrpc2: protocol error: %v\n", cause) + } + + close(c.disconnect) + c.closed = true + return c.stream.Close() +} + +func (c *Conn) readMessages(ctx context.Context) { + var err error + for err == nil { + var m anyMessage + err = c.stream.ReadObject(&m) + if err != nil { + break + } + + switch { + case m.request != nil: + for _, onRecv := range c.onRecv { + onRecv(m.request, nil) + } + c.h.Handle(ctx, c, m.request) + + case m.response != nil: + resp := m.response + if resp != nil { + id := resp.ID + c.mu.Lock() + call := c.pending[id] + delete(c.pending, id) + c.mu.Unlock() + + if call != nil { + call.response = resp + } + + if len(c.onRecv) > 0 { + var req *Request + if call != nil { + req = call.request + } + for _, onRecv := range c.onRecv { + onRecv(req, resp) + } + } + + switch { + case call == nil: + c.logger.Printf("jsonrpc2: ignoring response #%s with no corresponding request\n", id) + + case resp.Error != nil: + call.done <- resp.Error + close(call.done) + + default: + call.done <- nil + close(call.done) + } + } + } + } + c.close(err) +} + +func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err error) { + c.sending.Lock() + defer c.sending.Unlock() + + // m.request.ID could be changed, so we store a copy to correctly + // clean up pending + var id ID + + c.mu.Lock() + if c.closed { + c.mu.Unlock() + return nil, ErrClosed + } + + // Assign a default id if not set + if m.request != nil && wait { + cc = &call{request: m.request, seq: c.seq, done: make(chan error, 1)} + + isIDUnset := len(m.request.ID.Str) == 0 && m.request.ID.Num == 0 + if isIDUnset { + if m.request.ID.IsString { + m.request.ID.Str = strconv.FormatUint(c.seq, 10) + } else { + m.request.ID.Num = c.seq + } + } + c.seq++ + } + c.mu.Unlock() + + if len(c.onSend) > 0 { + var ( + req *Request + resp *Response + ) + switch { + case m.request != nil: + req = m.request + case m.response != nil: + resp = m.response + } + for _, onSend := range c.onSend { + onSend(req, resp) + } + } + + // Store requests so we can later associate them with incoming + // responses. + if m.request != nil && wait { + c.mu.Lock() + id = m.request.ID + c.pending[id] = cc + c.mu.Unlock() + } + + // From here on, if we fail to send this, then we need to remove + // this from the pending map so we don't block on it or pile up + // pending entries for unsent messages. + defer func() { + if err != nil { + if cc != nil { + c.mu.Lock() + delete(c.pending, id) + c.mu.Unlock() + } + } + }() + + if err := c.stream.WriteObject(m); err != nil { + return nil, err + } + return cc, nil +} + +// Waiter proxies an ongoing JSON-RPC call. +type Waiter struct { + *call +} + +// Wait for the result of an ongoing JSON-RPC call. If the response +// is successful, its result is stored in result (a pointer to a +// value that can be JSON-unmarshaled into); otherwise, a non-nil +// error is returned. +func (w Waiter) Wait(ctx context.Context, result interface{}) error { + select { + case err, ok := <-w.call.done: + if !ok { + err = ErrClosed + } + if err != nil { + return err + } + if result != nil { + if w.call.response.Result == nil { + w.call.response.Result = &jsonNull + } + if err := json.Unmarshal(*w.call.response.Result, result); err != nil { + return err + } + } + return nil + + case <-ctx.Done(): + return ctx.Err() + } +} + +// call represents a JSON-RPC call over its entire lifecycle. +type call struct { + request *Request + response *Response + seq uint64 // the seq of the request + done chan error +} + +// anyMessage represents either a JSON Request or Response. +type anyMessage struct { + request *Request + response *Response +} + +func (m anyMessage) MarshalJSON() ([]byte, error) { + var v interface{} + switch { + case m.request != nil && m.response == nil: + v = m.request + case m.request == nil && m.response != nil: + v = m.response + } + if v != nil { + return json.Marshal(v) + } + return nil, errors.New("jsonrpc2: message must have exactly one of the request or response fields set") +} + +func (m *anyMessage) UnmarshalJSON(data []byte) error { + // The presence of these fields distinguishes between the 2 + // message types. + type msg struct { + ID interface{} `json:"id"` + Method *string `json:"method"` + Result anyValueWithExplicitNull `json:"result"` + Error interface{} `json:"error"` + } + + var isRequest, isResponse bool + checkType := func(m *msg) error { + mIsRequest := m.Method != nil + mIsResponse := m.Result.null || m.Result.value != nil || m.Error != nil + if (!mIsRequest && !mIsResponse) || (mIsRequest && mIsResponse) { + return errors.New("jsonrpc2: unable to determine message type (request or response)") + } + if (mIsRequest && isResponse) || (mIsResponse && isRequest) { + return errors.New("jsonrpc2: batch message type mismatch (must be all requests or all responses)") + } + isRequest = mIsRequest + isResponse = mIsResponse + return nil + } + + if isArray := len(data) > 0 && data[0] == '['; isArray { + var msgs []msg + if err := json.Unmarshal(data, &msgs); err != nil { + return err + } + if len(msgs) == 0 { + return errors.New("jsonrpc2: invalid empty batch") + } + for i := range msgs { + if err := checkType(&msg{ + ID: msgs[i].ID, + Method: msgs[i].Method, + Result: msgs[i].Result, + Error: msgs[i].Error, + }); err != nil { + return err + } + } + } else { + var m msg + if err := json.Unmarshal(data, &m); err != nil { + return err + } + if err := checkType(&m); err != nil { + return err + } + } + + var v interface{} + switch { + case isRequest && !isResponse: + v = &m.request + case !isRequest && isResponse: + v = &m.response + } + if err := json.Unmarshal(data, v); err != nil { + return err + } + if !isRequest && isResponse && m.response.Error == nil && m.response.Result == nil { + m.response.Result = &jsonNull + } + return nil +} + +// anyValueWithExplicitNull is used to distinguish {} from +// {"result":null} by anyMessage's JSON unmarshaler. +type anyValueWithExplicitNull struct { + null bool // JSON "null" + value interface{} +} + +func (v anyValueWithExplicitNull) MarshalJSON() ([]byte, error) { + return json.Marshal(v.value) +} + +func (v *anyValueWithExplicitNull) UnmarshalJSON(data []byte) error { + data = bytes.TrimSpace(data) + if string(data) == "null" { + *v = anyValueWithExplicitNull{null: true} + return nil + } + *v = anyValueWithExplicitNull{} + return json.Unmarshal(data, &v.value) +} diff --git a/conn_test.go b/conn_test.go new file mode 100644 index 0000000..773c5cb --- /dev/null +++ b/conn_test.go @@ -0,0 +1,103 @@ +package jsonrpc2_test + +import ( + "context" + "io" + "net" + "testing" + "time" + + "github.com/sourcegraph/jsonrpc2" +) + +func TestConn_DisconnectNotify(t *testing.T) { + + t.Run("EOF", func(t *testing.T) { + connA, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + // By closing connA, connB receives io.EOF + if err := connA.Close(); err != nil { + t.Error(err) + } + assertDisconnect(t, c, connB) + }) + + t.Run("Close", func(t *testing.T) { + _, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + if err := c.Close(); err != nil { + t.Error(err) + } + assertDisconnect(t, c, connB) + }) + + t.Run("Close async", func(t *testing.T) { + done := make(chan struct{}) + _, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + go func() { + if err := c.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Error(err) + } + close(done) + }() + assertDisconnect(t, c, connB) + <-done + }) + + t.Run("protocol error", func(t *testing.T) { + connA, connB := net.Pipe() + c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + connA.Write([]byte("invalid json")) + assertDisconnect(t, c, connB) + }) +} + +func TestConn_Close(t *testing.T) { + t.Run("waiting for response", func(t *testing.T) { + connA, connB := net.Pipe() + nodeA := jsonrpc2.NewConn( + context.Background(), + jsonrpc2.NewPlainObjectStream(connA), noopHandler{}, + ) + defer nodeA.Close() + nodeB := jsonrpc2.NewConn( + context.Background(), + jsonrpc2.NewPlainObjectStream(connB), + noopHandler{}, + ) + defer nodeB.Close() + + ready := make(chan struct{}) + done := make(chan struct{}) + go func() { + close(ready) + err := nodeB.Call(context.Background(), "m", nil, nil) + if err != jsonrpc2.ErrClosed { + t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) + } + close(done) + }() + // Wait for the request to be sent before we close the connection. + <-ready + if err := nodeB.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Error(err) + } + assertDisconnect(t, nodeB, connB) + <-done + }) +} + +func assertDisconnect(t *testing.T, c *jsonrpc2.Conn, conn io.Writer) { + select { + case <-c.DisconnectNotify(): + case <-time.After(200 * time.Millisecond): + t.Fatal("no disconnect notification") + } + // Assert that conn is closed by trying to write to it. + _, got := conn.Write(nil) + want := io.ErrClosedPipe + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} diff --git a/internal_test.go b/internal_test.go new file mode 100644 index 0000000..990fb24 --- /dev/null +++ b/internal_test.go @@ -0,0 +1,35 @@ +package jsonrpc2 + +import ( + "encoding/json" + "testing" +) + +func TestAnyMessage(t *testing.T) { + tests := map[string]struct { + request, response, invalid bool + }{ + // Single messages + `{}`: {invalid: true}, + `{"foo":"bar"}`: {invalid: true}, + `{"method":"m"}`: {request: true}, + `{"result":123}`: {response: true}, + `{"result":null}`: {response: true}, + `{"error":{"code":456,"message":"m"}}`: {response: true}, + } + for s, want := range tests { + var m anyMessage + if err := json.Unmarshal([]byte(s), &m); err != nil { + if !want.invalid { + t.Errorf("%s: error: %v", s, err) + } + continue + } + if (m.request != nil) != want.request { + t.Errorf("%s: got request %v, want %v", s, m.request != nil, want.request) + } + if (m.response != nil) != want.response { + t.Errorf("%s: got response %v, want %v", s, m.response != nil, want.response) + } + } +} diff --git a/jsonrpc2.go b/jsonrpc2.go index 32bc98c..17e1a59 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -3,16 +3,11 @@ package jsonrpc2 import ( - "bytes" "context" "encoding/json" "errors" "fmt" - "io" - "log" - "os" "strconv" - "sync" ) // JSONRPC2 describes an interface for issuing requests that speak the @@ -30,247 +25,6 @@ type JSONRPC2 interface { Close() error } -// RequestField is a top-level field that can be added to the JSON-RPC request. -type RequestField struct { - Name string - Value interface{} -} - -// Request represents a JSON-RPC request or -// notification. See -// http://www.jsonrpc.org/specification#request_object and -// http://www.jsonrpc.org/specification#notification. -type Request struct { - Method string `json:"method"` - Params *json.RawMessage `json:"params,omitempty"` - ID ID `json:"id"` - Notif bool `json:"-"` - - // Meta optionally provides metadata to include in the request. - // - // NOTE: It is not part of spec. However, it is useful for propogating - // tracing context, etc. - Meta *json.RawMessage `json:"meta,omitempty"` - - // ExtraFields optionally adds fields to the root of the JSON-RPC request. - // - // NOTE: It is not part of the spec, but there are other protocols based on - // JSON-RPC 2 that require it. - ExtraFields []RequestField `json:"-"` - // OmitNilParams instructs the SetParams method to not JSON encode a nil - // value and set Params to nil instead. - OmitNilParams bool `json:"-"` -} - -// MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" -// property. -func (r Request) MarshalJSON() ([]byte, error) { - r2 := map[string]interface{}{ - "jsonrpc": "2.0", - "method": r.Method, - } - for _, field := range r.ExtraFields { - r2[field.Name] = field.Value - } - if !r.Notif { - r2["id"] = &r.ID - } - if r.Params != nil { - r2["params"] = r.Params - } - if r.Meta != nil { - r2["meta"] = r.Meta - } - return json.Marshal(r2) -} - -// UnmarshalJSON implements json.Unmarshaler. -func (r *Request) UnmarshalJSON(data []byte) error { - r2 := make(map[string]interface{}) - - // Detect if the "params" or "meta" fields are JSON "null" or just not - // present by seeing if the field gets overwritten to nil. - emptyParams := &json.RawMessage{} - r2["params"] = emptyParams - emptyMeta := &json.RawMessage{} - r2["meta"] = emptyMeta - - decoder := json.NewDecoder(bytes.NewReader(data)) - decoder.UseNumber() - if err := decoder.Decode(&r2); err != nil { - return err - } - var ok bool - r.Method, ok = r2["method"].(string) - if !ok { - return errors.New("missing method field") - } - switch { - case r2["params"] == nil: - r.Params = &jsonNull - case r2["params"] == emptyParams: - r.Params = nil - default: - b, err := json.Marshal(r2["params"]) - if err != nil { - return fmt.Errorf("failed to marshal params: %w", err) - } - r.Params = (*json.RawMessage)(&b) - } - switch { - case r2["meta"] == nil: - r.Meta = &jsonNull - case r2["meta"] == emptyMeta: - r.Meta = nil - default: - b, err := json.Marshal(r2["meta"]) - if err != nil { - return fmt.Errorf("failed to marshal Meta: %w", err) - } - r.Meta = (*json.RawMessage)(&b) - } - switch rawID := r2["id"].(type) { - case nil: - r.ID = ID{} - r.Notif = true - case string: - r.ID = ID{Str: rawID, IsString: true} - r.Notif = false - case json.Number: - id, err := rawID.Int64() - if err != nil { - return fmt.Errorf("failed to unmarshal ID: %w", err) - } - r.ID = ID{Num: uint64(id)} - r.Notif = false - default: - return fmt.Errorf("unexpected ID type: %T", rawID) - } - - // Clear the extra fields before populating them again. - r.ExtraFields = nil - for name, value := range r2 { - switch name { - case "id", "jsonrpc", "meta", "method", "params": - continue - } - r.ExtraFields = append(r.ExtraFields, RequestField{ - Name: name, - Value: value, - }) - } - return nil -} - -// SetParams sets r.Params to the JSON representation of v. If JSON marshaling -// fails, it returns an error. Beware that the JSON encoding of nil is null. If -// r.OmitNilParams is true and v is nil, then r.Params is set to nil and -// therefore omitted from the JSON-RPC request. -func (r *Request) SetParams(v interface{}) error { - if r.OmitNilParams && v == nil { - r.Params = nil - return nil - } - b, err := json.Marshal(v) - if err != nil { - return err - } - r.Params = (*json.RawMessage)(&b) - return nil -} - -// SetMeta sets r.Meta to the JSON representation of v. If JSON -// marshaling fails, it returns an error. -func (r *Request) SetMeta(v interface{}) error { - b, err := json.Marshal(v) - if err != nil { - return err - } - r.Meta = (*json.RawMessage)(&b) - return nil -} - -// SetExtraField adds an entry to r.ExtraFields, so that it is added to the -// JSON representation of the request, as a way to add arbitrary extensions to -// JSON RPC 2.0. If JSON marshaling fails, it returns an error. -func (r *Request) SetExtraField(name string, v interface{}) error { - switch name { - case "id", "jsonrpc", "meta", "method", "params": - return fmt.Errorf("invalid extra field %q", name) - } - r.ExtraFields = append(r.ExtraFields, RequestField{ - Name: name, - Value: v, - }) - return nil -} - -// Response represents a JSON-RPC response. See -// http://www.jsonrpc.org/specification#response_object. -type Response struct { - ID ID `json:"id"` - Result *json.RawMessage `json:"result,omitempty"` - Error *Error `json:"error,omitempty"` - - // Meta optionally provides metadata to include in the response. - // - // NOTE: It is not part of spec. However, it is useful for propogating - // tracing context, etc. - Meta *json.RawMessage `json:"meta,omitempty"` - - // SPEC NOTE: The spec says "If there was an error in detecting - // the id in the Request object (e.g. Parse error/Invalid - // Request), it MUST be Null." If we made the ID field nullable, - // then we'd have to make it a pointer type. For simplicity, we're - // ignoring the case where there was an error in detecting the ID - // in the Request object. -} - -// MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" -// property. -func (r Response) MarshalJSON() ([]byte, error) { - if (r.Result == nil || len(*r.Result) == 0) && r.Error == nil { - return nil, errors.New("can't marshal *jsonrpc2.Response (must have result or error)") - } - type tmpType Response // avoid infinite MarshalJSON recursion - b, err := json.Marshal(tmpType(r)) - if err != nil { - return nil, err - } - b = append(b[:len(b)-1], []byte(`,"jsonrpc":"2.0"}`)...) - return b, nil -} - -// UnmarshalJSON implements json.Unmarshaler. -func (r *Response) UnmarshalJSON(data []byte) error { - type tmpType Response - - // Detect if the "result" field is JSON "null" or just not present - // by seeing if the field gets overwritten to nil. - *r = Response{Result: &json.RawMessage{}} - - if err := json.Unmarshal(data, (*tmpType)(r)); err != nil { - return err - } - if r.Result == nil { // JSON "null" - r.Result = &jsonNull - } else if len(*r.Result) == 0 { - r.Result = nil - } - return nil -} - -// SetResult sets r.Result to the JSON representation of v. If JSON -// marshaling fails, it returns an error. -func (r *Response) SetResult(v interface{}) error { - b, err := json.Marshal(v) - if err != nil { - return err - } - r.Result = (*json.RawMessage)(&b) - return nil -} - // Error represents a JSON-RPC response error. type Error struct { Code int64 `json:"code"` @@ -358,455 +112,8 @@ func (id *ID) UnmarshalJSON(data []byte) error { return nil } -// Conn is a JSON-RPC client/server connection. The JSON-RPC protocol -// is symmetric, so a Conn runs on both ends of a client-server -// connection. -type Conn struct { - stream ObjectStream - - h Handler - - mu sync.Mutex - closed bool - seq uint64 - pending map[ID]*call - - sending sync.Mutex - - disconnect chan struct{} - - logger Logger - - // Set by ConnOpt funcs. - onRecv []func(*Request, *Response) - onSend []func(*Request, *Response) -} - -var _ JSONRPC2 = (*Conn)(nil) - // ErrClosed indicates that the JSON-RPC connection is closed (or in // the process of closing). var ErrClosed = errors.New("jsonrpc2: connection is closed") -// NewConn creates a new JSON-RPC client/server connection using the -// given ReadWriteCloser (typically a TCP connection or stdio). The -// JSON-RPC protocol is symmetric, so a Conn runs on both ends of a -// client-server connection. -// -// NewClient consumes conn, so you should call Close on the returned -// client not on the given conn. -func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOpt) *Conn { - c := &Conn{ - stream: stream, - h: h, - pending: map[ID]*call{}, - disconnect: make(chan struct{}), - logger: log.New(os.Stderr, "", log.LstdFlags), - } - for _, opt := range opts { - if opt == nil { - continue - } - opt(c) - } - go c.readMessages(ctx) - return c -} - -// Close closes the JSON-RPC connection. The connection may not be -// used after it has been closed. -func (c *Conn) Close() error { - return c.close(nil) -} - -func (c *Conn) close(cause error) error { - c.sending.Lock() - c.mu.Lock() - defer c.sending.Unlock() - defer c.mu.Unlock() - - if c.closed { - return ErrClosed - } - - for _, call := range c.pending { - close(call.done) - } - - if cause != nil && cause != io.EOF && cause != io.ErrUnexpectedEOF { - c.logger.Printf("jsonrpc2: protocol error: %v\n", cause) - } - - close(c.disconnect) - c.closed = true - return c.stream.Close() -} - -func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err error) { - c.sending.Lock() - defer c.sending.Unlock() - - // m.request.ID could be changed, so we store a copy to correctly - // clean up pending - var id ID - - c.mu.Lock() - if c.closed { - c.mu.Unlock() - return nil, ErrClosed - } - - // Assign a default id if not set - if m.request != nil && wait { - cc = &call{request: m.request, seq: c.seq, done: make(chan error, 1)} - - isIDUnset := len(m.request.ID.Str) == 0 && m.request.ID.Num == 0 - if isIDUnset { - if m.request.ID.IsString { - m.request.ID.Str = strconv.FormatUint(c.seq, 10) - } else { - m.request.ID.Num = c.seq - } - } - c.seq++ - } - c.mu.Unlock() - - if len(c.onSend) > 0 { - var ( - req *Request - resp *Response - ) - switch { - case m.request != nil: - req = m.request - case m.response != nil: - resp = m.response - } - for _, onSend := range c.onSend { - onSend(req, resp) - } - } - - // Store requests so we can later associate them with incoming - // responses. - if m.request != nil && wait { - c.mu.Lock() - id = m.request.ID - c.pending[id] = cc - c.mu.Unlock() - } - - // From here on, if we fail to send this, then we need to remove - // this from the pending map so we don't block on it or pile up - // pending entries for unsent messages. - defer func() { - if err != nil { - if cc != nil { - c.mu.Lock() - delete(c.pending, id) - c.mu.Unlock() - } - } - }() - - if err := c.stream.WriteObject(m); err != nil { - return nil, err - } - return cc, nil -} - -// Call initiates a JSON-RPC call using the specified method and -// params, and waits for the response. If the response is successful, -// its result is stored in result (a pointer to a value that can be -// JSON-unmarshaled into); otherwise, a non-nil error is returned. -func (c *Conn) Call(ctx context.Context, method string, params, result interface{}, opts ...CallOption) error { - call, err := c.DispatchCall(ctx, method, params, opts...) - if err != nil { - return err - } - return call.Wait(ctx, result) -} - -// DispatchCall dispatches a JSON-RPC call using the specified method -// and params, and returns a call proxy or an error. Call Wait() -// on the returned proxy to receive the response. Only use this -// function if you need to do work after dispatching the request, -// otherwise use Call. -func (c *Conn) DispatchCall(ctx context.Context, method string, params interface{}, opts ...CallOption) (Waiter, error) { - req := &Request{Method: method} - for _, opt := range opts { - if opt == nil { - continue - } - if err := opt.apply(req); err != nil { - return Waiter{}, err - } - } - if err := req.SetParams(params); err != nil { - return Waiter{}, err - } - call, err := c.send(ctx, &anyMessage{request: req}, true) - if err != nil { - return Waiter{}, err - } - return Waiter{call: call}, nil -} - -// Waiter proxies an ongoing JSON-RPC call. -type Waiter struct { - *call -} - -// Wait for the result of an ongoing JSON-RPC call. If the response -// is successful, its result is stored in result (a pointer to a -// value that can be JSON-unmarshaled into); otherwise, a non-nil -// error is returned. -func (w Waiter) Wait(ctx context.Context, result interface{}) error { - select { - case err, ok := <-w.call.done: - if !ok { - err = ErrClosed - } - if err != nil { - return err - } - if result != nil { - if w.call.response.Result == nil { - w.call.response.Result = &jsonNull - } - if err := json.Unmarshal(*w.call.response.Result, result); err != nil { - return err - } - } - return nil - - case <-ctx.Done(): - return ctx.Err() - } -} - var jsonNull = json.RawMessage("null") - -// Notify is like Call, but it returns when the notification request -// is sent (without waiting for a response, because JSON-RPC -// notifications do not have responses). -func (c *Conn) Notify(ctx context.Context, method string, params interface{}, opts ...CallOption) error { - req := &Request{Method: method, Notif: true} - for _, opt := range opts { - if opt == nil { - continue - } - if err := opt.apply(req); err != nil { - return err - } - } - if err := req.SetParams(params); err != nil { - return err - } - _, err := c.send(ctx, &anyMessage{request: req}, false) - return err -} - -// Reply sends a successful response with a result. -func (c *Conn) Reply(ctx context.Context, id ID, result interface{}) error { - resp := &Response{ID: id} - if err := resp.SetResult(result); err != nil { - return err - } - _, err := c.send(ctx, &anyMessage{response: resp}, false) - return err -} - -// ReplyWithError sends a response with an error. -func (c *Conn) ReplyWithError(ctx context.Context, id ID, respErr *Error) error { - _, err := c.send(ctx, &anyMessage{response: &Response{ID: id, Error: respErr}}, false) - return err -} - -// SendResponse sends resp to the peer. It is lower level than (*Conn).Reply. -func (c *Conn) SendResponse(ctx context.Context, resp *Response) error { - _, err := c.send(ctx, &anyMessage{response: resp}, false) - return err -} - -// DisconnectNotify returns a channel that is closed when the -// underlying connection is disconnected. -func (c *Conn) DisconnectNotify() <-chan struct{} { - return c.disconnect -} - -func (c *Conn) readMessages(ctx context.Context) { - var err error - for err == nil { - var m anyMessage - err = c.stream.ReadObject(&m) - if err != nil { - break - } - - switch { - case m.request != nil: - for _, onRecv := range c.onRecv { - onRecv(m.request, nil) - } - c.h.Handle(ctx, c, m.request) - - case m.response != nil: - resp := m.response - if resp != nil { - id := resp.ID - c.mu.Lock() - call := c.pending[id] - delete(c.pending, id) - c.mu.Unlock() - - if call != nil { - call.response = resp - } - - if len(c.onRecv) > 0 { - var req *Request - if call != nil { - req = call.request - } - for _, onRecv := range c.onRecv { - onRecv(req, resp) - } - } - - switch { - case call == nil: - c.logger.Printf("jsonrpc2: ignoring response #%s with no corresponding request\n", id) - - case resp.Error != nil: - call.done <- resp.Error - close(call.done) - - default: - call.done <- nil - close(call.done) - } - } - } - } - c.close(err) -} - -// call represents a JSON-RPC call over its entire lifecycle. -type call struct { - request *Request - response *Response - seq uint64 // the seq of the request - done chan error -} - -// anyMessage represents either a JSON Request or Response. -type anyMessage struct { - request *Request - response *Response -} - -func (m anyMessage) MarshalJSON() ([]byte, error) { - var v interface{} - switch { - case m.request != nil && m.response == nil: - v = m.request - case m.request == nil && m.response != nil: - v = m.response - } - if v != nil { - return json.Marshal(v) - } - return nil, errors.New("jsonrpc2: message must have exactly one of the request or response fields set") -} - -func (m *anyMessage) UnmarshalJSON(data []byte) error { - // The presence of these fields distinguishes between the 2 - // message types. - type msg struct { - ID interface{} `json:"id"` - Method *string `json:"method"` - Result anyValueWithExplicitNull `json:"result"` - Error interface{} `json:"error"` - } - - var isRequest, isResponse bool - checkType := func(m *msg) error { - mIsRequest := m.Method != nil - mIsResponse := m.Result.null || m.Result.value != nil || m.Error != nil - if (!mIsRequest && !mIsResponse) || (mIsRequest && mIsResponse) { - return errors.New("jsonrpc2: unable to determine message type (request or response)") - } - if (mIsRequest && isResponse) || (mIsResponse && isRequest) { - return errors.New("jsonrpc2: batch message type mismatch (must be all requests or all responses)") - } - isRequest = mIsRequest - isResponse = mIsResponse - return nil - } - - if isArray := len(data) > 0 && data[0] == '['; isArray { - var msgs []msg - if err := json.Unmarshal(data, &msgs); err != nil { - return err - } - if len(msgs) == 0 { - return errors.New("jsonrpc2: invalid empty batch") - } - for i := range msgs { - if err := checkType(&msg{ - ID: msgs[i].ID, - Method: msgs[i].Method, - Result: msgs[i].Result, - Error: msgs[i].Error, - }); err != nil { - return err - } - } - } else { - var m msg - if err := json.Unmarshal(data, &m); err != nil { - return err - } - if err := checkType(&m); err != nil { - return err - } - } - - var v interface{} - switch { - case isRequest && !isResponse: - v = &m.request - case !isRequest && isResponse: - v = &m.response - } - if err := json.Unmarshal(data, v); err != nil { - return err - } - if !isRequest && isResponse && m.response.Error == nil && m.response.Result == nil { - m.response.Result = &jsonNull - } - return nil -} - -// anyValueWithExplicitNull is used to distinguish {} from -// {"result":null} by anyMessage's JSON unmarshaler. -type anyValueWithExplicitNull struct { - null bool // JSON "null" - value interface{} -} - -func (v anyValueWithExplicitNull) MarshalJSON() ([]byte, error) { - return json.Marshal(v.value) -} - -func (v *anyValueWithExplicitNull) UnmarshalJSON(data []byte) error { - data = bytes.TrimSpace(data) - if string(data) == "null" { - *v = anyValueWithExplicitNull{null: true} - return nil - } - *v = anyValueWithExplicitNull{} - return json.Unmarshal(data, &v.value) -} diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index b68f3c1..1a73744 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -1,7 +1,6 @@ package jsonrpc2_test import ( - "bytes" "context" "encoding/json" "fmt" @@ -19,66 +18,6 @@ import ( websocketjsonrpc2 "github.com/sourcegraph/jsonrpc2/websocket" ) -func TestRequest_MarshalJSON_jsonrpc(t *testing.T) { - b, err := json.Marshal(&jsonrpc2.Request{}) - if err != nil { - t.Fatal(err) - } - if want := `{"id":0,"jsonrpc":"2.0","method":""}`; string(b) != want { - t.Errorf("got %q, want %q", b, want) - } -} - -func TestResponse_MarshalJSON_jsonrpc(t *testing.T) { - null := json.RawMessage("null") - b, err := json.Marshal(&jsonrpc2.Response{Result: &null}) - if err != nil { - t.Fatal(err) - } - if want := `{"id":0,"result":null,"jsonrpc":"2.0"}`; string(b) != want { - t.Errorf("got %q, want %q", b, want) - } -} - -func TestResponseMarshalJSON_Notif(t *testing.T) { - tests := map[*jsonrpc2.Request]bool{ - {ID: jsonrpc2.ID{Num: 0}}: true, - {ID: jsonrpc2.ID{Num: 1}}: true, - {ID: jsonrpc2.ID{Str: "", IsString: true}}: true, - {ID: jsonrpc2.ID{Str: "a", IsString: true}}: true, - {Notif: true}: false, - } - for r, wantIDKey := range tests { - b, err := json.Marshal(r) - if err != nil { - t.Fatal(err) - } - hasIDKey := bytes.Contains(b, []byte(`"id"`)) - if hasIDKey != wantIDKey { - t.Errorf("got %s, want contain id key: %v", b, wantIDKey) - } - } -} - -func TestResponseUnmarshalJSON_Notif(t *testing.T) { - tests := map[string]bool{ - `{"method":"f","id":0}`: false, - `{"method":"f","id":1}`: false, - `{"method":"f","id":"a"}`: false, - `{"method":"f","id":""}`: false, - `{"method":"f"}`: true, - } - for s, want := range tests { - var r jsonrpc2.Request - if err := json.Unmarshal([]byte(s), &r); err != nil { - t.Fatal(err) - } - if r.Notif != want { - t.Errorf("%s: got %v, want %v", s, r.Notif, want) - } - } -} - // testHandlerA is the "server" handler. type testHandlerA struct{ t *testing.T } @@ -314,84 +253,6 @@ type noopHandler struct{} func (noopHandler) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) {} -func TestConn_DisconnectNotify(t *testing.T) { - - t.Run("EOF", func(t *testing.T) { - connA, connB := net.Pipe() - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) - // By closing connA, connB receives io.EOF - if err := connA.Close(); err != nil { - t.Error(err) - } - assertDisconnect(t, c, connB) - }) - - t.Run("Close", func(t *testing.T) { - _, connB := net.Pipe() - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) - if err := c.Close(); err != nil { - t.Error(err) - } - assertDisconnect(t, c, connB) - }) - - t.Run("Close async", func(t *testing.T) { - done := make(chan struct{}) - _, connB := net.Pipe() - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) - go func() { - if err := c.Close(); err != nil && err != jsonrpc2.ErrClosed { - t.Error(err) - } - close(done) - }() - assertDisconnect(t, c, connB) - <-done - }) - - t.Run("protocol error", func(t *testing.T) { - connA, connB := net.Pipe() - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) - connA.Write([]byte("invalid json")) - assertDisconnect(t, c, connB) - }) -} - -func TestConn_Close(t *testing.T) { - t.Run("waiting for response", func(t *testing.T) { - connA, connB := net.Pipe() - nodeA := jsonrpc2.NewConn( - context.Background(), - jsonrpc2.NewPlainObjectStream(connA), noopHandler{}, - ) - defer nodeA.Close() - nodeB := jsonrpc2.NewConn( - context.Background(), - jsonrpc2.NewPlainObjectStream(connB), - noopHandler{}, - ) - defer nodeB.Close() - - ready := make(chan struct{}) - done := make(chan struct{}) - go func() { - close(ready) - err := nodeB.Call(context.Background(), "m", nil, nil) - if err != jsonrpc2.ErrClosed { - t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) - } - close(done) - }() - // Wait for the request to be sent before we close the connection. - <-ready - if err := nodeB.Close(); err != nil && err != jsonrpc2.ErrClosed { - t.Error(err) - } - assertDisconnect(t, nodeB, connB) - <-done - }) -} - func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMaker streamMaker, opts ...jsonrpc2.ConnOpt) error { for { conn, err := lis.Accept() @@ -401,17 +262,3 @@ func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMake jsonrpc2.NewConn(ctx, streamMaker(conn), h, opts...) } } - -func assertDisconnect(t *testing.T, c *jsonrpc2.Conn, conn io.Writer) { - select { - case <-c.DisconnectNotify(): - case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") - } - // Assert that conn is closed by trying to write to it. - _, got := conn.Write(nil) - want := io.ErrClosedPipe - if got != want { - t.Fatalf("got %q, want %q", got, want) - } -} diff --git a/object_test.go b/object_test.go deleted file mode 100644 index cfa5b00..0000000 --- a/object_test.go +++ /dev/null @@ -1,132 +0,0 @@ -package jsonrpc2 - -import ( - "bytes" - "encoding/json" - "reflect" - "testing" -) - -func TestAnyMessage(t *testing.T) { - tests := map[string]struct { - request, response, invalid bool - }{ - // Single messages - `{}`: {invalid: true}, - `{"foo":"bar"}`: {invalid: true}, - `{"method":"m"}`: {request: true}, - `{"result":123}`: {response: true}, - `{"result":null}`: {response: true}, - `{"error":{"code":456,"message":"m"}}`: {response: true}, - } - for s, want := range tests { - var m anyMessage - if err := json.Unmarshal([]byte(s), &m); err != nil { - if !want.invalid { - t.Errorf("%s: error: %v", s, err) - } - continue - } - if (m.request != nil) != want.request { - t.Errorf("%s: got request %v, want %v", s, m.request != nil, want.request) - } - if (m.response != nil) != want.response { - t.Errorf("%s: got response %v, want %v", s, m.response != nil, want.response) - } - } -} - -func TestRequest_MarshalUnmarshalJSON(t *testing.T) { - null := json.RawMessage("null") - obj := json.RawMessage(`{"foo":"bar"}`) - tests := []struct { - data []byte - want Request - }{ - { - data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":{"foo":"bar"}}`), - want: Request{ID: ID{Num: 123}, Method: "m", Params: &obj}, - }, - { - data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":null}`), - want: Request{ID: ID{Num: 123}, Method: "m", Params: &null}, - }, - { - data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m"}`), - want: Request{ID: ID{Num: 123}, Method: "m", Params: nil}, - }, - { - data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","sessionId":"session"}`), - want: Request{ID: ID{Num: 123}, Method: "m", Params: nil, ExtraFields: []RequestField{{Name: "sessionId", Value: "session"}}}, - }, - } - for _, test := range tests { - var got Request - if err := json.Unmarshal(test.data, &got); err != nil { - t.Error(err) - continue - } - if !reflect.DeepEqual(got, test.want) { - t.Errorf("%q: got %+v, want %+v", test.data, got, test.want) - continue - } - data, err := json.Marshal(got) - if err != nil { - t.Error(err) - continue - } - if !bytes.Equal(data, test.data) { - t.Errorf("got JSON %q, want %q", data, test.data) - } - } -} - -func TestResponse_MarshalUnmarshalJSON(t *testing.T) { - null := json.RawMessage("null") - obj := json.RawMessage(`{"foo":"bar"}`) - tests := []struct { - data []byte - want Response - error bool - }{ - { - data: []byte(`{"id":123,"result":{"foo":"bar"},"jsonrpc":"2.0"}`), - want: Response{ID: ID{Num: 123}, Result: &obj}, - }, - { - data: []byte(`{"id":123,"result":null,"jsonrpc":"2.0"}`), - want: Response{ID: ID{Num: 123}, Result: &null}, - }, - { - data: []byte(`{"id":123,"jsonrpc":"2.0"}`), - want: Response{ID: ID{Num: 123}, Result: nil}, - error: true, // either result or error field must be set - }, - } - for _, test := range tests { - var got Response - if err := json.Unmarshal(test.data, &got); err != nil { - t.Error(err) - continue - } - if !reflect.DeepEqual(got, test.want) { - t.Errorf("%q: got %+v, want %+v", test.data, got, test.want) - continue - } - data, err := json.Marshal(got) - if err != nil { - if test.error { - continue - } - t.Error(err) - continue - } - if test.error { - t.Errorf("%q: expected error", test.data) - continue - } - if !bytes.Equal(data, test.data) { - t.Errorf("got JSON %q, want %q", data, test.data) - } - } -} diff --git a/request.go b/request.go new file mode 100644 index 0000000..666573b --- /dev/null +++ b/request.go @@ -0,0 +1,183 @@ +package jsonrpc2 + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" +) + +// Request represents a JSON-RPC request or +// notification. See +// http://www.jsonrpc.org/specification#request_object and +// http://www.jsonrpc.org/specification#notification. +type Request struct { + Method string `json:"method"` + Params *json.RawMessage `json:"params,omitempty"` + ID ID `json:"id"` + Notif bool `json:"-"` + + // Meta optionally provides metadata to include in the request. + // + // NOTE: It is not part of spec. However, it is useful for propagating + // tracing context, etc. + Meta *json.RawMessage `json:"meta,omitempty"` + + // ExtraFields optionally adds fields to the root of the JSON-RPC request. + // + // NOTE: It is not part of the spec, but there are other protocols based on + // JSON-RPC 2 that require it. + ExtraFields []RequestField `json:"-"` + // OmitNilParams instructs the SetParams method to not JSON encode a nil + // value and set Params to nil instead. + OmitNilParams bool `json:"-"` +} + +// MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" +// property. +func (r Request) MarshalJSON() ([]byte, error) { + r2 := map[string]interface{}{ + "jsonrpc": "2.0", + "method": r.Method, + } + for _, field := range r.ExtraFields { + r2[field.Name] = field.Value + } + if !r.Notif { + r2["id"] = &r.ID + } + if r.Params != nil { + r2["params"] = r.Params + } + if r.Meta != nil { + r2["meta"] = r.Meta + } + return json.Marshal(r2) +} + +// UnmarshalJSON implements json.Unmarshaler. +func (r *Request) UnmarshalJSON(data []byte) error { + r2 := make(map[string]interface{}) + + // Detect if the "params" or "meta" fields are JSON "null" or just not + // present by seeing if the field gets overwritten to nil. + emptyParams := &json.RawMessage{} + r2["params"] = emptyParams + emptyMeta := &json.RawMessage{} + r2["meta"] = emptyMeta + + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.UseNumber() + if err := decoder.Decode(&r2); err != nil { + return err + } + var ok bool + r.Method, ok = r2["method"].(string) + if !ok { + return errors.New("missing method field") + } + switch { + case r2["params"] == nil: + r.Params = &jsonNull + case r2["params"] == emptyParams: + r.Params = nil + default: + b, err := json.Marshal(r2["params"]) + if err != nil { + return fmt.Errorf("failed to marshal params: %w", err) + } + r.Params = (*json.RawMessage)(&b) + } + switch { + case r2["meta"] == nil: + r.Meta = &jsonNull + case r2["meta"] == emptyMeta: + r.Meta = nil + default: + b, err := json.Marshal(r2["meta"]) + if err != nil { + return fmt.Errorf("failed to marshal Meta: %w", err) + } + r.Meta = (*json.RawMessage)(&b) + } + switch rawID := r2["id"].(type) { + case nil: + r.ID = ID{} + r.Notif = true + case string: + r.ID = ID{Str: rawID, IsString: true} + r.Notif = false + case json.Number: + id, err := rawID.Int64() + if err != nil { + return fmt.Errorf("failed to unmarshal ID: %w", err) + } + r.ID = ID{Num: uint64(id)} + r.Notif = false + default: + return fmt.Errorf("unexpected ID type: %T", rawID) + } + + // Clear the extra fields before populating them again. + r.ExtraFields = nil + for name, value := range r2 { + switch name { + case "id", "jsonrpc", "meta", "method", "params": + continue + } + r.ExtraFields = append(r.ExtraFields, RequestField{ + Name: name, + Value: value, + }) + } + return nil +} + +// SetParams sets r.Params to the JSON representation of v. If JSON marshaling +// fails, it returns an error. Beware that the JSON encoding of nil is null. If +// r.OmitNilParams is true and v is nil, then r.Params is set to nil and +// therefore omitted from the JSON-RPC request. +func (r *Request) SetParams(v interface{}) error { + if r.OmitNilParams && v == nil { + r.Params = nil + return nil + } + b, err := json.Marshal(v) + if err != nil { + return err + } + r.Params = (*json.RawMessage)(&b) + return nil +} + +// SetMeta sets r.Meta to the JSON representation of v. If JSON +// marshaling fails, it returns an error. +func (r *Request) SetMeta(v interface{}) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + r.Meta = (*json.RawMessage)(&b) + return nil +} + +// SetExtraField adds an entry to r.ExtraFields, so that it is added to the +// JSON representation of the request, as a way to add arbitrary extensions to +// JSON RPC 2.0. If JSON marshaling fails, it returns an error. +func (r *Request) SetExtraField(name string, v interface{}) error { + switch name { + case "id", "jsonrpc", "meta", "method", "params": + return fmt.Errorf("invalid extra field %q", name) + } + r.ExtraFields = append(r.ExtraFields, RequestField{ + Name: name, + Value: v, + }) + return nil +} + +// RequestField is a top-level field that can be added to the JSON-RPC request. +type RequestField struct { + Name string + Value interface{} +} diff --git a/request_test.go b/request_test.go new file mode 100644 index 0000000..9d6c243 --- /dev/null +++ b/request_test.go @@ -0,0 +1,65 @@ +package jsonrpc2_test + +import ( + "bytes" + "encoding/json" + "reflect" + "testing" + + "github.com/sourcegraph/jsonrpc2" +) + +func TestRequest_MarshalJSON_jsonrpc(t *testing.T) { + b, err := json.Marshal(&jsonrpc2.Request{}) + if err != nil { + t.Fatal(err) + } + if want := `{"id":0,"jsonrpc":"2.0","method":""}`; string(b) != want { + t.Errorf("got %q, want %q", b, want) + } +} + +func TestRequest_MarshalUnmarshalJSON(t *testing.T) { + null := json.RawMessage("null") + obj := json.RawMessage(`{"foo":"bar"}`) + tests := []struct { + data []byte + want jsonrpc2.Request + }{ + { + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":{"foo":"bar"}}`), + want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: &obj}, + }, + { + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":null}`), + want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: &null}, + }, + { + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m"}`), + want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: nil}, + }, + { + data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","sessionId":"session"}`), + want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: nil, ExtraFields: []jsonrpc2.RequestField{{Name: "sessionId", Value: "session"}}}, + }, + } + for _, test := range tests { + var got jsonrpc2.Request + if err := json.Unmarshal(test.data, &got); err != nil { + t.Error(err) + continue + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("%q: got %+v, want %+v", test.data, got, test.want) + continue + } + data, err := json.Marshal(got) + if err != nil { + t.Error(err) + continue + } + if !bytes.Equal(data, test.data) { + t.Errorf("got JSON %q, want %q", data, test.data) + } + } +} diff --git a/response.go b/response.go new file mode 100644 index 0000000..c9a0bfe --- /dev/null +++ b/response.go @@ -0,0 +1,72 @@ +package jsonrpc2 + +import ( + "encoding/json" + "errors" +) + +// Response represents a JSON-RPC response. See +// http://www.jsonrpc.org/specification#response_object. +type Response struct { + ID ID `json:"id"` + Result *json.RawMessage `json:"result,omitempty"` + Error *Error `json:"error,omitempty"` + + // Meta optionally provides metadata to include in the response. + // + // NOTE: It is not part of spec. However, it is useful for propagating + // tracing context, etc. + Meta *json.RawMessage `json:"meta,omitempty"` + + // SPEC NOTE: The spec says "If there was an error in detecting + // the id in the Request object (e.g. Parse error/Invalid + // Request), it MUST be Null." If we made the ID field nullable, + // then we'd have to make it a pointer type. For simplicity, we're + // ignoring the case where there was an error in detecting the ID + // in the Request object. +} + +// MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" +// property. +func (r Response) MarshalJSON() ([]byte, error) { + if (r.Result == nil || len(*r.Result) == 0) && r.Error == nil { + return nil, errors.New("can't marshal *jsonrpc2.Response (must have result or error)") + } + type tmpType Response // avoid infinite MarshalJSON recursion + b, err := json.Marshal(tmpType(r)) + if err != nil { + return nil, err + } + b = append(b[:len(b)-1], []byte(`,"jsonrpc":"2.0"}`)...) + return b, nil +} + +// UnmarshalJSON implements json.Unmarshaler. +func (r *Response) UnmarshalJSON(data []byte) error { + type tmpType Response + + // Detect if the "result" field is JSON "null" or just not present + // by seeing if the field gets overwritten to nil. + *r = Response{Result: &json.RawMessage{}} + + if err := json.Unmarshal(data, (*tmpType)(r)); err != nil { + return err + } + if r.Result == nil { // JSON "null" + r.Result = &jsonNull + } else if len(*r.Result) == 0 { + r.Result = nil + } + return nil +} + +// SetResult sets r.Result to the JSON representation of v. If JSON +// marshaling fails, it returns an error. +func (r *Response) SetResult(v interface{}) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + r.Result = (*json.RawMessage)(&b) + return nil +} diff --git a/response_test.go b/response_test.go new file mode 100644 index 0000000..38eeb52 --- /dev/null +++ b/response_test.go @@ -0,0 +1,110 @@ +package jsonrpc2_test + +import ( + "bytes" + "encoding/json" + "reflect" + "testing" + + "github.com/sourcegraph/jsonrpc2" +) + +func TestResponse_MarshalJSON_jsonrpc(t *testing.T) { + null := json.RawMessage("null") + b, err := json.Marshal(&jsonrpc2.Response{Result: &null}) + if err != nil { + t.Fatal(err) + } + if want := `{"id":0,"result":null,"jsonrpc":"2.0"}`; string(b) != want { + t.Errorf("got %q, want %q", b, want) + } +} + +func TestResponseMarshalJSON_Notif(t *testing.T) { + tests := map[*jsonrpc2.Request]bool{ + {ID: jsonrpc2.ID{Num: 0}}: true, + {ID: jsonrpc2.ID{Num: 1}}: true, + {ID: jsonrpc2.ID{Str: "", IsString: true}}: true, + {ID: jsonrpc2.ID{Str: "a", IsString: true}}: true, + {Notif: true}: false, + } + for r, wantIDKey := range tests { + b, err := json.Marshal(r) + if err != nil { + t.Fatal(err) + } + hasIDKey := bytes.Contains(b, []byte(`"id"`)) + if hasIDKey != wantIDKey { + t.Errorf("got %s, want contain id key: %v", b, wantIDKey) + } + } +} + +func TestResponseUnmarshalJSON_Notif(t *testing.T) { + tests := map[string]bool{ + `{"method":"f","id":0}`: false, + `{"method":"f","id":1}`: false, + `{"method":"f","id":"a"}`: false, + `{"method":"f","id":""}`: false, + `{"method":"f"}`: true, + } + for s, want := range tests { + var r jsonrpc2.Request + if err := json.Unmarshal([]byte(s), &r); err != nil { + t.Fatal(err) + } + if r.Notif != want { + t.Errorf("%s: got %v, want %v", s, r.Notif, want) + } + } +} + +func TestResponse_MarshalUnmarshalJSON(t *testing.T) { + null := json.RawMessage("null") + obj := json.RawMessage(`{"foo":"bar"}`) + tests := []struct { + data []byte + want jsonrpc2.Response + error bool + }{ + { + data: []byte(`{"id":123,"result":{"foo":"bar"},"jsonrpc":"2.0"}`), + want: jsonrpc2.Response{ID: jsonrpc2.ID{Num: 123}, Result: &obj}, + }, + { + data: []byte(`{"id":123,"result":null,"jsonrpc":"2.0"}`), + want: jsonrpc2.Response{ID: jsonrpc2.ID{Num: 123}, Result: &null}, + }, + { + data: []byte(`{"id":123,"jsonrpc":"2.0"}`), + want: jsonrpc2.Response{ID: jsonrpc2.ID{Num: 123}, Result: nil}, + error: true, // either result or error field must be set + }, + } + for _, test := range tests { + var got jsonrpc2.Response + if err := json.Unmarshal(test.data, &got); err != nil { + t.Error(err) + continue + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("%q: got %+v, want %+v", test.data, got, test.want) + continue + } + data, err := json.Marshal(got) + if err != nil { + if test.error { + continue + } + t.Error(err) + continue + } + if test.error { + t.Errorf("%q: expected error", test.data) + continue + } + if !bytes.Equal(data, test.data) { + t.Errorf("got JSON %q, want %q", data, test.data) + } + } +} From 6864d8cc6d35a79f50745f8990cb4d594a8036f4 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Sun, 12 Feb 2023 07:52:17 -0500 Subject: [PATCH 13/29] Omit data field from error when empty (#66) The JSON-RPC 2.0 specification allows the data member of an error object to be omitted. Before this commit, if Error.Data was nil then the JSON encoding was "null". That means that the data member was included in every JSON-RPC error object, even when the data member was not explicitly set. This commit adds the omitempty struct tag to the Error.Data field, meaning that the data member is omitted from the JSON encoding unless explicitly set with the Error.SetError() method. Beware that this is a breaking change for clients that may have strict null checks on the data member. --- jsonrpc2.go | 4 ++-- jsonrpc2_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/jsonrpc2.go b/jsonrpc2.go index 17e1a59..46273bc 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -29,10 +29,10 @@ type JSONRPC2 interface { type Error struct { Code int64 `json:"code"` Message string `json:"message"` - Data *json.RawMessage `json:"data"` + Data *json.RawMessage `json:"data,omitempty"` } -// SetError sets e.Error to the JSON representation of v. If JSON +// SetError sets e.Data to the JSON representation of v. If JSON // marshaling fails, it panics. func (e *Error) SetError(v interface{}) { b, err := json.Marshal(v) diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index 1a73744..dcc2679 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -18,6 +18,65 @@ import ( websocketjsonrpc2 "github.com/sourcegraph/jsonrpc2/websocket" ) +func TestError_MarshalJSON(t *testing.T) { + tests := []struct { + name string + setError func(err *jsonrpc2.Error) + want string + }{ + { + name: "Data == nil", + want: `{"code":-32603,"message":"Internal error"}`, + }, + { + name: "Error.SetError(nil)", + setError: func(err *jsonrpc2.Error) { + err.SetError(nil) + }, + want: `{"code":-32603,"message":"Internal error","data":null}`, + }, + { + name: "Error.SetError(0)", + setError: func(err *jsonrpc2.Error) { + err.SetError(0) + }, + want: `{"code":-32603,"message":"Internal error","data":0}`, + }, + { + name: `Error.SetError("")`, + setError: func(err *jsonrpc2.Error) { + err.SetError("") + }, + want: `{"code":-32603,"message":"Internal error","data":""}`, + }, + { + name: `Error.SetError(false)`, + setError: func(err *jsonrpc2.Error) { + err.SetError(false) + }, + want: `{"code":-32603,"message":"Internal error","data":false}`, + }, + } + + for _, test := range tests { + e := &jsonrpc2.Error{ + Code: jsonrpc2.CodeInternalError, + Message: "Internal error", + } + if test.setError != nil { + test.setError(e) + } + b, err := json.Marshal(e) + if err != nil { + t.Error(err) + } + got := string(b) + if got != test.want { + t.Fatalf("%s: got %q, want %q", test.name, got, test.want) + } + } +} + // testHandlerA is the "server" handler. type testHandlerA struct{ t *testing.T } From ae88a5e7c0fede2c609c9ac56162c73e688f8b3d Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Wed, 22 Feb 2023 03:53:44 -0500 Subject: [PATCH 14/29] Always omit params member from request when empty (#67) With this commit, the JSON encoding of `Request` always omits the params member when calling `Conn.Call`, `Conn.DispatchCall`, or `Conn.Notify` with the `params` argument set to `nil`. This change also removes the `OmitNilParams` call option that was added in commit 8012d496 (#62). As of this commit, if users desire to send a JSON-RPC request with a `params` value of `null`, then they may do so by explicitly setting the `params` argument of `Conn.Call`/`Conn.DispatchCall`/`Conn.Notify` to `json.RawMessage("null")`. --- call_opt.go | 9 ---- call_opt_test.go | 111 ------------------------------------------ conn.go | 43 +++++++++++------ conn_test.go | 123 +++++++++++++++++++++++++++++++++++++++++++++-- example_test.go | 78 ++++++++++++++++++++++++++++++ jsonrpc2.go | 2 +- jsonrpc2_test.go | 7 +++ request.go | 17 ++----- request_test.go | 3 +- response_test.go | 6 +-- 10 files changed, 240 insertions(+), 159 deletions(-) create mode 100644 example_test.go diff --git a/call_opt.go b/call_opt.go index 84a04c2..851baa5 100644 --- a/call_opt.go +++ b/call_opt.go @@ -46,12 +46,3 @@ func StringID() CallOption { return nil }) } - -// OmitNilParams returns a call option that instructs requests to omit params -// values of nil instead of JSON encoding them to null. -func OmitNilParams() CallOption { - return callOptionFunc(func(r *Request) error { - r.OmitNilParams = true - return nil - }) -} diff --git a/call_opt_test.go b/call_opt_test.go index aff8003..b64f661 100644 --- a/call_opt_test.go +++ b/call_opt_test.go @@ -2,10 +2,7 @@ package jsonrpc2_test import ( "context" - "encoding/json" "fmt" - "net" - "sync" "testing" "github.com/sourcegraph/jsonrpc2" @@ -143,111 +140,3 @@ func TestExtraField(t *testing.T) { t.Fatal(err) } } - -func TestOmitNilParams(t *testing.T) { - rawJSONMessage := func(v string) *json.RawMessage { - b := []byte(v) - return (*json.RawMessage)(&b) - } - - type testCase struct { - callOpt jsonrpc2.CallOption - sendParams interface{} - wantParams *json.RawMessage - } - - testCases := []testCase{ - { - sendParams: nil, - wantParams: rawJSONMessage("null"), - }, - { - sendParams: rawJSONMessage("null"), - wantParams: rawJSONMessage("null"), - }, - { - callOpt: jsonrpc2.OmitNilParams(), - sendParams: nil, - wantParams: nil, - }, - { - callOpt: jsonrpc2.OmitNilParams(), - sendParams: rawJSONMessage("null"), - wantParams: rawJSONMessage("null"), - }, - } - - assert := func(got *json.RawMessage, want *json.RawMessage) error { - // Assert pointers. - if got == nil || want == nil { - if got != want { - return fmt.Errorf("got %v, want %v", got, want) - } - return nil - } - { - // If pointers are not nil, then assert values. - got := string(*got) - want := string(*want) - if got != want { - return fmt.Errorf("got %q, want %q", got, want) - } - } - return nil - } - - newClientServer := func(handler jsonrpc2.Handler) (client *jsonrpc2.Conn, server *jsonrpc2.Conn) { - ctx := context.Background() - connA, connB := net.Pipe() - client = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connA), - noopHandler{}, - ) - server = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connB), - handler, - ) - return client, server - } - - for i, tc := range testCases { - - t.Run(fmt.Sprintf("test case %v", i), func(t *testing.T) { - t.Run("call", func(t *testing.T) { - handler := jsonrpc2.HandlerWithError(func(ctx context.Context, c *jsonrpc2.Conn, r *jsonrpc2.Request) (result interface{}, err error) { - return nil, assert(r.Params, tc.wantParams) - }) - - client, server := newClientServer(handler) - defer client.Close() - defer server.Close() - - if err := client.Call(context.Background(), "f", tc.sendParams, nil, tc.callOpt); err != nil { - t.Fatal(err) - } - }) - t.Run("notify", func(t *testing.T) { - wg := &sync.WaitGroup{} - handler := handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) { - err := assert(req.Params, tc.wantParams) - if err != nil { - t.Error(err) - } - wg.Done() - }) - - client, server := newClientServer(handler) - defer client.Close() - defer server.Close() - - wg.Add(1) - if err := client.Notify(context.Background(), "f", tc.sendParams, tc.callOpt); err != nil { - t.Fatal(err) - } - wg.Wait() - }) - }) - } -} diff --git a/conn.go b/conn.go index c2016bf..9de994a 100644 --- a/conn.go +++ b/conn.go @@ -69,10 +69,10 @@ func (c *Conn) Close() error { return c.close(nil) } -// Call initiates a JSON-RPC call using the specified method and -// params, and waits for the response. If the response is successful, -// its result is stored in result (a pointer to a value that can be -// JSON-unmarshaled into); otherwise, a non-nil error is returned. +// Call initiates a JSON-RPC call using the specified method and params, and +// waits for the response. If the response is successful, its result is stored +// in result (a pointer to a value that can be JSON-unmarshaled into); +// otherwise, a non-nil error is returned. See DispatchCall for more details. func (c *Conn) Call(ctx context.Context, method string, params, result interface{}, opts ...CallOption) error { call, err := c.DispatchCall(ctx, method, params, opts...) if err != nil { @@ -87,11 +87,14 @@ func (c *Conn) DisconnectNotify() <-chan struct{} { return c.disconnect } -// DispatchCall dispatches a JSON-RPC call using the specified method -// and params, and returns a call proxy or an error. Call Wait() -// on the returned proxy to receive the response. Only use this -// function if you need to do work after dispatching the request, -// otherwise use Call. +// DispatchCall dispatches a JSON-RPC call using the specified method and +// params, and returns a call proxy or an error. Call Wait() on the returned +// proxy to receive the response. Only use this function if you need to do work +// after dispatching the request, otherwise use Call. +// +// The params member is omitted from the JSON-RPC request if the given params is +// nil. Use json.RawMessage("null") to send a JSON-RPC request with its params +// member set to null. func (c *Conn) DispatchCall(ctx context.Context, method string, params interface{}, opts ...CallOption) (Waiter, error) { req := &Request{Method: method} for _, opt := range opts { @@ -102,8 +105,10 @@ func (c *Conn) DispatchCall(ctx context.Context, method string, params interface return Waiter{}, err } } - if err := req.SetParams(params); err != nil { - return Waiter{}, err + if params != nil { + if err := req.SetParams(params); err != nil { + return Waiter{}, err + } } call, err := c.send(ctx, &anyMessage{request: req}, true) if err != nil { @@ -112,9 +117,13 @@ func (c *Conn) DispatchCall(ctx context.Context, method string, params interface return Waiter{call: call}, nil } -// Notify is like Call, but it returns when the notification request -// is sent (without waiting for a response, because JSON-RPC -// notifications do not have responses). +// Notify is like Call, but it returns when the notification request is sent +// (without waiting for a response, because JSON-RPC notifications do not have +// responses). +// +// The params member is omitted from the JSON-RPC request if the given params is +// nil. Use json.RawMessage("null") to send a JSON-RPC request with its params +// member set to null. func (c *Conn) Notify(ctx context.Context, method string, params interface{}, opts ...CallOption) error { req := &Request{Method: method, Notif: true} for _, opt := range opts { @@ -125,8 +134,10 @@ func (c *Conn) Notify(ctx context.Context, method string, params interface{}, op return err } } - if err := req.SetParams(params); err != nil { - return err + if params != nil { + if err := req.SetParams(params); err != nil { + return err + } } _, err := c.send(ctx, &anyMessage{request: req}, false) return err diff --git a/conn_test.go b/conn_test.go index 773c5cb..56e0350 100644 --- a/conn_test.go +++ b/conn_test.go @@ -2,14 +2,69 @@ package jsonrpc2_test import ( "context" + "encoding/json" + "fmt" "io" + "log" "net" + "sync" "testing" "time" "github.com/sourcegraph/jsonrpc2" ) +var paramsTests = []struct { + sendParams interface{} + wantParams *json.RawMessage +}{ + { + sendParams: nil, + wantParams: nil, + }, + { + sendParams: jsonNull, + wantParams: &jsonNull, + }, + { + sendParams: false, + wantParams: rawJSONMessage("false"), + }, + { + sendParams: 0, + wantParams: rawJSONMessage("0"), + }, + { + sendParams: "", + wantParams: rawJSONMessage(`""`), + }, + { + sendParams: rawJSONMessage(`{"foo":"bar"}`), + wantParams: rawJSONMessage(`{"foo":"bar"}`), + }, +} + +func TestConn_DispatchCall(t *testing.T) { + for _, test := range paramsTests { + t.Run(fmt.Sprintf("%s", test.sendParams), func(t *testing.T) { + testParams(t, test.wantParams, func(c *jsonrpc2.Conn) error { + _, err := c.DispatchCall(context.Background(), "f", test.sendParams) + return err + }) + }) + } +} + +func TestConn_Notify(t *testing.T) { + for _, test := range paramsTests { + t.Run(fmt.Sprintf("%s", test.sendParams), func(t *testing.T) { + testParams(t, test.wantParams, func(c *jsonrpc2.Conn) error { + return c.Notify(context.Background(), "f", test.sendParams) + }) + }) + } +} + func TestConn_DisconnectNotify(t *testing.T) { t.Run("EOF", func(t *testing.T) { @@ -47,7 +102,16 @@ func TestConn_DisconnectNotify(t *testing.T) { t.Run("protocol error", func(t *testing.T) { connA, connB := net.Pipe() - c := jsonrpc2.NewConn(context.Background(), jsonrpc2.NewPlainObjectStream(connB), nil) + c := jsonrpc2.NewConn( + context.Background(), + jsonrpc2.NewPlainObjectStream(connB), + noopHandler{}, + // Suppress log message. This connection receives an invalid JSON + // message that causes an error to be written to the logger. We + // don't want this expected error to appear in os.Stderr though when + // running tests in verbose mode or when other tests fail. + jsonrpc2.SetLogger(log.New(io.Discard, "", 0)), + ) connA.Write([]byte("invalid json")) assertDisconnect(t, c, connB) }) @@ -88,16 +152,69 @@ func TestConn_Close(t *testing.T) { }) } +func testParams(t *testing.T, want *json.RawMessage, fn func(c *jsonrpc2.Conn) error) { + wg := &sync.WaitGroup{} + handler := handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, r *jsonrpc2.Request) { + assertRawJSONMessage(t, r.Params, want) + wg.Done() + }) + + client, server := newClientServer(handler) + defer client.Close() + defer server.Close() + + wg.Add(1) + if err := fn(client); err != nil { + t.Error(err) + } + wg.Wait() +} + func assertDisconnect(t *testing.T, c *jsonrpc2.Conn, conn io.Writer) { select { case <-c.DisconnectNotify(): case <-time.After(200 * time.Millisecond): - t.Fatal("no disconnect notification") + t.Error("no disconnect notification") + return } // Assert that conn is closed by trying to write to it. _, got := conn.Write(nil) want := io.ErrClosedPipe if got != want { - t.Fatalf("got %q, want %q", got, want) + t.Errorf("got %s, want %s", got, want) } } + +func assertRawJSONMessage(t *testing.T, got *json.RawMessage, want *json.RawMessage) { + // Assert pointers. + if got == nil || want == nil { + if got != want { + t.Errorf("pointer: got %s, want %s", got, want) + } + return + } + { + // If pointers are not nil, then assert values. + got := string(*got) + want := string(*want) + if got != want { + t.Errorf("value: got %q, want %q", got, want) + } + } +} + +func newClientServer(handler jsonrpc2.Handler) (client *jsonrpc2.Conn, server *jsonrpc2.Conn) { + ctx := context.Background() + connA, connB := net.Pipe() + client = jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connA), + noopHandler{}, + ) + server = jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connB), + handler, + ) + return client, server +} diff --git a/example_test.go b/example_test.go new file mode 100644 index 0000000..9b2b75a --- /dev/null +++ b/example_test.go @@ -0,0 +1,78 @@ +package jsonrpc2_test + +import ( + "context" + "encoding/json" + "fmt" + "net" + "os" + + "github.com/sourcegraph/jsonrpc2" +) + +// Send a JSON-RPC notification with its params member omitted. +func ExampleConn_Notify_paramsOmitted() { + ctx := context.Background() + + connA, connB := net.Pipe() + defer connA.Close() + defer connB.Close() + + rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) + + // Send the JSON-RPC notification. + go func() { + // Set params to nil. + if err := rpcConn.Notify(ctx, "foo", nil); err != nil { + fmt.Fprintln(os.Stderr, "notify:", err) + } + }() + + // Read the raw JSON-RPC notification on connB. + // + // Reading the raw JSON-RPC request is for the purpose of this example only. + // Use a jsonrpc2.Handler to read parsed requests. + buf := make([]byte, 64) + n, err := connB.Read(buf) + if err != nil { + fmt.Fprintln(os.Stderr, "read:", err) + } + + fmt.Printf("%s\n", buf[:n]) + + // Output: {"jsonrpc":"2.0","method":"foo"} +} + +// Send a JSON-RPC notification with its params member set to null. +func ExampleConn_Notify_nullParams() { + ctx := context.Background() + + connA, connB := net.Pipe() + defer connA.Close() + defer connB.Close() + + rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) + + // Send the JSON-RPC notification. + go func() { + // Set params to the JSON null value. + params := json.RawMessage("null") + if err := rpcConn.Notify(ctx, "foo", params); err != nil { + fmt.Fprintln(os.Stderr, "notify:", err) + } + }() + + // Read the raw JSON-RPC notification on connB. + // + // Reading the raw JSON-RPC request is for the purpose of this example only. + // Use a jsonrpc2.Handler to read parsed requests. + buf := make([]byte, 64) + n, err := connB.Read(buf) + if err != nil { + fmt.Fprintln(os.Stderr, "read:", err) + } + + fmt.Printf("%s\n", buf[:n]) + + // Output: {"jsonrpc":"2.0","method":"foo","params":null} +} diff --git a/jsonrpc2.go b/jsonrpc2.go index 46273bc..97e26d7 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -32,7 +32,7 @@ type Error struct { Data *json.RawMessage `json:"data,omitempty"` } -// SetError sets e.Data to the JSON representation of v. If JSON +// SetError sets e.Data to the JSON encoding of v. If JSON // marshaling fails, it panics. func (e *Error) SetError(v interface{}) { b, err := json.Marshal(v) diff --git a/jsonrpc2_test.go b/jsonrpc2_test.go index dcc2679..8d7968f 100644 --- a/jsonrpc2_test.go +++ b/jsonrpc2_test.go @@ -321,3 +321,10 @@ func serve(ctx context.Context, lis net.Listener, h jsonrpc2.Handler, streamMake jsonrpc2.NewConn(ctx, streamMaker(conn), h, opts...) } } + +func rawJSONMessage(v string) *json.RawMessage { + b := []byte(v) + return (*json.RawMessage)(&b) +} + +var jsonNull = json.RawMessage("null") diff --git a/request.go b/request.go index 666573b..372b3e7 100644 --- a/request.go +++ b/request.go @@ -28,9 +28,6 @@ type Request struct { // NOTE: It is not part of the spec, but there are other protocols based on // JSON-RPC 2 that require it. ExtraFields []RequestField `json:"-"` - // OmitNilParams instructs the SetParams method to not JSON encode a nil - // value and set Params to nil instead. - OmitNilParams bool `json:"-"` } // MarshalJSON implements json.Marshaler and adds the "jsonrpc":"2.0" @@ -133,15 +130,9 @@ func (r *Request) UnmarshalJSON(data []byte) error { return nil } -// SetParams sets r.Params to the JSON representation of v. If JSON marshaling -// fails, it returns an error. Beware that the JSON encoding of nil is null. If -// r.OmitNilParams is true and v is nil, then r.Params is set to nil and -// therefore omitted from the JSON-RPC request. +// SetParams sets r.Params to the JSON encoding of v. If JSON +// marshaling fails, it returns an error. func (r *Request) SetParams(v interface{}) error { - if r.OmitNilParams && v == nil { - r.Params = nil - return nil - } b, err := json.Marshal(v) if err != nil { return err @@ -150,7 +141,7 @@ func (r *Request) SetParams(v interface{}) error { return nil } -// SetMeta sets r.Meta to the JSON representation of v. If JSON +// SetMeta sets r.Meta to the JSON encoding of v. If JSON // marshaling fails, it returns an error. func (r *Request) SetMeta(v interface{}) error { b, err := json.Marshal(v) @@ -162,7 +153,7 @@ func (r *Request) SetMeta(v interface{}) error { } // SetExtraField adds an entry to r.ExtraFields, so that it is added to the -// JSON representation of the request, as a way to add arbitrary extensions to +// JSON encoding of the request, as a way to add arbitrary extensions to // JSON RPC 2.0. If JSON marshaling fails, it returns an error. func (r *Request) SetExtraField(name string, v interface{}) error { switch name { diff --git a/request_test.go b/request_test.go index 9d6c243..0e7a7f4 100644 --- a/request_test.go +++ b/request_test.go @@ -20,7 +20,6 @@ func TestRequest_MarshalJSON_jsonrpc(t *testing.T) { } func TestRequest_MarshalUnmarshalJSON(t *testing.T) { - null := json.RawMessage("null") obj := json.RawMessage(`{"foo":"bar"}`) tests := []struct { data []byte @@ -32,7 +31,7 @@ func TestRequest_MarshalUnmarshalJSON(t *testing.T) { }, { data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m","params":null}`), - want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: &null}, + want: jsonrpc2.Request{ID: jsonrpc2.ID{Num: 123}, Method: "m", Params: &jsonNull}, }, { data: []byte(`{"id":123,"jsonrpc":"2.0","method":"m"}`), diff --git a/response_test.go b/response_test.go index 38eeb52..4819c0e 100644 --- a/response_test.go +++ b/response_test.go @@ -10,8 +10,7 @@ import ( ) func TestResponse_MarshalJSON_jsonrpc(t *testing.T) { - null := json.RawMessage("null") - b, err := json.Marshal(&jsonrpc2.Response{Result: &null}) + b, err := json.Marshal(&jsonrpc2.Response{Result: &jsonNull}) if err != nil { t.Fatal(err) } @@ -60,7 +59,6 @@ func TestResponseUnmarshalJSON_Notif(t *testing.T) { } func TestResponse_MarshalUnmarshalJSON(t *testing.T) { - null := json.RawMessage("null") obj := json.RawMessage(`{"foo":"bar"}`) tests := []struct { data []byte @@ -73,7 +71,7 @@ func TestResponse_MarshalUnmarshalJSON(t *testing.T) { }, { data: []byte(`{"id":123,"result":null,"jsonrpc":"2.0"}`), - want: jsonrpc2.Response{ID: jsonrpc2.ID{Num: 123}, Result: &null}, + want: jsonrpc2.Response{ID: jsonrpc2.ID{Num: 123}, Result: &jsonNull}, }, { data: []byte(`{"id":123,"jsonrpc":"2.0"}`), From 040dc22f8a6046fee3e8b2c3986a23e262c9cf2f Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 28 Feb 2023 23:46:15 -0500 Subject: [PATCH 15/29] Add package example test (#68) --- README.md | 5 +-- example_params_test.go | 78 ++++++++++++++++++++++++++++++++++++ example_test.go | 90 ++++++++++++++++++------------------------ 3 files changed, 118 insertions(+), 55 deletions(-) create mode 100644 example_params_test.go diff --git a/README.md b/README.md index d2406ab..df4f081 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,8 @@ Package jsonrpc2 provides a [Go](https://golang.org) implementation of [JSON-RPC 2.0](http://www.jsonrpc.org/specification). -This package is **experimental** until further notice. - -[**Open the code in Sourcegraph**](https://sourcegraph.com/github.com/sourcegraph/jsonrpc2) +* [Documentation](https://pkg.go.dev/github.com/sourcegraph/jsonrpc2) +* [Open the code in Sourcegraph](https://sourcegraph.com/github.com/sourcegraph/jsonrpc2) ## Known issues diff --git a/example_params_test.go b/example_params_test.go new file mode 100644 index 0000000..9b2b75a --- /dev/null +++ b/example_params_test.go @@ -0,0 +1,78 @@ +package jsonrpc2_test + +import ( + "context" + "encoding/json" + "fmt" + "net" + "os" + + "github.com/sourcegraph/jsonrpc2" +) + +// Send a JSON-RPC notification with its params member omitted. +func ExampleConn_Notify_paramsOmitted() { + ctx := context.Background() + + connA, connB := net.Pipe() + defer connA.Close() + defer connB.Close() + + rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) + + // Send the JSON-RPC notification. + go func() { + // Set params to nil. + if err := rpcConn.Notify(ctx, "foo", nil); err != nil { + fmt.Fprintln(os.Stderr, "notify:", err) + } + }() + + // Read the raw JSON-RPC notification on connB. + // + // Reading the raw JSON-RPC request is for the purpose of this example only. + // Use a jsonrpc2.Handler to read parsed requests. + buf := make([]byte, 64) + n, err := connB.Read(buf) + if err != nil { + fmt.Fprintln(os.Stderr, "read:", err) + } + + fmt.Printf("%s\n", buf[:n]) + + // Output: {"jsonrpc":"2.0","method":"foo"} +} + +// Send a JSON-RPC notification with its params member set to null. +func ExampleConn_Notify_nullParams() { + ctx := context.Background() + + connA, connB := net.Pipe() + defer connA.Close() + defer connB.Close() + + rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) + + // Send the JSON-RPC notification. + go func() { + // Set params to the JSON null value. + params := json.RawMessage("null") + if err := rpcConn.Notify(ctx, "foo", params); err != nil { + fmt.Fprintln(os.Stderr, "notify:", err) + } + }() + + // Read the raw JSON-RPC notification on connB. + // + // Reading the raw JSON-RPC request is for the purpose of this example only. + // Use a jsonrpc2.Handler to read parsed requests. + buf := make([]byte, 64) + n, err := connB.Read(buf) + if err != nil { + fmt.Fprintln(os.Stderr, "read:", err) + } + + fmt.Printf("%s\n", buf[:n]) + + // Output: {"jsonrpc":"2.0","method":"foo","params":null} +} diff --git a/example_test.go b/example_test.go index 9b2b75a..00f4a57 100644 --- a/example_test.go +++ b/example_test.go @@ -2,77 +2,63 @@ package jsonrpc2_test import ( "context" - "encoding/json" "fmt" + "log" "net" "os" "github.com/sourcegraph/jsonrpc2" ) -// Send a JSON-RPC notification with its params member omitted. -func ExampleConn_Notify_paramsOmitted() { +func Example() { ctx := context.Background() + // Create an in-memory network connection. This connection is used below to + // transport the JSON-RPC messages. However, any io.ReadWriteCloser may be + // used to send/receive JSON-RPC messages. connA, connB := net.Pipe() - defer connA.Close() - defer connB.Close() - rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) + // The following JSON-RPC connection is both a client and a server. It can + // send requests as well as receive requests. The incoming requests are + // handled by myHandler. + jsonrpcConnA := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), &myHandler{}) + defer jsonrpcConnA.Close() - // Send the JSON-RPC notification. - go func() { - // Set params to nil. - if err := rpcConn.Notify(ctx, "foo", nil); err != nil { - fmt.Fprintln(os.Stderr, "notify:", err) - } - }() + // The following JSON-RPC connection has no handler, meaning that it is + // configured to only be a client. It can send requests and receive the + // responses to those requests, but it will ignore any incoming requests. + jsonrpcConnB := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connB), nil) + defer jsonrpcConnB.Close() - // Read the raw JSON-RPC notification on connB. - // - // Reading the raw JSON-RPC request is for the purpose of this example only. - // Use a jsonrpc2.Handler to read parsed requests. - buf := make([]byte, 64) - n, err := connB.Read(buf) - if err != nil { - fmt.Fprintln(os.Stderr, "read:", err) + // Send a request from jsonrpcConnB to jsonrpcConnA. The result of a + // successful call is stored in the result variable. + var result string + if err := jsonrpcConnB.Call(ctx, "sayHello", nil, &result); err != nil { + fmt.Fprintln(os.Stderr, err) + return } - fmt.Printf("%s\n", buf[:n]) + fmt.Println(result) - // Output: {"jsonrpc":"2.0","method":"foo"} + // Output: hello world } -// Send a JSON-RPC notification with its params member set to null. -func ExampleConn_Notify_nullParams() { - ctx := context.Background() +// myHandler is the jsonrpc2.Handler used by jsonrpcConnA. +type myHandler struct{} - connA, connB := net.Pipe() - defer connA.Close() - defer connB.Close() - - rpcConn := jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(connA), nil) - - // Send the JSON-RPC notification. - go func() { - // Set params to the JSON null value. - params := json.RawMessage("null") - if err := rpcConn.Notify(ctx, "foo", params); err != nil { - fmt.Fprintln(os.Stderr, "notify:", err) +// Handle implements the jsonrpc2.Handler interface. +func (h *myHandler) Handle(ctx context.Context, c *jsonrpc2.Conn, r *jsonrpc2.Request) { + switch r.Method { + case "sayHello": + if err := c.Reply(ctx, r.ID, "hello world"); err != nil { + log.Println(err) + return + } + default: + err := &jsonrpc2.Error{Code: jsonrpc2.CodeMethodNotFound, Message: "Method not found"} + if err := c.ReplyWithError(ctx, r.ID, err); err != nil { + log.Println(err) + return } - }() - - // Read the raw JSON-RPC notification on connB. - // - // Reading the raw JSON-RPC request is for the purpose of this example only. - // Use a jsonrpc2.Handler to read parsed requests. - buf := make([]byte, 64) - n, err := connB.Read(buf) - if err != nil { - fmt.Fprintln(os.Stderr, "read:", err) } - - fmt.Printf("%s\n", buf[:n]) - - // Output: {"jsonrpc":"2.0","method":"foo","params":null} } From 5d80b29f441bbb48e87bca31a6c9ecf3f92449a3 Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Wed, 7 Jun 2023 08:40:20 +0200 Subject: [PATCH 16/29] conn: do not lock sending when closing (#70) The sending mutex may be blocked due to the underlying conn blocking. If that happens then we can't call close since close also attempts to hold the sending mutex. Sending mutex is only used for serializing sends and doesn't protect the fields close reads/writes. I believe we introduced locking it so we would return ErrClosed. This commit instead introduces a check in send which rechecks if we have since closed while attempting to send. Test Plan: expanded the test coverage --- conn.go | 13 ++++++- conn_test.go | 101 +++++++++++++++++++++++++++++++++++---------------- 2 files changed, 81 insertions(+), 33 deletions(-) diff --git a/conn.go b/conn.go index 9de994a..8ce19d1 100644 --- a/conn.go +++ b/conn.go @@ -166,9 +166,7 @@ func (c *Conn) SendResponse(ctx context.Context, resp *Response) error { } func (c *Conn) close(cause error) error { - c.sending.Lock() c.mu.Lock() - defer c.sending.Unlock() defer c.mu.Unlock() if c.closed { @@ -249,6 +247,17 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err c.sending.Lock() defer c.sending.Unlock() + // double check the error isn't due to being closed while sending. + defer func() { + if err != nil { + c.mu.Lock() + if c.closed { + err = ErrClosed + } + c.mu.Unlock() + } + }() + // m.request.ID could be changed, so we store a copy to correctly // clean up pending var id ID diff --git a/conn_test.go b/conn_test.go index 56e0350..aa32fd9 100644 --- a/conn_test.go +++ b/conn_test.go @@ -118,38 +118,77 @@ func TestConn_DisconnectNotify(t *testing.T) { } func TestConn_Close(t *testing.T) { - t.Run("waiting for response", func(t *testing.T) { - connA, connB := net.Pipe() - nodeA := jsonrpc2.NewConn( - context.Background(), - jsonrpc2.NewPlainObjectStream(connA), noopHandler{}, - ) - defer nodeA.Close() - nodeB := jsonrpc2.NewConn( - context.Background(), - jsonrpc2.NewPlainObjectStream(connB), - noopHandler{}, - ) - defer nodeB.Close() - - ready := make(chan struct{}) - done := make(chan struct{}) - go func() { - close(ready) - err := nodeB.Call(context.Background(), "m", nil, nil) - if err != jsonrpc2.ErrClosed { - t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) + cases := []struct { + name string + run func(*testing.T, context.Context, *jsonrpc2.Conn) + }{{ + name: "during Call", + run: func(t *testing.T, ctx context.Context, conn *jsonrpc2.Conn) { + ready := make(chan struct{}) + done := make(chan struct{}) + go func() { + close(ready) + err := conn.Call(ctx, "m", nil, nil) + if err != jsonrpc2.ErrClosed { + t.Errorf("got error %v, want %v", err, jsonrpc2.ErrClosed) + } + close(done) + }() + // Wait for the request to be sent before we close the connection. + <-ready + if err := conn.Close(); err != nil && err != jsonrpc2.ErrClosed { + t.Error(err) } - close(done) - }() - // Wait for the request to be sent before we close the connection. - <-ready - if err := nodeB.Close(); err != nil && err != jsonrpc2.ErrClosed { - t.Error(err) - } - assertDisconnect(t, nodeB, connB) - <-done - }) + <-done + }, + }, { + name: "during Wait", + run: func(t *testing.T, ctx context.Context, conn *jsonrpc2.Conn) { + call, err := conn.DispatchCall(ctx, "m", nil, nil) + if err != nil { + t.Fatal(err) + } + if err := conn.Close(); err != nil { + t.Fatal(err) + } + if err := call.Wait(ctx, nil); err != jsonrpc2.ErrClosed { + t.Fatal(err) + } + }, + }, { + name: "during Dispatch", + run: func(t *testing.T, ctx context.Context, conn *jsonrpc2.Conn) { + if err := conn.Close(); err != nil { + t.Fatal(err) + } + if _, err := conn.DispatchCall(ctx, "m", nil, nil); err != jsonrpc2.ErrClosed { + t.Fatal(err) + } + }, + }} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + connA, connB := net.Pipe() + nodeA := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connA), noopHandler{}, + ) + defer nodeA.Close() + nodeB := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewPlainObjectStream(connB), + noopHandler{}, + ) + defer nodeB.Close() + + tc.run(t, ctx, nodeB) + + assertDisconnect(t, nodeB, connB) + }) + } } func testParams(t *testing.T, want *json.RawMessage, fn func(c *jsonrpc2.Conn) error) { From b9c1fbdb96da9d995ccc3b74609431df56d4c011 Mon Sep 17 00:00:00 2001 From: Fazlul Shahriar Date: Fri, 14 Jul 2023 07:00:57 -0400 Subject: [PATCH 17/29] Fix logging of received response messages (#71) As documented: OnRecv causes all requests received on conn to invoke f(req, nil) and all responses to invoke f(req, resp). Since OnRecv is called with both *Request and *Response being non-nil when we're handling a response, we need to check that *Response is non-nil before we check *Request is non-nil. This change just swaps the two cases in the switch statement to fix the issue. For consistency, I've swapped the cases for OnSend also, even when it's not needed. --- conn_opt.go | 40 ++++++++++++------------- conn_opt_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/conn_opt.go b/conn_opt.go index 423cf80..a83ccc3 100644 --- a/conn_opt.go +++ b/conn_opt.go @@ -43,18 +43,6 @@ func LogMessages(logger Logger) ConnOpt { OnRecv(func(req *Request, resp *Response) { switch { - case req != nil: - mu.Lock() - reqMethods[req.ID] = req.Method - mu.Unlock() - - params, _ := json.Marshal(req.Params) - if req.Notif { - logger.Printf("jsonrpc2: --> notif: %s: %s\n", req.Method, params) - } else { - logger.Printf("jsonrpc2: --> request #%s: %s: %s\n", req.ID, req.Method, params) - } - case resp != nil: var method string if req != nil { @@ -70,18 +58,22 @@ func LogMessages(logger Logger) ConnOpt { err, _ := json.Marshal(resp.Error) logger.Printf("jsonrpc2: --> error #%s: %s: %s\n", resp.ID, method, err) } + + case req != nil: + mu.Lock() + reqMethods[req.ID] = req.Method + mu.Unlock() + + params, _ := json.Marshal(req.Params) + if req.Notif { + logger.Printf("jsonrpc2: --> notif: %s: %s\n", req.Method, params) + } else { + logger.Printf("jsonrpc2: --> request #%s: %s: %s\n", req.ID, req.Method, params) + } } })(c) OnSend(func(req *Request, resp *Response) { switch { - case req != nil: - params, _ := json.Marshal(req.Params) - if req.Notif { - logger.Printf("jsonrpc2: <-- notif: %s: %s\n", req.Method, params) - } else { - logger.Printf("jsonrpc2: <-- request #%s: %s: %s\n", req.ID, req.Method, params) - } - case resp != nil: mu.Lock() method := reqMethods[resp.ID] @@ -98,6 +90,14 @@ func LogMessages(logger Logger) ConnOpt { err, _ := json.Marshal(resp.Error) logger.Printf("jsonrpc2: <-- error #%s: %s: %s\n", resp.ID, method, err) } + + case req != nil: + params, _ := json.Marshal(req.Params) + if req.Notif { + logger.Printf("jsonrpc2: <-- notif: %s: %s\n", req.Method, params) + } else { + logger.Printf("jsonrpc2: <-- request #%s: %s: %s\n", req.ID, req.Method, params) + } } })(c) } diff --git a/conn_opt_test.go b/conn_opt_test.go index df53a1a..97f59e4 100644 --- a/conn_opt_test.go +++ b/conn_opt_test.go @@ -51,3 +51,80 @@ func TestSetLogger(t *testing.T) { t.Fatalf("got %q, want %q", got, want) } } + +type dummyHandler struct { + t *testing.T +} + +func (h *dummyHandler) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) { + if !req.Notif { + err := conn.Reply(ctx, req.ID, nil) + if err != nil { + h.t.Error(err) + return + } + } +} + +func TestLogMessages(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + rd, wr := io.Pipe() + defer rd.Close() + defer wr.Close() + + buf := bufio.NewReader(rd) + logger := log.New(wr, "", log.Lmsgprefix) + + a, b := net.Pipe() + connA := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewBufferedStream(a, jsonrpc2.VSCodeObjectCodec{}), + &dummyHandler{t}, + jsonrpc2.LogMessages(logger), + ) + connB := jsonrpc2.NewConn( + ctx, + jsonrpc2.NewBufferedStream(b, jsonrpc2.VSCodeObjectCodec{}), + &dummyHandler{t}, + ) + defer connA.Close() + defer connB.Close() + + go func() { + if err := connA.Call(ctx, "method1", nil, nil); err != nil { + t.Error(err) + return + } + if err := connB.Call(ctx, "method2", nil, nil); err != nil { + t.Error(err) + return + } + if err := connA.Notify(ctx, "notification1", nil); err != nil { + t.Error(err) + return + } + if err := connB.Notify(ctx, "notification2", nil); err != nil { + t.Error(err) + return + } + }() + + for i, want := range []string{ + "jsonrpc2: <-- request #0: method1: null\n", + "jsonrpc2: --> result #0: method1: null\n", + "jsonrpc2: --> request #0: method2: null\n", + "jsonrpc2: <-- result #0: method2: null\n", + "jsonrpc2: <-- notif: notification1: null\n", + "jsonrpc2: --> notif: notification2: null\n", + } { + got, err := buf.ReadString('\n') + if err != nil { + t.Fatal(err) + } + if got != want { + t.Errorf("message %v: got %q, want %q", i, got, want) + } + } +} From 8a0bf06edfb027f0c070c1f6e6515aaf295a051f Mon Sep 17 00:00:00 2001 From: Diogo Teles Sant'Anna Date: Wed, 30 Aug 2023 04:07:57 -0300 Subject: [PATCH 18/29] ci: set minimal permissions to GitHub workflows (#73) Signed-off-by: Diogo Teles Sant'Anna --- .github/workflows/ci.yml | 4 ++++ .github/workflows/lsif.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9bf8e75..a80e98a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,10 @@ on: push: branches: - master + +permissions: + contents: read + jobs: test: strategy: diff --git a/.github/workflows/lsif.yml b/.github/workflows/lsif.yml index 83d4bfd..fbb6b0a 100644 --- a/.github/workflows/lsif.yml +++ b/.github/workflows/lsif.yml @@ -1,6 +1,10 @@ name: LSIF on: - push + +permissions: + contents: read + jobs: lsif-go: runs-on: ubuntu-latest From 510183e8821c2955c6ec2a331254fc652e125d30 Mon Sep 17 00:00:00 2001 From: Diogo Teles Sant'Anna Date: Mon, 11 Sep 2023 14:51:22 -0300 Subject: [PATCH 19/29] Create Security Policy --- SECURITY.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..db3af96 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,13 @@ +# Security Policy + +## Supported Versions + +Security updates are applied only to the latest release. + +## Reporting a Vulnerability + +If you have discovered a security vulnerability in this project, please report it privately. **Do not disclose it as a public issue.** This gives us time to work with you to evaluate and fix the issue before public exposure, reducing the chance that the exploit will be used before a patch is released. + +Please disclose it at our [security advisory](https://github.com/sourcegraph/jsonrpc2/security/advisories/new). + +This project is maintained by a team of volunteers on a reasonable-effort basis. As such, vulnerabilities will be handled and/or disclosed in a best effort base. From e4e2e6324cc227e225424248104c7a63da107647 Mon Sep 17 00:00:00 2001 From: Will Dollman Date: Mon, 25 Sep 2023 11:50:00 +0100 Subject: [PATCH 20/29] Update security policy to use email for reporting --- SECURITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index db3af96..60be4f2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -8,6 +8,6 @@ Security updates are applied only to the latest release. If you have discovered a security vulnerability in this project, please report it privately. **Do not disclose it as a public issue.** This gives us time to work with you to evaluate and fix the issue before public exposure, reducing the chance that the exploit will be used before a patch is released. -Please disclose it at our [security advisory](https://github.com/sourcegraph/jsonrpc2/security/advisories/new). +Please disclose it privately via email to security@sourcegraph.com. We will work with you to understand and resolve the issue promptly. This project is maintained by a team of volunteers on a reasonable-effort basis. As such, vulnerabilities will be handled and/or disclosed in a best effort base. From 943e53c8e9bdcbf503287c020a077fc38ccfde5e Mon Sep 17 00:00:00 2001 From: Will Dollman Date: Mon, 25 Sep 2023 11:53:41 +0100 Subject: [PATCH 21/29] Update SECURITY.md Co-authored-by: Vincent --- SECURITY.md | 1 - 1 file changed, 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index 60be4f2..d889a1f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -10,4 +10,3 @@ If you have discovered a security vulnerability in this project, please report i Please disclose it privately via email to security@sourcegraph.com. We will work with you to understand and resolve the issue promptly. -This project is maintained by a team of volunteers on a reasonable-effort basis. As such, vulnerabilities will be handled and/or disclosed in a best effort base. From bf47ec21a6c61c6c2db9c33ee92d3bef8ca18ba1 Mon Sep 17 00:00:00 2001 From: Joyce Date: Wed, 10 Jan 2024 07:06:38 -0300 Subject: [PATCH 22/29] ci: Create dependabot.yml (#78) Signed-off-by: Joyce --- .github/dependabot.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..2390d8c --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,10 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "monthly" + groups: + github-actions: + patterns: + - "*" From dd69e185faab6cc9b302b1f991b4a2c47e2cf191 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:16:40 +0200 Subject: [PATCH 23/29] Bump the github-actions group with 2 updates (#79) Updates `actions/checkout` from 1 to 4 Updates `actions/setup-go` from 2 to 5 Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- .github/workflows/lsif.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a80e98a..633144a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,9 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} id: go diff --git a/.github/workflows/lsif.yml b/.github/workflows/lsif.yml index fbb6b0a..f1a5b1d 100644 --- a/.github/workflows/lsif.yml +++ b/.github/workflows/lsif.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest container: sourcegraph/lsif-go steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v4 - name: Generate LSIF data run: lsif-go - name: Upload LSIF data From 4963d1c241e116cdbdf3ea0c53dfdfb5b119045e Mon Sep 17 00:00:00 2001 From: Noah S-C Date: Fri, 23 Feb 2024 15:34:49 +0000 Subject: [PATCH 24/29] chore: use scip-go instead of lsif-go for precise indexing in CI --- .github/workflows/lsif.yml | 17 ----------------- .github/workflows/scip.yml | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 17 deletions(-) delete mode 100644 .github/workflows/lsif.yml create mode 100644 .github/workflows/scip.yml diff --git a/.github/workflows/lsif.yml b/.github/workflows/lsif.yml deleted file mode 100644 index f1a5b1d..0000000 --- a/.github/workflows/lsif.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: LSIF -on: - - push - -permissions: - contents: read - -jobs: - lsif-go: - runs-on: ubuntu-latest - container: sourcegraph/lsif-go - steps: - - uses: actions/checkout@v4 - - name: Generate LSIF data - run: lsif-go - - name: Upload LSIF data - run: src lsif upload -github-token=${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml new file mode 100644 index 0000000..24a4a0e --- /dev/null +++ b/.github/workflows/scip.yml @@ -0,0 +1,20 @@ +name: SCIP +'on': + - push +permissions: + contents: read +jobs: + scip-go: + runs-on: ubuntu-latest + container: sourcegraph/scip-go + steps: + - uses: actions/checkout@v4 + - name: Get src-cli + run: curl -L https://sourcegraph.com/.api/src-cli/src_linux_amd64 -o /usr/local/bin/src; + chmod +x /usr/local/bin/src + - name: Set directory to safe for git + run: git config --global --add safe.directory $GITHUB_WORKSPACE + - name: Generate SCIP data + run: scip-go + - name: Upload SCIP data + run: src code-intel upload -github-token=${{ secrets.GITHUB_TOKEN }} From 2cc94179e136b565cb9df7d9d0bff9fc06912ac9 Mon Sep 17 00:00:00 2001 From: Kevin Gillette Date: Mon, 17 Feb 2025 07:55:54 -0700 Subject: [PATCH 25/29] transparently simplify control flow (#83) --- conn.go | 105 ++++++++++++++++++------------------------ conn_opt.go | 4 +- handler_with_error.go | 20 ++++---- request.go | 32 +++++++------ 4 files changed, 72 insertions(+), 89 deletions(-) diff --git a/conn.go b/conn.go index 8ce19d1..18466cb 100644 --- a/conn.go +++ b/conn.go @@ -187,15 +187,17 @@ func (c *Conn) close(cause error) error { } func (c *Conn) readMessages(ctx context.Context) { - var err error - for err == nil { + for { var m anyMessage - err = c.stream.ReadObject(&m) + err := c.stream.ReadObject(&m) if err != nil { - break + c.close(err) + return } switch { + // TODO: handle the case where both request and response are nil. + case m.request != nil: for _, onRecv := range c.onRecv { onRecv(m.request, nil) @@ -204,43 +206,36 @@ func (c *Conn) readMessages(ctx context.Context) { case m.response != nil: resp := m.response - if resp != nil { - id := resp.ID - c.mu.Lock() - call := c.pending[id] - delete(c.pending, id) - c.mu.Unlock() + id := resp.ID + c.mu.Lock() + call := c.pending[id] + delete(c.pending, id) + c.mu.Unlock() - if call != nil { - call.response = resp - } - - if len(c.onRecv) > 0 { - var req *Request - if call != nil { - req = call.request - } - for _, onRecv := range c.onRecv { - onRecv(req, resp) - } - } - - switch { - case call == nil: - c.logger.Printf("jsonrpc2: ignoring response #%s with no corresponding request\n", id) - - case resp.Error != nil: - call.done <- resp.Error - close(call.done) - - default: - call.done <- nil - close(call.done) - } + var req *Request + if call != nil { + call.response = resp + req = call.request } + + for _, onRecv := range c.onRecv { + onRecv(req, resp) + } + + if call == nil { + c.logger.Printf("jsonrpc2: ignoring response #%s with no corresponding request\n", id) + continue + } + + var err error + if resp.Error != nil { + err = resp.Error + } + + call.done <- err + close(call.done) } } - c.close(err) } func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err error) { @@ -339,25 +334,20 @@ type Waiter struct { // error is returned. func (w Waiter) Wait(ctx context.Context, result interface{}) error { select { - case err, ok := <-w.call.done: - if !ok { - err = ErrClosed - } - if err != nil { - return err - } - if result != nil { - if w.call.response.Result == nil { - w.call.response.Result = &jsonNull - } - if err := json.Unmarshal(*w.call.response.Result, result); err != nil { - return err - } - } - return nil - case <-ctx.Done(): return ctx.Err() + + case err, ok := <-w.call.done: + if !ok { + return ErrClosed + } + if err != nil || result == nil { + return err + } + if w.call.response.Result == nil { + w.call.response.Result = &jsonNull + } + return json.Unmarshal(*w.call.response.Result, result) } } @@ -423,12 +413,7 @@ func (m *anyMessage) UnmarshalJSON(data []byte) error { return errors.New("jsonrpc2: invalid empty batch") } for i := range msgs { - if err := checkType(&msg{ - ID: msgs[i].ID, - Method: msgs[i].Method, - Result: msgs[i].Result, - Error: msgs[i].Error, - }); err != nil { + if err := checkType(&msgs[i]); err != nil { return err } } diff --git a/conn_opt.go b/conn_opt.go index a83ccc3..8a29f80 100644 --- a/conn_opt.go +++ b/conn_opt.go @@ -44,11 +44,9 @@ func LogMessages(logger Logger) ConnOpt { OnRecv(func(req *Request, resp *Response) { switch { case resp != nil: - var method string + method := "(no matching request)" if req != nil { method = req.Method - } else { - method = "(no matching request)" } switch { case resp.Result != nil: diff --git a/handler_with_error.go b/handler_with_error.go index 2bd5c1d..d727237 100644 --- a/handler_with_error.go +++ b/handler_with_error.go @@ -30,20 +30,16 @@ func (h *HandlerWithErrorConfigurer) Handle(ctx context.Context, conn *Conn, req if err == nil { err = resp.SetResult(result) } - if err != nil { - if e, ok := err.(*Error); ok { - resp.Error = e - } else { - resp.Error = &Error{Message: err.Error()} - } + + if e, ok := err.(*Error); ok { + resp.Error = e + } else if err != nil { + resp.Error = &Error{Message: err.Error()} } - if !req.Notif { - if err := conn.SendResponse(ctx, resp); err != nil { - if err != ErrClosed || !h.suppressErrClosed { - conn.logger.Printf("jsonrpc2 handler: sending response %s: %v\n", resp.ID, err) - } - } + err = conn.SendResponse(ctx, resp) + if err != nil && (err != ErrClosed || !h.suppressErrClosed) { + conn.logger.Printf("jsonrpc2 handler: sending response %s: %v\n", resp.ID, err) } } diff --git a/request.go b/request.go index 372b3e7..b9cdde0 100644 --- a/request.go +++ b/request.go @@ -55,6 +55,10 @@ func (r Request) MarshalJSON() ([]byte, error) { // UnmarshalJSON implements json.Unmarshaler. func (r *Request) UnmarshalJSON(data []byte) error { r2 := make(map[string]interface{}) + pop := func(key string) interface{} { + defer delete(r2, key) + return r2[key] + } // Detect if the "params" or "meta" fields are JSON "null" or just not // present by seeing if the field gets overwritten to nil. @@ -68,36 +72,37 @@ func (r *Request) UnmarshalJSON(data []byte) error { if err := decoder.Decode(&r2); err != nil { return err } + var ok bool - r.Method, ok = r2["method"].(string) + r.Method, ok = pop("method").(string) if !ok { return errors.New("missing method field") } - switch { - case r2["params"] == nil: + switch params := pop("params"); params { + case nil: r.Params = &jsonNull - case r2["params"] == emptyParams: + case emptyParams: r.Params = nil default: - b, err := json.Marshal(r2["params"]) + b, err := json.Marshal(params) if err != nil { return fmt.Errorf("failed to marshal params: %w", err) } r.Params = (*json.RawMessage)(&b) } - switch { - case r2["meta"] == nil: + switch meta := pop("meta"); meta { + case nil: r.Meta = &jsonNull - case r2["meta"] == emptyMeta: + case emptyMeta: r.Meta = nil default: - b, err := json.Marshal(r2["meta"]) + b, err := json.Marshal(meta) if err != nil { return fmt.Errorf("failed to marshal Meta: %w", err) } r.Meta = (*json.RawMessage)(&b) } - switch rawID := r2["id"].(type) { + switch rawID := pop("id").(type) { case nil: r.ID = ID{} r.Notif = true @@ -115,13 +120,12 @@ func (r *Request) UnmarshalJSON(data []byte) error { return fmt.Errorf("unexpected ID type: %T", rawID) } + // The jsonrpc field should not be added to ExtraFields. + delete(r2, "jsonrpc") + // Clear the extra fields before populating them again. r.ExtraFields = nil for name, value := range r2 { - switch name { - case "id", "jsonrpc", "meta", "method", "params": - continue - } r.ExtraFields = append(r.ExtraFields, RequestField{ Name: name, Value: value, From ddb146fd0d34a371871ddf7e697f31507a23c3b6 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 19 Aug 2025 10:19:52 -0400 Subject: [PATCH 26/29] Cancel Handler context when connection closes (#90) --- conn.go | 18 ++++++++++-- conn_test.go | 81 ++++++++++++++++++++++++++++++++++++++++------------ jsonrpc2.go | 8 +++--- 3 files changed, 83 insertions(+), 24 deletions(-) diff --git a/conn.go b/conn.go index 18466cb..35c6279 100644 --- a/conn.go +++ b/conn.go @@ -27,6 +27,7 @@ type Conn struct { sending sync.Mutex + cancelCtx context.CancelFunc disconnect chan struct{} logger Logger @@ -43,13 +44,19 @@ var _ JSONRPC2 = (*Conn)(nil) // JSON-RPC protocol is symmetric, so a Conn runs on both ends of a // client-server connection. // -// NewClient consumes conn, so you should call Close on the returned -// client not on the given conn. +// NewConn consumes stream, so you should call Close on the returned +// Conn not on the given stream or its underlying connection. +// +// Conn is closed when the given context's Done channel is closed. func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOpt) *Conn { + + ctx, cancel := context.WithCancel(ctx) + c := &Conn{ stream: stream, h: h, pending: map[ID]*call{}, + cancelCtx: cancel, disconnect: make(chan struct{}), logger: log.New(os.Stderr, "", log.LstdFlags), } @@ -60,6 +67,12 @@ func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOp opt(c) } go c.readMessages(ctx) + + go func() { + <-ctx.Done() + c.close(nil) + }() + return c } @@ -182,6 +195,7 @@ func (c *Conn) close(cause error) error { } close(c.disconnect) + c.cancelCtx() c.closed = true return c.stream.Close() } diff --git a/conn_test.go b/conn_test.go index aa32fd9..5d2a7e4 100644 --- a/conn_test.go +++ b/conn_test.go @@ -14,6 +14,58 @@ import ( "github.com/sourcegraph/jsonrpc2" ) +func TestConn(t *testing.T) { + + t.Run("closes when context is done", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + connA, connB := Pipe(ctx, noopHandler{}, noopHandler{}) + defer connA.Close() + defer connB.Close() + + cancel() + <-connA.DisconnectNotify() + + got := connA.Close() + want := jsonrpc2.ErrClosed + if got != want { + t.Fatalf("got %v, want %v", got, want) + } + }) + + t.Run("cancels context when closed", func(t *testing.T) { + ctxCanceled := make(chan struct{}) + + handler := handlerFunc(func(ctx context.Context, c *jsonrpc2.Conn, r *jsonrpc2.Request) { + // Block until the context is canceled. + <-ctx.Done() + close(ctxCanceled) + }) + + connA, connB := Pipe(context.Background(), noopHandler{}, jsonrpc2.AsyncHandler(handler)) + defer connA.Close() + defer connB.Close() + + // Send a notification from connA to connB to trigger connB's handler + // function. + if err := connA.Notify(context.Background(), "foo", nil, nil); err != nil { + t.Fatal(err) + } + + // Disconnect connA from connB. + if err := connA.Close(); err != nil { + t.Fatal(err) + } + + select { + case <-ctxCanceled: + // Test passed, the handler's context was canceled. + case <-time.After(time.Second): + t.Fatal("context not canceled") + } + }) +} + var paramsTests = []struct { sendParams interface{} wantParams *json.RawMessage @@ -198,12 +250,12 @@ func testParams(t *testing.T, want *json.RawMessage, fn func(c *jsonrpc2.Conn) e wg.Done() }) - client, server := newClientServer(handler) - defer client.Close() - defer server.Close() + connA, connB := Pipe(context.Background(), noopHandler{}, handler) + defer connA.Close() + defer connB.Close() wg.Add(1) - if err := fn(client); err != nil { + if err := fn(connA); err != nil { t.Error(err) } wg.Wait() @@ -242,18 +294,11 @@ func assertRawJSONMessage(t *testing.T, got *json.RawMessage, want *json.RawMess } } -func newClientServer(handler jsonrpc2.Handler) (client *jsonrpc2.Conn, server *jsonrpc2.Conn) { - ctx := context.Background() - connA, connB := net.Pipe() - client = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connA), - noopHandler{}, - ) - server = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connB), - handler, - ) - return client, server +// Pipe returns two jsonrpc2.Conn, connected via a synchronous, in-memory, full +// duplex network connection. +func Pipe(ctx context.Context, handlerA, handlerB jsonrpc2.Handler) (connA *jsonrpc2.Conn, connB *jsonrpc2.Conn) { + a, b := net.Pipe() + connA = jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(a), handlerA) + connB = jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(b), handlerB) + return connA, connB } diff --git a/jsonrpc2.go b/jsonrpc2.go index 97e26d7..7d3e132 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -59,10 +59,10 @@ const ( // Handler handles JSON-RPC requests and notifications. type Handler interface { - // Handle is called to handle a request. No other requests are handled - // until it returns. If you do not require strict ordering behavior - // of received RPCs, it is suggested to wrap your handler in - // AsyncHandler. + // Handle is called to handle a request. No other requests are handled until + // it returns. If you do not require strict ordering behavior of received + // RPCs, it is suggested to wrap your handler in AsyncHandler. The context + // is automatically canceled when the connection closes. Handle(context.Context, *Conn, *Request) } From 3c4c92ad61e8a64c37816d2c573f5d0094d96d33 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:36:38 +0200 Subject: [PATCH 27/29] Bump actions/checkout from 4 to 5 in the github-actions group (#91) Bumps the github-actions group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 4 to 5 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- .github/workflows/scip.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 633144a..e38deb3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml index 24a4a0e..4d72d7e 100644 --- a/.github/workflows/scip.yml +++ b/.github/workflows/scip.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest container: sourcegraph/scip-go steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Get src-cli run: curl -L https://sourcegraph.com/.api/src-cli/src_linux_amd64 -o /usr/local/bin/src; chmod +x /usr/local/bin/src From ef3ea8b2ea04744e4806bd2de8e9fdb2de1b2a73 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 2 Oct 2025 20:55:33 +0200 Subject: [PATCH 28/29] Bump actions/setup-go from 5 to 6 in the github-actions group (#92) Bumps the github-actions group with 1 update: [actions/setup-go](https://github.com/actions/setup-go). Updates `actions/setup-go` from 5 to 6 - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e38deb3..66ba42e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: steps: - uses: actions/checkout@v5 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v6 with: go-version: ${{ matrix.go }} id: go From 4756698b1e49a0d796ed81be119fd5bafdf7ff16 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Dec 2025 09:01:38 +0200 Subject: [PATCH 29/29] Bump actions/checkout from 5 to 6 in the github-actions group (#93) --- .github/workflows/ci.yml | 2 +- .github/workflows/scip.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66ba42e..29413b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 with: diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml index 4d72d7e..c0dc33b 100644 --- a/.github/workflows/scip.yml +++ b/.github/workflows/scip.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest container: sourcegraph/scip-go steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Get src-cli run: curl -L https://sourcegraph.com/.api/src-cli/src_linux_amd64 -o /usr/local/bin/src; chmod +x /usr/local/bin/src