Consul crashes on empty body because args.Query is nil

[jameshartig]

consul version for both Client and Server

Client: 1.0.2 (also happens on 1.0.0)
Server: 1.0.2 (also happens on 1.0.0)

Operating system and Environment details

Tested on CentOS and Ubuntu. Also happens on Mac.

Description of the Issue (and unexpected/desired result)

Consul crashes when sent empty POST body to /v1/query via a client.

Reproduction steps

Spin up a server and client

./consul agent -dev -log-level=TRACE &
./consul agent -retry-join 127.0.0.1 -client 127.0.0.2 -bind 127.0.0.2 -data-dir /tmp -disable-host-node-id -node agent -log-level=TRACE &
sleep 1 && ./consul members

That should print out:

Node         Address         Status  Type    Build  Protocol  DC   Segment
james-GS-PC  127.0.0.1:8301  alive   server  1.0.2  2         dc1  <all>
agent        127.0.0.2:8301  alive   client  1.0.2  2         dc1  <default>

Then make POST request via curl:

$ curl --request POST --data '' -H "Content-Length: 0" http://127.0.0.2:8500/v1/query
rpc error making call: EOF

Then the server should crash (and agent will complain and remove server):

    2018/01/10 11:01:33 [ERR] consul: "PreparedQuery.Apply" RPC failed to server 127.0.0.1:8300: rpc error making call: EOF
    2018/01/10 11:01:33 [ERR] http: Request POST /v1/query, error: rpc error making call: EOF from=127.0.0.1:60668
    2018/01/10 11:01:33 [DEBUG] http: Request POST /v1/query (562µs) from=127.0.0.1:60668
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x9da58c]

goroutine 116 [running]:
github.com/hashicorp/consul/agent/consul.(*PreparedQuery).Apply(0xc420221258, 0xc4201ef680, 0xc42052e0a0, 0x0, 0x0)
        /gopath/src/github.com/hashicorp/consul/agent/consul/prepared_query_endpoint.go:41 +0x2ac
reflect.Value.call(0xc4203cbe00, 0xc420221280, 0x13, 0x13ed386, 0x4, 0xc420357e18, 0x3, 0x3, 0x10d2760, 0x18, ...)
        /goroot/src/reflect/value.go:434 +0x905
reflect.Value.Call(0xc4203cbe00, 0xc420221280, 0x13, 0xc420357e18, 0x3, 0x3, 0x16, 0x16, 0x16)
        /goroot/src/reflect/value.go:302 +0xa4
net/rpc.(*service).call(0xc420404a40, 0xc42042c230, 0xc42052c1e8, 0xc420133b80, 0xc4203d68c0, 0x1220fc0, 0xc4201ef680, 0x16, 0x10970a0, 0xc42052e0a0,
 ...)
        /goroot/src/net/rpc/server.go:381 +0x142
net/rpc.(*Server).ServeRequest(0xc42042c230, 0x1c9ef20, 0xc420404fc0, 0x3f800000, 0x0)
        /goroot/src/net/rpc/server.go:496 +0x22c
github.com/hashicorp/consul/agent/consul.(*Server).handleConsulConn(0xc4202946c0, 0x1ca41c0, 0xc4202329c0)
        /gopath/src/github.com/hashicorp/consul/agent/consul/rpc.go:156 +0x145
created by github.com/hashicorp/consul/agent/consul.(*Server).handleMultiplexV2
        /gopath/src/github.com/hashicorp/consul/agent/consul/rpc.go:141 +0x18d
    2018/01/10 11:01:34 [DEBUG] memberlist: Failed ping: james-GS-PC (timeout reached)
    2018/01/10 11:01:35 [INFO] memberlist: Suspect james-GS-PC has failed, no acks received
    2018/01/10 11:01:36 [DEBUG] memberlist: Failed ping: james-GS-PC (timeout reached)
    2018/01/10 11:01:37 [INFO] memberlist: Suspect james-GS-PC has failed, no acks received
    2018/01/10 11:01:39 [DEBUG] memberlist: Failed ping: james-GS-PC (timeout reached)
    2018/01/10 11:01:39 [INFO] memberlist: Marking james-GS-PC as failed, suspect timeout reached (0 peer confirmations)
    2018/01/10 11:01:39 [INFO] serf: EventMemberFailed: james-GS-PC 127.0.0.1
    2018/01/10 11:01:39 [INFO] consul: removing server james-GS-PC (Addr: tcp/127.0.0.1:8300) (DC: dc1)

Looking at the stacktrace, the problem seems to be that args.Query is nil and it's checking to see if args.Query.ID is not empty. The preparedQueryCreate function is not json decoding unless if req.ContentLength > 0 {. Maybe it should be erroring if the body is empty?

[aaronhurt]

I can confirm this is reproducible on osx with 1.0.0, 1.0.1 and 1.0.2 running a full 3 node local cluster and one agent node.

Sending a blank body to a server or client agent results in a crash of one of the server agents in my testing.

[slackpad]

Thanks for the issue @fastest963 and the confirmation @leprechau - that should not have been made optional so now that will return a 400!