From 3cd679da63f86512d4940b0b54080a4d6145d5bd Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Wed, 16 Mar 2016 18:04:42 -0600 Subject: [PATCH 1/4] feat(httputil): add a Decoder This adds a Content-Type-sensitive decoder for HTTP requests. --- pkg/httputil/encoder.go | 58 +++++++++++++++++++++++++++++++++++++- pkg/httputil/httperrors.go | 6 ++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/pkg/httputil/encoder.go b/pkg/httputil/encoder.go index df5cfb428..c491a4449 100644 --- a/pkg/httputil/encoder.go +++ b/pkg/httputil/encoder.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" "mime" "net/http" "reflect" @@ -29,7 +30,10 @@ import ( ) // DefaultEncoder is an *AcceptEncoder with the default application/json encoding. -var DefaultEncoder = &AcceptEncoder{DefaultEncoding: "application/json"} +var DefaultEncoder = &AcceptEncoder{DefaultEncoding: "application/json", MaxReadLen: DefaultMaxReadLen} + +// DefaultMaxReadLen is the default maximum length to accept in an HTTP request body. +var DefaultMaxReadLen int64 = 1024 * 1024 // Encoder takes input and translate it to an expected encoded output. // @@ -46,6 +50,19 @@ type Encoder interface { // // The integer must be a valid http.Status* status code. Encode(http.ResponseWriter, *http.Request, interface{}, int) + + // Decode reads and decodes a request body. + Decode(http.ResponseWriter, *http.Request, interface{}) error +} + +// Decode decodes a request body using the DefaultEncoder. +func Decode(w http.ResponseWriter, r *http.Request, v interface{}) error { + return DefaultEncoder.Decode(w, r, v) +} + +// Encode encodes a request body using the DefaultEncoder. +func Encode(w http.ResponseWriter, r *http.Request, v interface{}, statusCode int) { + DefaultEncoder.Encode(w, r, v, statusCode) } // AcceptEncoder uses the accept headers on a request to determine the response type. @@ -58,6 +75,7 @@ type Encoder interface { // It treats `application/x-yaml` as `text/yaml`. type AcceptEncoder struct { DefaultEncoding string + MaxReadLen int64 } // Encode encodeds the given interface to the first available type in the Accept header. @@ -79,6 +97,35 @@ func (e *AcceptEncoder) Encode(w http.ResponseWriter, r *http.Request, out inter w.Write(data) } +// Decode decodes the given request into the given interface. +// +// It selects the marshal based on the value of the Content-Type header. If no +// viable decoder is found, it attempts to use the DefaultEncoder. +func (e *AcceptEncoder) Decode(w http.ResponseWriter, r *http.Request, v interface{}) error { + if e.MaxReadLen > 0 && r.ContentLength > int64(e.MaxReadLen) { + RequestEntityTooLarge(w, r, fmt.Sprintf("Max len is %d, submitted len is %d.", e.MaxReadLen, r.ContentLength)) + } + data, err := ioutil.ReadAll(r.Body) + r.Body.Close() + if err != nil { + return err + } + + ct := r.Header.Get("content-type") + mt, _, err := mime.ParseMediaType(ct) + if err != nil { + mt = "application/x-octet-stream" + } + + for n, fn := range decoders { + if n == mt { + return fn(data, v) + } + } + + return decoders[e.DefaultEncoding](data, v) +} + // parseAccept parses the value of an Accept: header and returns the best match. // // This returns the matched MIME type and the Marshal function. @@ -100,6 +147,9 @@ func (e *AcceptEncoder) parseAccept(h string) (string, Marshaler) { // Marshaler marshals an interface{} into a []byte. type Marshaler func(interface{}) ([]byte, error) +// Unmarshaler unmarshals []byte to an interface{}. +type Unmarshaler func([]byte, interface{}) error + var encoders = map[string]Marshaler{ "application/json": json.Marshal, "text/yaml": yaml.Marshal, @@ -107,6 +157,12 @@ var encoders = map[string]Marshaler{ "text/plain": textMarshal, } +var decoders = map[string]Unmarshaler{ + "application/json": json.Unmarshal, + "text/yaml": yaml.Unmarshal, + "application/x-yaml": yaml.Unmarshal, +} + // ErrUnsupportedKind indicates that the marshal cannot marshal a particular Go Kind (e.g. struct or chan). var ErrUnsupportedKind = errors.New("unsupported kind") diff --git a/pkg/httputil/httperrors.go b/pkg/httputil/httperrors.go index 2a2577b39..551dc8f54 100644 --- a/pkg/httputil/httperrors.go +++ b/pkg/httputil/httperrors.go @@ -54,6 +54,12 @@ func NotFound(w http.ResponseWriter, r *http.Request) { writeErr(w, r, msg, http.StatusNotFound) } +// RequestEntityTooLarge writes a 413 to the client and logs an error. +func RequestEntityTooLarge(w http.ResponseWriter, r *http.Request, msg string) { + log.Println(msg) + writeErr(w, r, msg, http.StatusRequestEntityTooLarge) +} + // BadRequest writes an HTTP 400. func BadRequest(w http.ResponseWriter, r *http.Request, err error) { log.Printf(LogBadRequest, r.Method, r.URL, err) From dec2f0605a73df75b86bef1c31aad90afc1126e7 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Wed, 16 Mar 2016 18:05:18 -0600 Subject: [PATCH 2/4] fix(manager): replace Unmarshal with Decode Instead of repeating io.Read/Unmarshal pattern, use the Decoder, which also gets us content-type negotiation. --- cmd/manager/deployments.go | 34 ++++++---------------------------- cmd/manager/main.go | 9 ++++++--- 2 files changed, 12 insertions(+), 31 deletions(-) diff --git a/cmd/manager/deployments.go b/cmd/manager/deployments.go index a1de5e80a..c64f99439 100644 --- a/cmd/manager/deployments.go +++ b/cmd/manager/deployments.go @@ -17,7 +17,6 @@ limitations under the License. package main import ( - "encoding/json" "errors" "fmt" "io" @@ -298,19 +297,11 @@ func getPathVariable(w http.ResponseWriter, r *http.Request, variable, handler s func getTemplate(w http.ResponseWriter, r *http.Request, handler string) *common.Template { util.LogHandlerEntry(handler, r) - j, err := getJSONFromRequest(w, r, handler) - - if err != nil { - return nil - } - t := &common.Template{} - if err := json.Unmarshal(j, t); err != nil { - e := fmt.Errorf("%v\n%v", err, string(j)) - util.LogAndReturnError(handler, http.StatusBadRequest, e, w) + if err := httputil.Decode(w, r, t); err != nil { + httputil.BadRequest(w, r, err) return nil } - return t } @@ -479,18 +470,12 @@ func getRegistryHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Co func getRegistry(w http.ResponseWriter, r *http.Request, handler string) *common.Registry { util.LogHandlerEntry(handler, r) - j, err := getJSONFromRequest(w, r, handler) - if err != nil { - return nil - } t := &common.Registry{} - if err := json.Unmarshal(j, t); err != nil { - e := fmt.Errorf("%v\n%v", err, string(j)) - util.LogAndReturnError(handler, http.StatusBadRequest, e, w) + if err := httputil.Decode(w, r, t); err != nil { + httputil.BadRequest(w, r, err) return nil } - return t } @@ -611,18 +596,11 @@ func getFileHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Contex func getCredential(w http.ResponseWriter, r *http.Request, handler string) *common.RegistryCredential { util.LogHandlerEntry(handler, r) - j, err := getJSONFromRequest(w, r, handler) - if err != nil { - return nil - } - t := &common.RegistryCredential{} - if err := json.Unmarshal(j, t); err != nil { - e := fmt.Errorf("%v\n%v", err, string(j)) - util.LogAndReturnError(handler, http.StatusBadRequest, e, w) + if err := httputil.Decode(w, r, t); err != nil { + httputil.BadRequest(w, r, err) return nil } - return t } diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 126e9ed77..7005b888c 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -17,14 +17,15 @@ limitations under the License. package main import ( - "github.com/kubernetes/helm/cmd/manager/router" - "github.com/kubernetes/helm/pkg/version" - "flag" "fmt" "log" "net/http" "os" + + "github.com/kubernetes/helm/cmd/manager/router" + "github.com/kubernetes/helm/pkg/httputil" + "github.com/kubernetes/helm/pkg/version" ) var ( @@ -52,6 +53,8 @@ func main() { os.Exit(1) } + httputil.DefaultEncoder.MaxReadLen = c.Config.MaxTemplateLength + // Set up routes handler := router.NewHandler(c) registerDeploymentRoutes(c, handler) From 035ba623f26a7f8a53d4fa9dec436dfe3918c311 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 17 Mar 2016 09:58:10 -0600 Subject: [PATCH 3/4] fix(httputil): add test for decoder --- pkg/httputil/encoder_test.go | 54 +++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/pkg/httputil/encoder_test.go b/pkg/httputil/encoder_test.go index 71ddc19eb..ba5274aa7 100644 --- a/pkg/httputil/encoder_test.go +++ b/pkg/httputil/encoder_test.go @@ -17,6 +17,7 @@ limitations under the License. package httputil import ( + "bytes" "encoding/json" "errors" "io/ioutil" @@ -72,7 +73,58 @@ func TestTextMarshal(t *testing.T) { } } -func TestAcceptEncoder(t *testing.T) { +type encDec struct { + Name string +} + +func TestDefaultEncoder(t *testing.T) { + in := &encDec{Name: "Foo"} + var out, out2 encDec + + fn := func(w http.ResponseWriter, r *http.Request) { + if err := Decode(w, r, &out); err != nil { + t.Fatalf("Failed to decode data: %s", err) + } + if out.Name != in.Name { + t.Fatalf("Expected %q, got %q", in.Name, out.Name) + } + Encode(w, r, out, http.StatusOK) + } + s := httptest.NewServer(http.HandlerFunc(fn)) + defer s.Close() + + data, err := json.Marshal(in) + if err != nil { + t.Fatalf("Failed to marshal JSON: %s", err) + } + req, err := http.NewRequest("GET", s.URL, bytes.NewBuffer(data)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("content-type", "application/json") + + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Errorf("Failed request: %s", err) + } + if res.StatusCode != http.StatusOK { + t.Errorf("Expected 200, got %d", res.StatusCode) + } + + data, err = ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(data, &out2); err != nil { + t.Fatal(err) + } + if out2.Name != in.Name { + t.Errorf("Expected final output to have name %q, got %q", in.Name, out2.Name) + } +} + +func TestAcceptEncoderEncoder(t *testing.T) { enc := &AcceptEncoder{ DefaultEncoding: "application/json", } From 70db55d66e2ccd5a5fc310e87c78670013a30f09 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Thu, 17 Mar 2016 16:45:13 -0600 Subject: [PATCH 4/4] fix(httputil): change order of struct fields This is because the marshals traverse structs in field order, and it makes more sense for the fields to have status first and details second. --- pkg/httputil/httperrors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/httputil/httperrors.go b/pkg/httputil/httperrors.go index 551dc8f54..6be920a73 100644 --- a/pkg/httputil/httperrors.go +++ b/pkg/httputil/httperrors.go @@ -38,8 +38,8 @@ const ( // For example, and error can be serialized to JSON or YAML. Likewise, the // string marshal can convert it to a string. type Error struct { - Msg string `json:"message, omitempty"` Status string `json:"status"` + Msg string `json:"message, omitempty"` } // Error implements the error interface.