jsonrpc2: do not panic when sending unmarshalable params
If params given to (*Connection).Call is not JSON-marshalable, Call
returns immediately an *AsyncCall instance containing the
corresponding error, but its ctx field is not initialized. This
produces a panic later in golang.org/x/exp/event.New, as it does not
expect a nil context.Context.
Fixes golang/go#61654.
Change-Id: I6464ab5fde85670267b25821e0b57af04c331c8c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/516556
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
diff --git a/jsonrpc2/conn.go b/jsonrpc2/conn.go
index 882d15b..f0e527c 100644
--- a/jsonrpc2/conn.go
+++ b/jsonrpc2/conn.go
@@ -144,18 +144,18 @@
id: Int64ID(atomic.AddInt64(&c.seq, 1)),
resultBox: make(chan asyncResult, 1),
}
- // generate a new request identifier
- call, err := NewCall(result.id, method, params)
- if err != nil {
- //set the result to failed
- result.resultBox <- asyncResult{err: errors.Errorf("marshaling call parameters: %w", err)}
- return result
- }
- //TODO: rewrite this using the new target/prototype stuff
+ // TODO: rewrite this using the new target/prototype stuff
ctx = event.Start(ctx, method,
Method(method), RPCDirection(Outbound), RPCID(fmt.Sprintf("%q", result.id)))
Started.Record(ctx, 1, Method(method))
result.ctx = ctx
+ // generate a new request identifier
+ call, err := NewCall(result.id, method, params)
+ if err != nil {
+ // set the result to failed
+ result.resultBox <- asyncResult{err: errors.Errorf("marshaling call parameters: %w", err)}
+ return result
+ }
// We have to add ourselves to the pending map before we send, otherwise we
// are racing the response.
// rchan is buffered in case the response arrives without a listener.
@@ -355,7 +355,7 @@
select {
case nextReq, ok = <-fromRead:
case toDeliver <- q[0]:
- //TODO: this causes a lot of shuffling, should we use a growing ring buffer? compaction?
+ // TODO: this causes a lot of shuffling, should we use a growing ring buffer? compaction?
q = q[1:]
}
}
@@ -413,7 +413,7 @@
}
if err := c.respond(entry, result, rerr); err != nil {
// no way to propagate this error
- //TODO: should we do more than just log it?
+ // TODO: should we do more than just log it?
event.Error(entry.baseCtx, "jsonrpc2 message delivery failed", err)
}
}
@@ -441,7 +441,7 @@
err = errors.Errorf("%w: %q notification failed: %v", ErrInternal, entry.request.Method, rerr)
rerr = nil
case result != nil:
- //notification produced a response, which is an error
+ // notification produced a response, which is an error
err = errors.Errorf("%w: %q produced unwanted response", ErrInternal, entry.request.Method)
default:
// normal notification finish
diff --git a/jsonrpc2/jsonrpc2_test.go b/jsonrpc2/jsonrpc2_test.go
index 85a4981..40df6ed 100644
--- a/jsonrpc2/jsonrpc2_test.go
+++ b/jsonrpc2/jsonrpc2_test.go
@@ -10,6 +10,7 @@
"fmt"
"path"
"reflect"
+ "strings"
"testing"
"time"
@@ -60,6 +61,7 @@
notify{"unblock", "a"},
collect{"a", true, false},
}},
+ callErr{"error", func() {}, "marshaling call parameters: json: unsupported type"},
}
type binder struct {
@@ -90,6 +92,12 @@
expect interface{}
}
+type callErr struct {
+ method string
+ params interface{}
+ expectErr string
+}
+
type async struct {
name string
method string
@@ -175,6 +183,18 @@
verifyResults(t, test.method, results, test.expect)
}
+func (test callErr) Name() string { return test.method }
+func (test callErr) Invoke(t *testing.T, ctx context.Context, h *handler) {
+ var results interface{}
+ if err := h.conn.Call(ctx, test.method, test.params).Await(ctx, &results); err != nil {
+ if serr := err.Error(); !strings.Contains(serr, test.expectErr) {
+ t.Fatalf("%v:Call failed but with unexpected error: %q does not contain %q", test.method, serr, test.expectErr)
+ }
+ return
+ }
+ t.Fatalf("%v:Call succeeded (%v) but should have failed with error containing %q", test.method, results, test.expectErr)
+}
+
func (test echo) Invoke(t *testing.T, ctx context.Context, h *handler) {
results := newResults(test.expect)
if err := h.conn.Call(ctx, "echo", []interface{}{test.method, test.params}).Await(ctx, results); err != nil {
