Skip to content

Commit

Permalink
service: return an error when a client calls an unknown method (#1571)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aarzilli authored and derekparker committed Jun 4, 2019
1 parent 2d6d016 commit 72fae3c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
1 change: 1 addition & 0 deletions service/rpccommon/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
38 changes: 36 additions & 2 deletions service/test/integration2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"math/rand"
"net"
"net/rpc"
"net/rpc/jsonrpc"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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},
Expand All @@ -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()
Expand Down Expand Up @@ -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)
}
}

0 comments on commit 72fae3c

Please sign in to comment.