From 72fae3c9c11da101bd40d76c125970acb3d3ec4f Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Tue, 4 Jun 2019 23:33:19 +0200 Subject: [PATCH] service: return an error when a client calls an unknown method (#1571) Without this a client calling an method on a version of Delve that doesn't have that method (for example because it's old) will never get a response back. --- service/rpccommon/server.go | 1 + service/test/integration2_test.go | 38 +++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go index 5b60c045..470b4914 100644 --- a/service/rpccommon/server.go +++ b/service/rpccommon/server.go @@ -278,6 +278,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) { mtype, ok := s.methodMaps[s.config.APIVersion-1][req.ServiceMethod] if !ok { s.log.Errorf("rpc: can't find method %s", req.ServiceMethod) + s.sendResponse(sending, &req, &rpc.Response{}, nil, codec, fmt.Sprintf("unknown method: %s", req.ServiceMethod)) continue } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 252bb399..c0f152ee 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -5,6 +5,8 @@ import ( "fmt" "math/rand" "net" + "net/rpc" + "net/rpc/jsonrpc" "os" "path/filepath" "reflect" @@ -48,7 +50,7 @@ func withTestClient2(name string, t *testing.T, fn func(c service.Client)) { }) } -func withTestClient2Extended(name string, t *testing.T, fn func(c service.Client, fixture protest.Fixture)) { +func startServer(name string, t *testing.T) (clientConn net.Conn, fixture protest.Fixture) { if testBackend == "rr" { protest.MustHaveRecordingAllowed(t) } @@ -58,7 +60,7 @@ func withTestClient2Extended(name string, t *testing.T, fn func(c service.Client if buildMode == "pie" { buildFlags = protest.BuildModePIE } - fixture := protest.BuildFixture(name, buildFlags) + fixture = protest.BuildFixture(name, buildFlags) server := rpccommon.NewServer(&service.Config{ Listener: listener, ProcessArgs: []string{fixture.Path}, @@ -68,6 +70,11 @@ func withTestClient2Extended(name string, t *testing.T, fn func(c service.Client if err := server.Run(); err != nil { t.Fatal(err) } + return clientConn, fixture +} + +func withTestClient2Extended(name string, t *testing.T, fn func(c service.Client, fixture protest.Fixture)) { + clientConn, fixture := startServer(name, t) client := rpc2.NewClientFromConn(clientConn) defer func() { dir, _ := client.TraceDirectory() @@ -1675,3 +1682,30 @@ func TestAncestors(t *testing.T) { } }) } + +type brokenRPCClient struct { + client *rpc.Client +} + +func (c *brokenRPCClient) Detach(kill bool) error { + defer c.client.Close() + out := new(rpc2.DetachOut) + return c.call("Detach", rpc2.DetachIn{kill}, out) +} + +func (c *brokenRPCClient) call(method string, args, reply interface{}) error { + return c.client.Call("RPCServer."+method, args, reply) +} + +func TestUnknownMethodCall(t *testing.T) { + clientConn, _ := startServer("continuetestprog", t) + client := &brokenRPCClient{jsonrpc.NewClient(clientConn)} + client.call("SetApiVersion", api.SetAPIVersionIn{2}, &api.SetAPIVersionOut{}) + defer client.Detach(true) + var out int + err := client.call("NonexistentRPCCall", nil, &out) + assertError(err, t, "call()") + if !strings.HasPrefix(err.Error(), "unknown method: ") { + t.Errorf("wrong error message: %v", err) + } +} -- GitLab