code review 6446051: net/rpc/jsonrpc: handles missing "params" in jsonrpc.

net/rpc/jsonrpc: handles missing "params" in jsonrpc.

A crash happens in the first request in a connection
if "params" field is missing because c.req.Params is Nil.

Fixes issue 3848.

[brtzsnr, 2012-07-26 14:48:29]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[rsc, 2012-08-06 20:48:24]

Thanks for working on this.

http://codereview.appspot.com/6446051/diff/3007/src/pkg/net/rpc/jsonrpc/serve...
File src/pkg/net/rpc/jsonrpc/server.go (right):

http://codereview.appspot.com/6446051/diff/3007/src/pkg/net/rpc/jsonrpc/serve...
src/pkg/net/rpc/jsonrpc/server.go:53: if r.Params == nil {
I don't know what this code (before your edits) was trying to do, but it looks
buggy. Please make this

r.Params = nil
r.Id = nil

That's obviously a real reset, and it avoids possibly reusing memory that might
not belong to it.

http://codereview.appspot.com/6446051/diff/3007/src/pkg/net/rpc/jsonrpc/serve...
src/pkg/net/rpc/jsonrpc/server.go:98: return json.Unmarshal(*c.req.Params,
&params)
Then here you would need

if c.req.Params == nil {
    return errMissingParams
}

where there's a top-level global:

var errMissingParams = errors.New("jsonrpc: request body missing params")

[brtzsnr, 2012-08-07 08:59:33]

PTAL.

[brtzsnr, 2012-08-20 12:22:57]

On 2012/08/07 08:59:33, brtzsnr wrote:
> PTAL.

Any update on this patch?

[rsc, 2012-08-31 16:01:27]

Sorry, I had an unplanned absence for a while taking care of other
things. Your CL is patched into my client from back before that, which
suggests I was in the middle of trying to submit it. I should be able
to figure out what the state is later today.

Russ

[rsc, 2012-08-31 19:51:20]

LGTM

[rsc, 2012-08-31 19:52:32]

*** Submitted as http://code.google.com/p/go/source/detail?r=d15f71f90167 ***

net/rpc/jsonrpc: handles missing "params" in jsonrpc.

A crash happens in the first request in a connection
if "params" field is missing because c.req.Params is Nil.

Fixes issue 3848.

R=golang-dev, rsc
CC=golang-dev
http://codereview.appspot.com/6446051

Committer: Russ Cox <rsc@golang.org>