From 040dc22f8a6046fee3e8b2c3986a23e262c9cf2f Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 28 Feb 2023 23:46:15 -0500 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 04/15] 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 05/15] 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 06/15] 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 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] 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 12/15] 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 13/15] 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 14/15] 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 15/15] 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