server/prpc: updated server according to protocol

server/prpc/decoding.go

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package prpc
 
 import (
         "bytes"
         "encoding/base64"
         "fmt"
@@ skipping 46 matching lines @@
         case formatBinary, formatJSONPB, formatText:
                 return f, nil
 
         default:
                 panic("cannot happen")
         }
 }
 
 // readMessage decodes a protobuf message from an HTTP request.
 // Does not close the request body.
-func readMessage(r *http.Request, msg proto.Message) *httpError {
+func readMessage(r *http.Request, msg proto.Message) *protocolError {
         if msg == nil {
-                panicf("cannot decode to nil")
+                panic("cannot decode to nil")
         }
         format, err := requestFormat(r.Header.Get(headerContentType))
         if err != nil {
                 // Spec: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.16
                 return errorf(http.StatusUnsupportedMediaType, "Content-Type header: %s", err)
         }
 
         buf, err := ioutil.ReadAll(r.Body)
         if err != nil {
                 return errorf(http.StatusBadRequest, "could not read body: %s", err)
         }
         if len(buf) == 0 {
                 // no body, no message

[dnj, 2016/01/26 19:26:38]

Is this true? A completely zero-value protobuf would be zero length, no? Do we
need to check this, anyway? If there is insufficient body, the unmarshalling
will fail below.

                 return nil
         }
 
         switch format {
         // Do not redefine "err" below.
 
         case formatBinary:
                 err = proto.Unmarshal(buf, msg)
 
         case formatJSONPB:
                 err = jsonpb.Unmarshal(bytes.NewBuffer(buf), msg)
 
         case formatText:
                 err = proto.UnmarshalText(string(buf), msg)
 
         default:
-                panicf("cannot happen")
+                panic("cannot happen")

[dnj, 2016/01/26 19:26:38]

nit: I'd prefer that you either don't check this case (b/c it can't happen) or
return a protocol error here instead of panic. I understand that this can never
happen, but having a panic inline with user request handling is a bad practice
in general.

         }
         if err != nil {
                 return errorf(http.StatusBadRequest, "could not decode body: %s", err)
         }
         return nil
 }
 
 // parseHeader parses HTTP headers and derives a new context.
 // Supports headerTimeout.
 // Ignores "Accept" and "Content-Type" headers.
@@ skipping 47 matching lines @@
                                 }
                                 md[trimmedName] = append(md[trimmedName], string(decoded))
                         }
                 }
         }
         if addedMeta {
                 c = metadata.NewContext(c, md)
         }
         return c, nil
 }