server/prpc: updated server according to protocol

server/prpc: updated server according to protocol

Update pRPC server to conform the protocol
https://godoc.org/github.com/luci/luci-go/common/prpc#hdr-Protocol

R=dnj@chromium.org
BUG=

Committed: https://github.com/luci/luci-go/commit/a35263a6fb32d3247fe84019e0e336223472c02a

[dnj, 2016-01-27 00:10:28]

dnj@chromium.org changed reviewers:
+ vadimsh@chromium.org

[dnj, 2016-01-27 00:10:29]

PTAL. Continuation of: https://codereview.chromium.org/1638493004/

[dnj, 2016-01-27 00:10:37]

Description was changed from

==========
server/prpc: updated server according to protocol

Update pRPC server to conform the protocol
https://godoc.org/github.com/luci/luci-go/common/prpc#hdr-Protocol

R=dnj@chromium.org
BUG=
==========

to

==========
server/prpc: updated server according to protocol

Update pRPC server to conform the protocol
https://godoc.org/github.com/luci/luci-go/common/prpc#hdr-Protocol

R=dnj@chromium.org
BUG=
==========

[dnj, 2016-01-27 00:10:37]

dnj@chromium.org changed reviewers:
+ iannucci@chromium.org

[nodir, 2016-01-27 00:16:51]

nodir@chromium.org changed reviewers:
+ nodir@chromium.org

[nodir, 2016-01-27 00:16:51]

lgtm

[iannucci, 2016-01-27 00:17:29]

iannucci@chromium.org changed reviewers:
- nodir@chromium.org

[iannucci, 2016-01-27 00:17:30]

lgtm

https://chromiumcodereview.appspot.com/1636873006/diff/20001/server/prpc/enco...
File server/prpc/encoding.go (right):

https://chromiumcodereview.appspot.com/1636873006/diff/20001/server/prpc/enco...
server/prpc/encoding.go:23: csrfPrefix   = ")]}'\n"
gross.

https://chromiumcodereview.appspot.com/1636873006/diff/20001/server/prpc/enco...
server/prpc/encoding.go:78: res := response{header: http.Header{}}
couldn't this just be `res := response{}`?

edit: nvm http.Header is a map type. grr inconsistency.

[Vadim Sh., 2016-01-27 00:21:10]

lgtm with nits

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go
File server/prpc/response.go (right):

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go...
server/prpc/response.go:28: newLine bool // whether to write \n after body
why not just write '\n' to the buffer instead of setting newLine = true? newLine
thing looks weird.

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go...
server/prpc/response.go:39: body:    []byte(fmt.Sprintf(format, a...)),
e.g. format + "\n" here

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/server.go
File server/prpc/server.go (right):

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/server.go#n...
server/prpc/server.go:124: fmt.Sprintf("service %q is not implemented",
serviceName))
errResponse does Sprintf inside, no need to call it separately

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/server.go#n...
server/prpc/server.go:132: fmt.Sprintf("method %q in service %q is not
implemented", methodName, serviceName))
here too

(check other invocations too, just in case)

[dnj, 2016-01-27 00:39:59]

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/encoding.go
File server/prpc/encoding.go (right):

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/encoding.go...
server/prpc/encoding.go:23: csrfPrefix   = ")]}'\n"
On 2016/01/27 00:17:29, iannucci wrote:
> gross.

Acknowledged.

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go
File server/prpc/response.go (right):

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go...
server/prpc/response.go:28: newLine bool // whether to write \n after body
On 2016/01/27 00:21:09, Vadim Sh. wrote:
> why not just write '\n' to the buffer instead of setting newLine = true?
newLine
> thing looks weird.

Done.

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/response.go...
server/prpc/response.go:39: body:    []byte(fmt.Sprintf(format, a...)),
On 2016/01/27 00:21:09, Vadim Sh. wrote:
> e.g. format + "\n" here

Done.

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/server.go
File server/prpc/server.go (right):

https://codereview.chromium.org/1636873006/diff/20001/server/prpc/server.go#n...
server/prpc/server.go:132: fmt.Sprintf("method %q in service %q is not
implemented", methodName, serviceName))
On 2016/01/27 00:21:10, Vadim Sh. wrote:
> here too
> 
> (check other invocations too, just in case)

Done.

[dnj, 2016-01-27 00:40:09]

The CQ bit was checked by dnj@chromium.org

[dnj, 2016-01-27 00:40:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
vadimsh@chromium.org, iannucci@chromium.org, nodir@chromium.org
Link to the patchset: https://codereview.chromium.org/1636873006/#ps40001
(title: "Respond to nits, escape externally-supplied strings when passed to
format parameters.")

[commit-bot: I haz the power, 2016-01-27 00:40:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1636873006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1636873006/40001

[commit-bot: I haz the power, 2016-01-27 00:42:59]

Description was changed from

==========
server/prpc: updated server according to protocol

Update pRPC server to conform the protocol
https://godoc.org/github.com/luci/luci-go/common/prpc#hdr-Protocol

R=dnj@chromium.org
BUG=
==========

to

==========
server/prpc: updated server according to protocol

Update pRPC server to conform the protocol
https://godoc.org/github.com/luci/luci-go/common/prpc#hdr-Protocol

R=dnj@chromium.org
BUG=

Committed:
https://github.com/luci/luci-go/commit/a35263a6fb32d3247fe84019e0e336223472c02a
==========

[commit-bot: I haz the power, 2016-01-27 00:43:00]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/a35263a6fb32d3247fe84019e0e336223472c02a