From 01181268d68c23cf2afb045b00b919677d72e724 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Wed, 17 Jun 2020 15:44:07 -0300 Subject: [PATCH] fix(api): use ajg/form decoder only for Parameters field --- api/api.go | 30 ----------- api/api_test.go | 96 ----------------------------------- api/service_handlers.go | 42 ++++++++++++++- api/service_handlers_test.go | 15 ++---- internal/pkg/rpaas/manager.go | 6 +++ 5 files changed, 50 insertions(+), 139 deletions(-) delete mode 100644 api/api_test.go diff --git a/api/api.go b/api/api.go index a9c66b7b2..7b5109518 100644 --- a/api/api.go +++ b/api/api.go @@ -7,16 +7,13 @@ package api import ( "context" "fmt" - "io" "net/http" "os" "os/signal" - "strings" "sync" "syscall" "time" - "github.com/ajg/form" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" "github.com/tsuru/rpaas-operator/config" @@ -161,7 +158,6 @@ func errorMiddleware(next echo.HandlerFunc) echo.HandlerFunc { func newEcho() *echo.Echo { e := echo.New() - e.Binder = new(requestBinder) e.HideBanner = true e.Use(middleware.Recover()) @@ -224,29 +220,3 @@ func newEcho() *echo.Echo { return e } - -type requestBinder struct{} - -func (b *requestBinder) Bind(i interface{}, c echo.Context) error { - req := c.Request() - - ctype := req.Header.Get(echo.HeaderContentType) - if !strings.HasPrefix(ctype, echo.MIMEApplicationForm) { - c.Response().Header().Set("Accept", echo.MIMEApplicationForm) - return echo.ErrUnsupportedMediaType - } - - if err := newDecoder(req.Body).Decode(i); err != nil { - return fmt.Errorf("cannot decode the parameters: %w", err) - } - defer req.Body.Close() - - return nil -} - -func newDecoder(r io.Reader) *form.Decoder { - decoder := form.NewDecoder(r) - decoder.IgnoreCase(true) - decoder.IgnoreUnknownKeys(true) - return decoder -} diff --git a/api/api_test.go b/api/api_test.go deleted file mode 100644 index be99361d7..000000000 --- a/api/api_test.go +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright 2020 tsuru 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 api - -import ( - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/labstack/echo/v4" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRequestBinder_Bind(t *testing.T) { - type t1 struct { - Name string `form:"name"` - Tags []string `form:"tags"` - Complex map[string]interface{} `form:"complex"` - Ignored bool `form:"-"` - } - - tests := []struct { - name string - c echo.Context - data interface{} - assert func(*testing.T, error, interface{}, echo.Context) - }{ - { - name: "when content-type is not application/x-www-form-urleconded", - c: func() echo.Context { - body := `{"name": "my-instance"}` - e := newEcho() - req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) - req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) - return e.NewContext(req, httptest.NewRecorder()) - }(), - assert: func(t *testing.T, err error, d interface{}, c echo.Context) { - require.Error(t, err) - assert.EqualError(t, err, "code=415, message=Unsupported Media Type, internal=") - assert.Equal(t, "application/x-www-form-urlencoded", c.Response().Header().Get("Accept")) - }, - }, - { - name: "submitting a complex object", - c: func() echo.Context { - body := `name=my-instance&tags.0=tag1&tags.1=tag2&tags.2=tag3&ignored=true&complex.key1=1&complex.other.key=value` - e := newEcho() - req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) - req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationForm) - return e.NewContext(req, httptest.NewRecorder()) - }(), - data: &t1{}, - assert: func(t *testing.T, err error, d interface{}, c echo.Context) { - require.NoError(t, err) - assert.Equal(t, &t1{ - Name: "my-instance", - Tags: []string{"tag1", "tag2", "tag3"}, - Complex: map[string]interface{}{ - "key1": "1", - "other": map[string]interface{}{ - "key": "value", - }, - }, - }, d) - }, - }, - { - name: "when some error occurs on decode method", - c: func() echo.Context { - body := `name=my-instance&tags.0=tag1&tags.1=tag2&tags.2=tag3&ignored=true&complex.key1=1&complex.other.key=value` - e := newEcho() - req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) - req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationForm) - return e.NewContext(req, httptest.NewRecorder()) - }(), - data: func() string { return "cannot decode a function" }, - assert: func(t *testing.T, err error, d interface{}, c echo.Context) { - require.Error(t, err) - assert.EqualError(t, err, "cannot decode the parameters: func() string has unsupported kind func") - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.NotNil(t, tt.assert) - b := &requestBinder{} - err := b.Bind(tt.data, tt.c) - tt.assert(t, err, tt.data, tt.c) - }) - } -} diff --git a/api/service_handlers.go b/api/service_handlers.go index f20bea8c4..afb45f2f0 100644 --- a/api/service_handlers.go +++ b/api/service_handlers.go @@ -5,16 +5,24 @@ package api import ( + "bytes" "fmt" + "io" + "io/ioutil" "net/http" "strings" + "github.com/ajg/form" "github.com/labstack/echo/v4" "github.com/tsuru/rpaas-operator/internal/pkg/rpaas" ) func serviceCreate(c echo.Context) error { - var args rpaas.CreateArgs + args := rpaas.CreateArgs{ + // NOTE: using a different decoder for Parameters since the `r.PostForm()` + // method does not understand the format used by github.com/ajf.form. + Parameters: decodeFormParameters(c.Request()), + } if err := c.Bind(&args); err != nil { return err } @@ -49,7 +57,11 @@ func serviceDelete(c echo.Context) error { } func serviceUpdate(c echo.Context) error { - var args rpaas.UpdateInstanceArgs + args := rpaas.UpdateInstanceArgs{ + // NOTE: using a different decoder for Parameters since the `r.PostForm()` + // method does not understand the format used by github.com/ajf.form. + Parameters: decodeFormParameters(c.Request()), + } if err := c.Bind(&args); err != nil { return err } @@ -191,3 +203,29 @@ func serviceStatus(c echo.Context) error { return c.NoContent(http.StatusNoContent) } + +func decodeFormParameters(r *http.Request) map[string]interface{} { + if r == nil { + return nil + } + + body := r.Body + defer body.Close() + + var buffer bytes.Buffer + reader := io.TeeReader(body, &buffer) + + var obj struct { + Parameters map[string]interface{} `form:"parameters"` + } + newFormDecoder(reader).Decode(&obj) + r.Body = ioutil.NopCloser(&buffer) + return obj.Parameters +} + +func newFormDecoder(r io.Reader) *form.Decoder { + decoder := form.NewDecoder(r) + decoder.IgnoreCase(true) + decoder.IgnoreUnknownKeys(true) + return decoder +} diff --git a/api/service_handlers_test.go b/api/service_handlers_test.go index ae55629c5..5f5f7f26a 100644 --- a/api/service_handlers_test.go +++ b/api/service_handlers_test.go @@ -44,7 +44,7 @@ func Test_serviceCreate(t *testing.T) { }, { name: "passing all create parameters on body", - requestBody: "name=my-instance&description=some%20description&plan=my-plan&team=my-team&tags.0=tsuru&tags.1=rpaas¶meters.flavors.0=orange¶meters.flavors.1=strawberry¶meters.flavors.2=blueberry", + requestBody: "name=my-instance&description=some%20description&plan=my-plan&team=my-team&tags=tsuru&tags=rpaas¶meters.flavors=orange,strawberry,blueberry", expectedCode: http.StatusCreated, manager: &fake.RpaasManager{ FakeCreateInstance: func(args rpaas.CreateArgs) error { @@ -55,11 +55,7 @@ func Test_serviceCreate(t *testing.T) { Team: "my-team", Tags: []string{"tsuru", "rpaas"}, Parameters: map[string]interface{}{ - "flavors": map[string]interface{}{ - "0": "orange", - "1": "strawberry", - "2": "blueberry", - }, + "flavors": "orange,strawberry,blueberry", }, } assert.Equal(t, expected, args) @@ -152,7 +148,7 @@ func Test_serviceUpdate(t *testing.T) { { name: "passing all update parameters on body", instance: "other-instance", - requestBody: "description=some%20description&plan=huge&team=team-one&tags.0=tag1&tags.1=tag2¶meters.flavors.0=orange¶meters.flavors.1=mango", + requestBody: "description=some%20description&plan=huge&team=team-one&tags=tag1&tags=tag2¶meters.flavors=orange,mango", expectedCode: http.StatusOK, manager: &fake.RpaasManager{ FakeUpdateInstance: func(instanceName string, args rpaas.UpdateInstanceArgs) error { @@ -163,10 +159,7 @@ func Test_serviceUpdate(t *testing.T) { Tags: []string{"tag1", "tag2"}, Team: "team-one", Parameters: map[string]interface{}{ - "flavors": map[string]interface{}{ - "0": "orange", - "1": "mango", - }, + "flavors": "orange,mango", }, }, args) assert.Equal(t, []string{"orange", "mango"}, args.Flavors()) diff --git a/internal/pkg/rpaas/manager.go b/internal/pkg/rpaas/manager.go index 3ead3e2c6..d330dd880 100644 --- a/internal/pkg/rpaas/manager.go +++ b/internal/pkg/rpaas/manager.go @@ -200,6 +200,12 @@ func getFlavors(params map[string]interface{}, tags []string) (flavors []string) return legacyGetFlavors(tags) } + flavorsString, ok := p.(string) + if ok { + flavors = strings.Split(flavorsString, ",") + return + } + flavorsParams, ok := p.(map[string]interface{}) if !ok { return