From f95820107d5f9fca521874e25d640354c3d3aa18 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch Date: Tue, 11 Feb 2020 11:40:36 -0800 Subject: [PATCH] support/http/httpdecode: add decoding of path params (#2245) ### What Add decoding of path parameters from requests into structs to the `httpdecode` package in a new `DecodePath` function and into the existing `Decode` function. ### Why We use httpdecode to decode requests into structs and this works really well for JSON and form bodys, but we have the odd endpoint that also has request parameters provided via the path string. An example of this is where an endpoint is for an account. In some situations it might be convenient to use middleware to pick out those path parameters earlier on and provide it in the context. In other situations, which this PR addresses, it can keep our code more succinct if we include those path parameters into the request object that we're decoding the entire request into. The hope is we can take this code into the `support/render/httpjson` package by having that package use `httpdecode.Decode` so that it is possible to capture all the request input in a single object for the handler. This PR is a step towards that but that benefit won't be realized yet. ### Important Note This change includes an update to the version of chi that is in use. Version 4 of chi allows for retrieving the context safely even if it is not present. --- go.list | 2 +- go.mod | 2 +- go.sum | 4 +- support/http/httpdecode/httpdecode.go | 35 +++++- support/http/httpdecode/httpdecode_test.go | 131 +++++++++++++++++++++ 5 files changed, 166 insertions(+), 8 deletions(-) diff --git a/go.list b/go.list index 109f178066..7e03ac8947 100644 --- a/go.list +++ b/go.list @@ -29,7 +29,7 @@ github.com/fortytw2/leaktest v1.3.0 github.com/fsnotify/fsnotify v1.4.7 github.com/gavv/monotime v0.0.0-20161010190848-47d58efa6955 github.com/getsentry/raven-go v0.0.0-20160805001729-c9d3cc542ad1 -github.com/go-chi/chi v3.1.5+incompatible +github.com/go-chi/chi v4.0.3+incompatible github.com/go-errors/errors v0.0.0-20150906023321-a41850380601 github.com/go-kit/kit v0.8.0 github.com/go-logfmt/logfmt v0.3.0 diff --git a/go.mod b/go.mod index c94cbfcbaa..98f755b351 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/fatih/structs v1.0.0 // indirect github.com/gavv/monotime v0.0.0-20161010190848-47d58efa6955 // indirect github.com/getsentry/raven-go v0.0.0-20160805001729-c9d3cc542ad1 - github.com/go-chi/chi v3.1.5+incompatible + github.com/go-chi/chi v4.0.3+incompatible github.com/go-errors/errors v0.0.0-20150906023321-a41850380601 github.com/gobuffalo/packr v1.12.1 // indirect github.com/goji/httpauth v0.0.0-20160601135302-2da839ab0f4d diff --git a/go.sum b/go.sum index 3e3b4ae461..c7b8409c15 100644 --- a/go.sum +++ b/go.sum @@ -50,8 +50,8 @@ github.com/gavv/monotime v0.0.0-20161010190848-47d58efa6955 h1:gmtGRvSexPU4B1T/y github.com/gavv/monotime v0.0.0-20161010190848-47d58efa6955/go.mod h1:vmp8DIyckQMXOPl0AQVHt+7n5h7Gb7hS6CUydiV8QeA= github.com/getsentry/raven-go v0.0.0-20160805001729-c9d3cc542ad1 h1:qIqziX4EA/OBdmMgtaqdKBWWOZIfyXYClCoa56NgVEk= github.com/getsentry/raven-go v0.0.0-20160805001729-c9d3cc542ad1/go.mod h1:KungGk8q33+aIAZUIVWZDr2OfAEBsO49PX4NzFV5kcQ= -github.com/go-chi/chi v3.1.5+incompatible h1:TcUFIZz4n9dleJjD6lxsoqPJDiEVQEWcNjj3mevAEFQ= -github.com/go-chi/chi v3.1.5+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxmMeXJVKy9tTv1XzQ= +github.com/go-chi/chi v4.0.3+incompatible h1:gakN3pDJnzZN5jqFV2TEdF66rTfKeITyR8qu6ekICEY= +github.com/go-chi/chi v4.0.3+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxmMeXJVKy9tTv1XzQ= github.com/go-errors/errors v0.0.0-20150906023321-a41850380601 h1:jxTbmDuqQUTI6MscgbqB39vtxGfr2fi61nYIcFQUnlE= github.com/go-errors/errors v0.0.0-20150906023321-a41850380601/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= diff --git a/support/http/httpdecode/httpdecode.go b/support/http/httpdecode/httpdecode.go index 919f86561f..883684ea22 100644 --- a/support/http/httpdecode/httpdecode.go +++ b/support/http/httpdecode/httpdecode.go @@ -5,10 +5,33 @@ import ( "mime" "net/http" + "github.com/go-chi/chi" "github.com/gorilla/schema" "github.com/stellar/go/support/errors" ) +// DecodePath decodes parameters from the path in a request used with the +// github.com/go-chi/chi muxing module. +func DecodePath(r *http.Request, v interface{}) error { + rctx := chi.RouteContext(r.Context()) + if rctx == nil { + return nil + } + params := rctx.URLParams + paramMap := map[string][]string{} + for i, k := range params.Keys { + if i >= len(params.Values) { + break + } + v := params.Values[i] + paramMap[k] = append(paramMap[k], v) + } + dec := schema.NewDecoder() + dec.SetAliasTag("path") + dec.IgnoreUnknownKeys(true) + return dec.Decode(v, paramMap) +} + // DecodeQuery decodes the query string from r into v. func DecodeQuery(r *http.Request, v interface{}) error { dec := schema.NewDecoder() @@ -61,7 +84,7 @@ func DecodeForm(r *http.Request, v interface{}) error { } // Decode decodes form URL encoded requests and JSON requests from r into v. -// Also decodes query parameters. +// Also decodes path (chi only) and query parameters. // // The requests Content-Type header informs if the request should be decoded // using a form URL encoded decoder or using a JSON decoder. @@ -75,10 +98,14 @@ func DecodeForm(r *http.Request, v interface{}) error { // An error is returned if the Content-Type cannot be parsed by a mime // media-type parser. // -// See DecodeQuery, DecodeForm and DecodeJSON for details about the types of -// errors that may occur. +// See DecodePath, DecodeQuery, DecodeForm and DecodeJSON for details about +// the types of errors that may occur. func Decode(r *http.Request, v interface{}) error { - err := DecodeQuery(r, v) + err := DecodePath(r, v) + if err != nil { + return errors.Wrap(err, "path params could not be parsed") + } + err = DecodeQuery(r, v) if err != nil { return errors.Wrap(err, "query could not be parsed") } diff --git a/support/http/httpdecode/httpdecode_test.go b/support/http/httpdecode/httpdecode_test.go index 8a214eb982..938ac3fea2 100644 --- a/support/http/httpdecode/httpdecode_test.go +++ b/support/http/httpdecode/httpdecode_test.go @@ -3,13 +3,58 @@ package httpdecode import ( "bufio" "net/http" + "net/http/httptest" "strings" "testing" + "github.com/go-chi/chi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestDecodePath_valid(t *testing.T) { + pathDecoded := struct { + Foo string `path:"foo"` + }{} + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := DecodePath(r, &pathDecoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + r, _ := http.NewRequest("POST", "/path/bar/path", nil) + mux.ServeHTTP(w, r) + + assert.Equal(t, "bar", pathDecoded.Foo) +} + +func TestDecodePath_empty(t *testing.T) { + pathDecoded := struct { + Foo string `path:"foo"` + }{} + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := DecodePath(r, &pathDecoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + r, _ := http.NewRequest("POST", "/path//path", nil) + mux.ServeHTTP(w, r) + + assert.Equal(t, "", pathDecoded.Foo) +} + +func TestDecodePath_notUsingChi(t *testing.T) { + pathDecoded := struct { + Foo string `path:"foo"` + }{} + r, _ := http.NewRequest("POST", "/path/bar/path", nil) + err := DecodePath(r, &pathDecoded) + require.NoError(t, err) + + assert.Equal(t, "", pathDecoded.Foo) +} + func TestDecodeQuery_valid(t *testing.T) { q := "foo=bar&list=a&list=b&enc=%2B+-%2F" r, _ := http.NewRequest("POST", "/?"+q, nil) @@ -323,3 +368,89 @@ func TestDecode_validJSONAndQuery(t *testing.T) { assert.Equal(t, "bar", bodyDecoded.FooName) assert.Equal(t, "boo", bodyDecoded.FarName) } + +func TestDecode_validFormAndPath(t *testing.T) { + decoded := struct { + FooName string `json:"foo" form:"foo"` + FarName string `path:"foo"` + }{} + + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := Decode(r, &decoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + body := `foo=bar` + r, _ := http.NewRequest("POST", "/path/boo/path", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + mux.ServeHTTP(w, r) + + assert.Equal(t, "bar", decoded.FooName) + assert.Equal(t, "boo", decoded.FarName) +} + +func TestDecode_validJSONAndPath(t *testing.T) { + decoded := struct { + FooName string `json:"foo" form:"foo"` + FarName string `path:"foo"` + }{} + + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := Decode(r, &decoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + body := `{"foo":"bar"}` + r, _ := http.NewRequest("POST", "/path/boo/path", strings.NewReader(body)) + mux.ServeHTTP(w, r) + + assert.Equal(t, "bar", decoded.FooName) + assert.Equal(t, "boo", decoded.FarName) +} + +func TestDecode_validFormAndPathAndQuery(t *testing.T) { + decoded := struct { + FooName string `json:"foo" form:"foo"` + FarName string `path:"foo"` + QueryName string `query:"q"` + }{} + + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := Decode(r, &decoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + body := `foo=bar` + r, _ := http.NewRequest("POST", "/path/boo/path?q=search+value", strings.NewReader(body)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + mux.ServeHTTP(w, r) + + assert.Equal(t, "bar", decoded.FooName) + assert.Equal(t, "boo", decoded.FarName) + assert.Equal(t, "search value", decoded.QueryName) +} + +func TestDecode_validJSONAndPathAndQuery(t *testing.T) { + decoded := struct { + FooName string `json:"foo" form:"foo"` + FarName string `path:"foo"` + QueryName string `query:"q"` + }{} + + mux := chi.NewMux() + mux.Post("/path/{foo}/path", func(w http.ResponseWriter, r *http.Request) { + err := Decode(r, &decoded) + require.NoError(t, err) + }) + w := httptest.NewRecorder() + body := `{"foo":"bar"}` + r, _ := http.NewRequest("POST", "/path/boo/path?q=search+value", strings.NewReader(body)) + mux.ServeHTTP(w, r) + + assert.Equal(t, "bar", decoded.FooName) + assert.Equal(t, "boo", decoded.FarName) + assert.Equal(t, "search value", decoded.QueryName) +}