diff --git a/README.md b/README.md index 03ad4dca..deba54f4 100644 --- a/README.md +++ b/README.md @@ -42,11 +42,14 @@ For older versions, please use the latest v3 tag. ## Benchmarks -Date: 2018/03/15
+Date: 2020/11/11
Source: https://github.com/vishr/web-framework-benchmark
Lower is better! - + + + +The benchmarks above were run on an Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz ## [Guide](https://echo.labstack.com/guide) diff --git a/bind.go b/bind.go index f8914743..c7be242b 100644 --- a/bind.go +++ b/bind.go @@ -30,10 +30,8 @@ type ( } ) -// Bind implements the `Binder#Bind` function. -func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) { - req := c.Request() - +// BindPathParams binds path params to bindable object +func (b *DefaultBinder) BindPathParams(c Context, i interface{}) error { names := c.ParamNames() values := c.ParamValues() params := map[string][]string{} @@ -43,12 +41,28 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) { if err := b.bindData(i, params, "param"); err != nil { return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) } - if err = b.bindData(i, c.QueryParams(), "query"); err != nil { + return nil +} + +// BindQueryParams binds query params to bindable object +func (b *DefaultBinder) BindQueryParams(c Context, i interface{}) error { + if err := b.bindData(i, c.QueryParams(), "query"); err != nil { return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) } + return nil +} + +// BindBody binds request body contents to bindable object +// NB: then binding forms take note that this implementation uses standard library form parsing +// which parses form data from BOTH URL and BODY if content type is not MIMEMultipartForm +// See non-MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseForm +// See MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseMultipartForm +func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) { + req := c.Request() if req.ContentLength == 0 { return } + ctype := req.Header.Get(HeaderContentType) switch { case strings.HasPrefix(ctype, MIMEApplicationJSON): @@ -80,7 +94,18 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) { default: return ErrUnsupportedMediaType } - return + return nil +} + +// Bind implements the `Binder#Bind` function. +func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) { + if err := b.BindPathParams(c, i); err != nil { + return err + } + if err = b.BindQueryParams(c, i); err != nil { + return err + } + return b.BindBody(c, i) } func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag string) error { diff --git a/bind_test.go b/bind_test.go index b9fb9de3..60c2f9e0 100644 --- a/bind_test.go +++ b/bind_test.go @@ -553,3 +553,305 @@ func testBindError(assert *assert.Assertions, r io.Reader, ctype string, expecte } } } + +func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) { + // tests to check binding behaviour when multiple sources path params, query params and request body are in use + // binding is done in steps and one source could overwrite previous source binded data + // these tests are to document this behaviour and detect further possible regressions when bind implementation is changed + + type Node struct { + ID int `json:"id"` + Node string `json:"node"` + } + + var testCases = []struct { + name string + givenURL string + givenContent io.Reader + givenMethod string + whenBindTarget interface{} + whenNoPathParams bool + expect interface{} + expectError string + }{ + { + name: "ok, POST bind to struct with: path param + query param + empty body", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{"id": 1}`), + expect: &Node{ID: 1, Node: "xxx"}, // in current implementation query params has higher priority than path params + }, + { + name: "ok, POST bind to struct with: path param + empty body", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint", + givenContent: strings.NewReader(`{"id": 1}`), + expect: &Node{ID: 1, Node: "real_node"}, + }, + { + name: "ok, POST bind to struct with path + query + body = body has priority", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`), + expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority + }, + { + name: "nok, POST body bind failure", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{`), + expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target + expectError: "code=400, message=unexpected EOF, internal=unexpected EOF", + }, + { + name: "nok, GET body bind failure - trying to bind json array to struct", + givenMethod: http.MethodGet, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`[{"id": 1}]`), + expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target + expectError: "code=400, message=Unmarshal type error: expected=echo.Node, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Node", + }, + { // binding query params interferes with body. b.BindBody() should be used to bind only body to slice + name: "nok, GET query params bind failure - trying to bind json array to slice", + givenMethod: http.MethodGet, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`[{"id": 1}]`), + whenNoPathParams: true, + whenBindTarget: &[]Node{}, + expect: &[]Node{}, + expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct", + }, + { // binding path params interferes with body. b.BindBody() should be used to bind only body to slice + name: "nok, GET path params bind failure - trying to bind json array to slice", + givenMethod: http.MethodGet, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`[{"id": 1}]`), + whenBindTarget: &[]Node{}, + expect: &[]Node{}, + expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct", + }, + { + name: "ok, GET body bind json array to slice", + givenMethod: http.MethodGet, + givenURL: "/api/real_node/endpoint", + givenContent: strings.NewReader(`[{"id": 1}]`), + whenNoPathParams: true, + whenBindTarget: &[]Node{}, + expect: &[]Node{{ID: 1, Node: ""}}, + expectError: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := New() + // assume route we are testing is "/api/:node/endpoint?some_query_params=here" + req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + if !tc.whenNoPathParams { + c.SetParamNames("node") + c.SetParamValues("real_node") + } + + var bindTarget interface{} + if tc.whenBindTarget != nil { + bindTarget = tc.whenBindTarget + } else { + bindTarget = &Node{} + } + b := new(DefaultBinder) + + err := b.Bind(bindTarget, c) + if tc.expectError != "" { + assert.EqualError(t, err, tc.expectError) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expect, bindTarget) + }) + } +} + +func TestDefaultBinder_BindBody(t *testing.T) { + // tests to check binding behaviour when multiple sources path params, query params and request body are in use + // generally when binding from request body - URL and path params are ignored - unless form is being binded. + // these tests are to document this behaviour and detect further possible regressions when bind implementation is changed + + type Node struct { + ID int `json:"id" xml:"id"` + Node string `json:"node" xml:"node"` + } + type Nodes struct { + Nodes []Node `xml:"node" form:"node"` + } + + var testCases = []struct { + name string + givenURL string + givenContent io.Reader + givenMethod string + givenContentType string + whenNoPathParams bool + whenBindTarget interface{} + expect interface{} + expectError string + }{ + { + name: "ok, JSON POST bind to struct with: path + query + empty field in body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`{"id": 1}`), + expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body + }, + { + name: "ok, JSON POST bind to struct with: path + query + body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`), + expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority + }, + { + name: "ok, JSON POST body bind json array to slice (has matching path/query params)", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`[{"id": 1}]`), + whenNoPathParams: true, + whenBindTarget: &[]Node{}, + expect: &[]Node{{ID: 1, Node: ""}}, + expectError: "", + }, + { // rare case as GET is not usually used to send request body + name: "ok, JSON GET bind to struct with: path + query + empty field in body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodGet, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`{"id": 1}`), + expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body + }, + { // rare case as GET is not usually used to send request body + name: "ok, JSON GET bind to struct with: path + query + body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodGet, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`), + expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority + }, + { + name: "nok, JSON POST body bind failure", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationJSON, + givenContent: strings.NewReader(`{`), + expect: &Node{ID: 0, Node: ""}, + expectError: "code=400, message=unexpected EOF, internal=unexpected EOF", + }, + { + name: "ok, XML POST bind to struct with: path + query + empty body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationXML, + givenContent: strings.NewReader(`1yyy`), + expect: &Node{ID: 1, Node: "yyy"}, + }, + { + name: "ok, XML POST bind array to slice with: path + query + body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationXML, + givenContent: strings.NewReader(`1yyy`), + whenBindTarget: &Nodes{}, + expect: &Nodes{Nodes: []Node{{ID: 1, Node: "yyy"}}}, + }, + { + name: "nok, XML POST bind failure", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationXML, + givenContent: strings.NewReader(`<`), + expect: &Node{ID: 0, Node: ""}, + expectError: "code=400, message=Syntax error: line=1, error=XML syntax error on line 1: unexpected EOF, internal=XML syntax error on line 1: unexpected EOF", + }, + { + name: "ok, FORM POST bind to struct with: path + query + empty body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationForm, + givenContent: strings.NewReader(`id=1&node=yyy`), + expect: &Node{ID: 1, Node: "yyy"}, + }, + { + // NB: form values are taken from BOTH body and query for POST/PUT/PATCH by standard library implementation + // See: https://golang.org/pkg/net/http/#Request.ParseForm + name: "ok, FORM POST bind to struct with: path + query + empty field in body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMEApplicationForm, + givenContent: strings.NewReader(`id=1`), + expect: &Node{ID: 1, Node: "xxx"}, + }, + { + // NB: form values are taken from query by standard library implementation + // See: https://golang.org/pkg/net/http/#Request.ParseForm + name: "ok, FORM GET bind to struct with: path + query + empty field in body", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodGet, + givenContentType: MIMEApplicationForm, + givenContent: strings.NewReader(`id=1`), + expect: &Node{ID: 0, Node: "xxx"}, // 'xxx' is taken from URL and body is not used with GET by implementation + }, + { + name: "nok, unsupported content type", + givenURL: "/api/real_node/endpoint?node=xxx", + givenMethod: http.MethodPost, + givenContentType: MIMETextPlain, + givenContent: strings.NewReader(``), + expect: &Node{ID: 0, Node: ""}, + expectError: "code=415, message=Unsupported Media Type", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := New() + // assume route we are testing is "/api/:node/endpoint?some_query_params=here" + req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent) + switch tc.givenContentType { + case MIMEApplicationXML: + req.Header.Set(HeaderContentType, MIMEApplicationXML) + case MIMEApplicationForm: + req.Header.Set(HeaderContentType, MIMEApplicationForm) + case MIMEApplicationJSON: + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + } + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + if !tc.whenNoPathParams { + c.SetParamNames("node") + c.SetParamValues("real_node") + } + + var bindTarget interface{} + if tc.whenBindTarget != nil { + bindTarget = tc.whenBindTarget + } else { + bindTarget = &Node{} + } + b := new(DefaultBinder) + + err := b.BindBody(c, bindTarget) + if tc.expectError != "" { + assert.EqualError(t, err, tc.expectError) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expect, bindTarget) + }) + } +} diff --git a/context.go b/context.go index 8ba98477..380a52e1 100644 --- a/context.go +++ b/context.go @@ -314,7 +314,19 @@ func (c *context) ParamNames() []string { func (c *context) SetParamNames(names ...string) { c.pnames = names - *c.echo.maxParam = len(names) + + l := len(names) + if *c.echo.maxParam < l { + *c.echo.maxParam = l + } + + if len(c.pvalues) < l { + // Keeping the old pvalues just for backward compatibility, but it sounds that doesn't make sense to keep them, + // probably those values will be overriden in a Context#SetParamValues + newPvalues := make([]string, l) + copy(newPvalues, c.pvalues) + c.pvalues = newPvalues + } } func (c *context) ParamValues() []string { @@ -322,7 +334,15 @@ func (c *context) ParamValues() []string { } func (c *context) SetParamValues(values ...string) { - c.pvalues = values + // NOTE: Don't just set c.pvalues = values, because it has to have length c.echo.maxParam at all times + // It will brake the Router#Find code + limit := len(values) + if limit > *c.echo.maxParam { + limit = *c.echo.maxParam + } + for i := 0; i < limit; i++ { + c.pvalues[i] = values[i] + } } func (c *context) QueryParam(name string) string { diff --git a/context_test.go b/context_test.go index 0044bf87..417d4a74 100644 --- a/context_test.go +++ b/context_test.go @@ -517,6 +517,40 @@ func TestContextGetAndSetParam(t *testing.T) { }) } +// Issue #1655 +func TestContextSetParamNamesShouldUpdateEchoMaxParam(t *testing.T) { + assert := testify.New(t) + + e := New() + assert.Equal(0, *e.maxParam) + + expectedOneParam := []string{"one"} + expectedTwoParams := []string{"one", "two"} + expectedThreeParams := []string{"one", "two", ""} + expectedABCParams := []string{"A", "B", "C"} + + c := e.NewContext(nil, nil) + c.SetParamNames("1", "2") + c.SetParamValues(expectedTwoParams...) + assert.Equal(2, *e.maxParam) + assert.EqualValues(expectedTwoParams, c.ParamValues()) + + c.SetParamNames("1") + assert.Equal(2, *e.maxParam) + // Here for backward compatibility the ParamValues remains as they are + assert.EqualValues(expectedOneParam, c.ParamValues()) + + c.SetParamNames("1", "2", "3") + assert.Equal(3, *e.maxParam) + // Here for backward compatibility the ParamValues remains as they are, but the len is extended to e.maxParam + assert.EqualValues(expectedThreeParams, c.ParamValues()) + + c.SetParamValues("A", "B", "C", "D") + assert.Equal(3, *e.maxParam) + // Here D shouldn't be returned + assert.EqualValues(expectedABCParams, c.ParamValues()) +} + func TestContextFormValue(t *testing.T) { f := make(url.Values) f.Set("name", "Jon Snow") diff --git a/echo.go b/echo.go index c4399801..d284ff39 100644 --- a/echo.go +++ b/echo.go @@ -49,7 +49,6 @@ import ( "net/http" "net/url" "os" - "path" "path/filepath" "reflect" "runtime" @@ -92,6 +91,7 @@ type ( Renderer Renderer Logger Logger IPExtractor IPExtractor + ListenerNetwork string } // Route contains a handler and information for matching against requests. @@ -281,6 +281,7 @@ var ( ErrInvalidRedirectCode = errors.New("invalid redirect status code") ErrCookieNotFound = errors.New("cookie not found") ErrInvalidCertOrKeyType = errors.New("invalid cert or key type, must be string or []byte") + ErrInvalidListenerNetwork = errors.New("invalid listener network") ) // Error handlers @@ -302,9 +303,10 @@ func New() (e *Echo) { AutoTLSManager: autocert.Manager{ Prompt: autocert.AcceptTOS, }, - Logger: log.New("echo"), - colorer: color.New(), - maxParam: new(int), + Logger: log.New("echo"), + colorer: color.New(), + maxParam: new(int), + ListenerNetwork: "tcp", } e.Server.Handler = e e.TLSServer.Handler = e @@ -483,7 +485,7 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl return err } - name := filepath.Join(root, path.Clean("/"+p)) // "/"+ for security + name := filepath.Join(root, filepath.Clean("/"+p)) // "/"+ for security fi, err := os.Stat(name) if err != nil { // The access path does not exist @@ -714,7 +716,7 @@ func (e *Echo) StartServer(s *http.Server) (err error) { if s.TLSConfig == nil { if e.Listener == nil { - e.Listener, err = newListener(s.Addr) + e.Listener, err = newListener(s.Addr, e.ListenerNetwork) if err != nil { return err } @@ -725,7 +727,7 @@ func (e *Echo) StartServer(s *http.Server) (err error) { return s.Serve(e.Listener) } if e.TLSListener == nil { - l, err := newListener(s.Addr) + l, err := newListener(s.Addr, e.ListenerNetwork) if err != nil { return err } @@ -754,7 +756,7 @@ func (e *Echo) StartH2CServer(address string, h2s *http2.Server) (err error) { } if e.Listener == nil { - e.Listener, err = newListener(s.Addr) + e.Listener, err = newListener(s.Addr, e.ListenerNetwork) if err != nil { return err } @@ -875,8 +877,11 @@ func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) { return } -func newListener(address string) (*tcpKeepAliveListener, error) { - l, err := net.Listen("tcp", address) +func newListener(address, network string) (*tcpKeepAliveListener, error) { + if network != "tcp" && network != "tcp4" && network != "tcp6" { + return nil, ErrInvalidListenerNetwork + } + l, err := net.Listen(network, address) if err != nil { return nil, err } diff --git a/echo_test.go b/echo_test.go index 71dc1ac0..a6071e12 100644 --- a/echo_test.go +++ b/echo_test.go @@ -4,6 +4,7 @@ import ( "bytes" stdContext "context" "errors" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -59,45 +60,105 @@ func TestEcho(t *testing.T) { } func TestEchoStatic(t *testing.T) { - e := New() + var testCases = []struct { + name string + givenPrefix string + givenRoot string + whenURL string + expectStatus int + expectHeaderLocation string + expectBodyStartsWith string + }{ + { + name: "ok", + givenPrefix: "/images", + givenRoot: "_fixture/images", + whenURL: "/images/walle.png", + expectStatus: http.StatusOK, + expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}), + }, + { + name: "No file", + givenPrefix: "/images", + givenRoot: "_fixture/scripts", + whenURL: "/images/bolt.png", + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "Directory", + givenPrefix: "/images", + givenRoot: "_fixture/images", + whenURL: "/images/", + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "Directory Redirect", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/folder", + expectStatus: http.StatusMovedPermanently, + expectHeaderLocation: "/folder/", + expectBodyStartsWith: "", + }, + { + name: "Directory with index.html", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/", + expectStatus: http.StatusOK, + expectBodyStartsWith: "", + }, + { + name: "Sub-directory with index.html", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/folder/", + expectStatus: http.StatusOK, + expectBodyStartsWith: "", + }, + { + name: "do not allow directory traversal (backslash - windows separator)", + givenPrefix: "/", + givenRoot: "_fixture/", + whenURL: `/..\\middleware/basic_auth.go`, + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "do not allow directory traversal (slash - unix separator)", + givenPrefix: "/", + givenRoot: "_fixture/", + whenURL: `/../middleware/basic_auth.go`, + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + } - assert := assert.New(t) - - // OK - e.Static("/images", "_fixture/images") - c, b := request(http.MethodGet, "/images/walle.png", e) - assert.Equal(http.StatusOK, c) - assert.NotEmpty(b) - - // No file - e.Static("/images", "_fixture/scripts") - c, _ = request(http.MethodGet, "/images/bolt.png", e) - assert.Equal(http.StatusNotFound, c) - - // Directory - e.Static("/images", "_fixture/images") - c, _ = request(http.MethodGet, "/images/", e) - assert.Equal(http.StatusNotFound, c) - - // Directory Redirect - e.Static("/", "_fixture") - req := httptest.NewRequest(http.MethodGet, "/folder", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - assert.Equal(http.StatusMovedPermanently, rec.Code) - assert.Equal("/folder/", rec.HeaderMap["Location"][0]) - - // Directory with index.html - e.Static("/", "_fixture") - c, r := request(http.MethodGet, "/", e) - assert.Equal(http.StatusOK, c) - assert.Equal(true, strings.HasPrefix(r, "")) - - // Sub-directory with index.html - c, r = request(http.MethodGet, "/folder/", e) - assert.Equal(http.StatusOK, c) - assert.Equal(true, strings.HasPrefix(r, "")) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := New() + e.Static(tc.givenPrefix, tc.givenRoot) + req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, tc.expectStatus, rec.Code) + body := rec.Body.String() + if tc.expectBodyStartsWith != "" { + assert.True(t, strings.HasPrefix(body, tc.expectBodyStartsWith)) + } else { + assert.Equal(t, "", body) + } + if tc.expectHeaderLocation != "" { + assert.Equal(t, tc.expectHeaderLocation, rec.Result().Header["Location"][0]) + } else { + _, ok := rec.Result().Header["Location"] + assert.False(t, ok) + } + }) + } } func TestEchoFile(t *testing.T) { @@ -658,6 +719,69 @@ func TestEchoShutdown(t *testing.T) { assert.Equal(t, err.Error(), "http: Server closed") } +var listenerNetworkTests = []struct { + test string + network string + address string +}{ + {"tcp ipv4 address", "tcp", "127.0.0.1:1323"}, + {"tcp ipv6 address", "tcp", "[::1]:1323"}, + {"tcp4 ipv4 address", "tcp4", "127.0.0.1:1323"}, + {"tcp6 ipv6 address", "tcp6", "[::1]:1323"}, +} + +func TestEchoListenerNetwork(t *testing.T) { + for _, tt := range listenerNetworkTests { + t.Run(tt.test, func(t *testing.T) { + e := New() + e.ListenerNetwork = tt.network + + // HandlerFunc + e.GET("/ok", func(c Context) error { + return c.String(http.StatusOK, "OK") + }) + + errCh := make(chan error) + + go func() { + errCh <- e.Start(tt.address) + }() + + time.Sleep(200 * time.Millisecond) + + if resp, err := http.Get(fmt.Sprintf("http://%s/ok", tt.address)); err == nil { + defer resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + if body, err := ioutil.ReadAll(resp.Body); err == nil { + assert.Equal(t, "OK", string(body)) + } else { + assert.Fail(t, err.Error()) + } + + } else { + assert.Fail(t, err.Error()) + } + + if err := e.Close(); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestEchoListenerNetworkInvalid(t *testing.T) { + e := New() + e.ListenerNetwork = "unix" + + // HandlerFunc + e.GET("/ok", func(c Context) error { + return c.String(http.StatusOK, "OK") + }) + + assert.Equal(t, ErrInvalidListenerNetwork, e.Start(":1323")) +} + func TestEchoReverse(t *testing.T) { assert := assert.New(t) diff --git a/middleware/request_id_test.go b/middleware/request_id_test.go index 30eecdef..86eec8c3 100644 --- a/middleware/request_id_test.go +++ b/middleware/request_id_test.go @@ -31,3 +31,20 @@ func TestRequestID(t *testing.T) { h(c) assert.Equal(t, rec.Header().Get(echo.HeaderXRequestID), "customGenerator") } + +func TestRequestID_IDNotAltered(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Add(echo.HeaderXRequestID, "") + + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + handler := func(c echo.Context) error { + return c.String(http.StatusOK, "test") + } + + rid := RequestIDWithConfig(RequestIDConfig{}) + h := rid(handler) + _ = h(c) + assert.Equal(t, rec.Header().Get(echo.HeaderXRequestID), "") +} diff --git a/middleware/static.go b/middleware/static.go index 58b7890a..ae79cb5f 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -167,7 +167,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { if err != nil { return } - name := filepath.Join(config.Root, path.Clean("/"+p)) // "/"+ for security + name := filepath.Join(config.Root, filepath.Clean("/"+p)) // "/"+ for security if config.IgnoreBase { routePath := path.Base(strings.TrimRight(c.Path(), "/*")) diff --git a/router.go b/router.go index 4c3898c4..749dbf4f 100644 --- a/router.go +++ b/router.go @@ -14,14 +14,16 @@ type ( echo *Echo } node struct { - kind kind - label byte - prefix string - parent *node - children children - ppath string - pnames []string - methodHandler *methodHandler + kind kind + label byte + prefix string + parent *node + staticChildrens children + ppath string + pnames []string + methodHandler *methodHandler + paramChildren *node + anyChildren *node } kind uint8 children []*node @@ -44,6 +46,9 @@ const ( skind kind = iota pkind akind + + paramLabel = byte(':') + anyLabel = byte('*') ) // NewRouter returns a new Router instance. @@ -134,23 +139,32 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string } } else if l < pl { // Split node - n := newNode(cn.kind, cn.prefix[l:], cn, cn.children, cn.methodHandler, cn.ppath, cn.pnames) + n := newNode(cn.kind, cn.prefix[l:], cn, cn.staticChildrens, cn.methodHandler, cn.ppath, cn.pnames, cn.paramChildren, cn.anyChildren) // Update parent path for all children to new node - for _, child := range cn.children { + for _, child := range cn.staticChildrens { child.parent = n } + if cn.paramChildren != nil { + cn.paramChildren.parent = n + } + if cn.anyChildren != nil { + cn.anyChildren.parent = n + } // Reset parent node cn.kind = skind cn.label = cn.prefix[0] cn.prefix = cn.prefix[:l] - cn.children = nil + cn.staticChildrens = nil cn.methodHandler = new(methodHandler) cn.ppath = "" cn.pnames = nil + cn.paramChildren = nil + cn.anyChildren = nil - cn.addChild(n) + // Only Static children could reach here + cn.addStaticChild(n) if l == sl { // At parent node @@ -160,9 +174,10 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string cn.pnames = pnames } else { // Create child node - n = newNode(t, search[l:], cn, nil, new(methodHandler), ppath, pnames) + n = newNode(t, search[l:], cn, nil, new(methodHandler), ppath, pnames, nil, nil) n.addHandler(method, h) - cn.addChild(n) + // Only Static children could reach here + cn.addStaticChild(n) } } else if l < sl { search = search[l:] @@ -173,9 +188,16 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string continue } // Create child node - n := newNode(t, search, cn, nil, new(methodHandler), ppath, pnames) + n := newNode(t, search, cn, nil, new(methodHandler), ppath, pnames, nil, nil) n.addHandler(method, h) - cn.addChild(n) + switch t { + case skind: + cn.addStaticChild(n) + case pkind: + cn.paramChildren = n + case akind: + cn.anyChildren = n + } } else { // Node already exists if h != nil { @@ -190,34 +212,27 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string } } -func newNode(t kind, pre string, p *node, c children, mh *methodHandler, ppath string, pnames []string) *node { +func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, ppath string, pnames []string, paramChildren, anyChildren *node) *node { return &node{ - kind: t, - label: pre[0], - prefix: pre, - parent: p, - children: c, - ppath: ppath, - pnames: pnames, - methodHandler: mh, + kind: t, + label: pre[0], + prefix: pre, + parent: p, + staticChildrens: sc, + ppath: ppath, + pnames: pnames, + methodHandler: mh, + paramChildren: paramChildren, + anyChildren: anyChildren, } } -func (n *node) addChild(c *node) { - n.children = append(n.children, c) +func (n *node) addStaticChild(c *node) { + n.staticChildrens = append(n.staticChildrens, c) } -func (n *node) findChild(l byte, t kind) *node { - for _, c := range n.children { - if c.label == l && c.kind == t { - return c - } - } - return nil -} - -func (n *node) findChildWithLabel(l byte) *node { - for _, c := range n.children { +func (n *node) findStaticChild(l byte) *node { + for _, c := range n.staticChildrens { if c.label == l { return c } @@ -225,12 +240,18 @@ func (n *node) findChildWithLabel(l byte) *node { return nil } -func (n *node) findChildByKind(t kind) *node { - for _, c := range n.children { - if c.kind == t { +func (n *node) findChildWithLabel(l byte) *node { + for _, c := range n.staticChildrens { + if c.label == l { return c } } + if l == paramLabel { + return n.paramChildren + } + if l == anyLabel { + return n.anyChildren + } return nil } @@ -356,7 +377,7 @@ func (r *Router) Find(method, path string, c Context) { // Attempt to go back up the tree on no matching prefix or no remaining search if l != pl || search == "" { // Handle special case of trailing slash route with existing any route (see #1526) - if path[len(path)-1] == '/' && cn.findChildByKind(akind) != nil { + if path[len(path)-1] == '/' && cn.anyChildren != nil { goto Any } if nn == nil { // Issue #1348 @@ -372,7 +393,7 @@ func (r *Router) Find(method, path string, c Context) { } // Static node - if child = cn.findChild(search[0], skind); child != nil { + if child = cn.findStaticChild(search[0]); child != nil { // Save next if cn.prefix[len(cn.prefix)-1] == '/' { // Issue #623 nk = pkind @@ -385,7 +406,7 @@ func (r *Router) Find(method, path string, c Context) { Param: // Param node - if child = cn.findChildByKind(pkind); child != nil { + if child = cn.paramChildren; child != nil { // Issue #378 if len(pvalues) == n { continue @@ -410,7 +431,7 @@ func (r *Router) Find(method, path string, c Context) { Any: // Any node - if cn = cn.findChildByKind(akind); cn != nil { + if cn = cn.anyChildren; cn != nil { // If any node is found, use remaining path for pvalues pvalues[len(cn.pnames)-1] = search break @@ -424,7 +445,7 @@ func (r *Router) Find(method, path string, c Context) { search = ns np := nn.parent // Consider param route one level up only - if cn = nn.findChildByKind(pkind); cn != nil { + if cn = nn.paramChildren; cn != nil { pos := strings.IndexByte(ns, '/') if pos == -1 { // If no slash is remaining in search string set param value @@ -443,7 +464,7 @@ func (r *Router) Find(method, path string, c Context) { // No param route found, try to resolve nearest any route for { np = nn.parent - if cn = nn.findChildByKind(akind); cn != nil { + if cn = nn.anyChildren; cn != nil { break } if np == nil { @@ -474,7 +495,7 @@ func (r *Router) Find(method, path string, c Context) { // Dig further for any, might have an empty value for *, e.g. // serving a directory. Issue #207. - if cn = cn.findChildByKind(akind); cn == nil { + if cn = cn.anyChildren; cn == nil { return } if h := cn.findHandler(method); h != nil { diff --git a/router_test.go b/router_test.go index fca3a79b..aafc622c 100644 --- a/router_test.go +++ b/router_test.go @@ -175,8 +175,10 @@ var ( {"GET", "/authorizations", ""}, {"GET", "/authorizations/:id", ""}, {"POST", "/authorizations", ""}, - //{"PUT", "/authorizations/clients/:client_id", ""}, - //{"PATCH", "/authorizations/:id", ""}, + + {"PUT", "/authorizations/clients/:client_id", ""}, + {"PATCH", "/authorizations/:id", ""}, + {"DELETE", "/authorizations/:id", ""}, {"GET", "/applications/:client_id/tokens/:access_token", ""}, {"DELETE", "/applications/:client_id/tokens", ""}, @@ -198,7 +200,9 @@ var ( {"PUT", "/notifications", ""}, {"PUT", "/repos/:owner/:repo/notifications", ""}, {"GET", "/notifications/threads/:id", ""}, - //{"PATCH", "/notifications/threads/:id", ""}, + + {"PATCH", "/notifications/threads/:id", ""}, + {"GET", "/notifications/threads/:id/subscription", ""}, {"PUT", "/notifications/threads/:id/subscription", ""}, {"DELETE", "/notifications/threads/:id/subscription", ""}, @@ -221,11 +225,15 @@ var ( // Gists {"GET", "/users/:user/gists", ""}, {"GET", "/gists", ""}, - //{"GET", "/gists/public", ""}, - //{"GET", "/gists/starred", ""}, + + {"GET", "/gists/public", ""}, + {"GET", "/gists/starred", ""}, + {"GET", "/gists/:id", ""}, {"POST", "/gists", ""}, - //{"PATCH", "/gists/:id", ""}, + + {"PATCH", "/gists/:id", ""}, + {"PUT", "/gists/:id/star", ""}, {"DELETE", "/gists/:id/star", ""}, {"GET", "/gists/:id/star", ""}, @@ -237,11 +245,15 @@ var ( {"POST", "/repos/:owner/:repo/git/blobs", ""}, {"GET", "/repos/:owner/:repo/git/commits/:sha", ""}, {"POST", "/repos/:owner/:repo/git/commits", ""}, - //{"GET", "/repos/:owner/:repo/git/refs/*ref", ""}, + + {"GET", "/repos/:owner/:repo/git/refs/*ref", ""}, + {"GET", "/repos/:owner/:repo/git/refs", ""}, {"POST", "/repos/:owner/:repo/git/refs", ""}, - //{"PATCH", "/repos/:owner/:repo/git/refs/*ref", ""}, - //{"DELETE", "/repos/:owner/:repo/git/refs/*ref", ""}, + + {"PATCH", "/repos/:owner/:repo/git/refs/*ref", ""}, + {"DELETE", "/repos/:owner/:repo/git/refs/*ref", ""}, + {"GET", "/repos/:owner/:repo/git/tags/:sha", ""}, {"POST", "/repos/:owner/:repo/git/tags", ""}, {"GET", "/repos/:owner/:repo/git/trees/:sha", ""}, @@ -254,22 +266,32 @@ var ( {"GET", "/repos/:owner/:repo/issues", ""}, {"GET", "/repos/:owner/:repo/issues/:number", ""}, {"POST", "/repos/:owner/:repo/issues", ""}, - //{"PATCH", "/repos/:owner/:repo/issues/:number", ""}, + + {"PATCH", "/repos/:owner/:repo/issues/:number", ""}, + {"GET", "/repos/:owner/:repo/assignees", ""}, {"GET", "/repos/:owner/:repo/assignees/:assignee", ""}, {"GET", "/repos/:owner/:repo/issues/:number/comments", ""}, - //{"GET", "/repos/:owner/:repo/issues/comments", ""}, - //{"GET", "/repos/:owner/:repo/issues/comments/:id", ""}, + + {"GET", "/repos/:owner/:repo/issues/comments", ""}, + {"GET", "/repos/:owner/:repo/issues/comments/:id", ""}, + {"POST", "/repos/:owner/:repo/issues/:number/comments", ""}, - //{"PATCH", "/repos/:owner/:repo/issues/comments/:id", ""}, - //{"DELETE", "/repos/:owner/:repo/issues/comments/:id", ""}, + + {"PATCH", "/repos/:owner/:repo/issues/comments/:id", ""}, + {"DELETE", "/repos/:owner/:repo/issues/comments/:id", ""}, + {"GET", "/repos/:owner/:repo/issues/:number/events", ""}, - //{"GET", "/repos/:owner/:repo/issues/events", ""}, - //{"GET", "/repos/:owner/:repo/issues/events/:id", ""}, + + {"GET", "/repos/:owner/:repo/issues/events", ""}, + {"GET", "/repos/:owner/:repo/issues/events/:id", ""}, + {"GET", "/repos/:owner/:repo/labels", ""}, {"GET", "/repos/:owner/:repo/labels/:name", ""}, {"POST", "/repos/:owner/:repo/labels", ""}, - //{"PATCH", "/repos/:owner/:repo/labels/:name", ""}, + + {"PATCH", "/repos/:owner/:repo/labels/:name", ""}, + {"DELETE", "/repos/:owner/:repo/labels/:name", ""}, {"GET", "/repos/:owner/:repo/issues/:number/labels", ""}, {"POST", "/repos/:owner/:repo/issues/:number/labels", ""}, @@ -280,7 +302,9 @@ var ( {"GET", "/repos/:owner/:repo/milestones", ""}, {"GET", "/repos/:owner/:repo/milestones/:number", ""}, {"POST", "/repos/:owner/:repo/milestones", ""}, - //{"PATCH", "/repos/:owner/:repo/milestones/:number", ""}, + + {"PATCH", "/repos/:owner/:repo/milestones/:number", ""}, + {"DELETE", "/repos/:owner/:repo/milestones/:number", ""}, // Miscellaneous @@ -296,7 +320,9 @@ var ( {"GET", "/users/:user/orgs", ""}, {"GET", "/user/orgs", ""}, {"GET", "/orgs/:org", ""}, - //{"PATCH", "/orgs/:org", ""}, + + {"PATCH", "/orgs/:org", ""}, + {"GET", "/orgs/:org/members", ""}, {"GET", "/orgs/:org/members/:user", ""}, {"DELETE", "/orgs/:org/members/:user", ""}, @@ -307,7 +333,9 @@ var ( {"GET", "/orgs/:org/teams", ""}, {"GET", "/teams/:id", ""}, {"POST", "/orgs/:org/teams", ""}, - //{"PATCH", "/teams/:id", ""}, + + {"PATCH", "/teams/:id", ""}, + {"DELETE", "/teams/:id", ""}, {"GET", "/teams/:id/members", ""}, {"GET", "/teams/:id/members/:user", ""}, @@ -323,17 +351,22 @@ var ( {"GET", "/repos/:owner/:repo/pulls", ""}, {"GET", "/repos/:owner/:repo/pulls/:number", ""}, {"POST", "/repos/:owner/:repo/pulls", ""}, - //{"PATCH", "/repos/:owner/:repo/pulls/:number", ""}, + + {"PATCH", "/repos/:owner/:repo/pulls/:number", ""}, + {"GET", "/repos/:owner/:repo/pulls/:number/commits", ""}, {"GET", "/repos/:owner/:repo/pulls/:number/files", ""}, {"GET", "/repos/:owner/:repo/pulls/:number/merge", ""}, {"PUT", "/repos/:owner/:repo/pulls/:number/merge", ""}, {"GET", "/repos/:owner/:repo/pulls/:number/comments", ""}, - //{"GET", "/repos/:owner/:repo/pulls/comments", ""}, - //{"GET", "/repos/:owner/:repo/pulls/comments/:number", ""}, + + {"GET", "/repos/:owner/:repo/pulls/comments", ""}, + {"GET", "/repos/:owner/:repo/pulls/comments/:number", ""}, + {"PUT", "/repos/:owner/:repo/pulls/:number/comments", ""}, - //{"PATCH", "/repos/:owner/:repo/pulls/comments/:number", ""}, - //{"DELETE", "/repos/:owner/:repo/pulls/comments/:number", ""}, + + {"PATCH", "/repos/:owner/:repo/pulls/comments/:number", ""}, + {"DELETE", "/repos/:owner/:repo/pulls/comments/:number", ""}, // Repositories {"GET", "/user/repos", ""}, @@ -343,7 +376,9 @@ var ( {"POST", "/user/repos", ""}, {"POST", "/orgs/:org/repos", ""}, {"GET", "/repos/:owner/:repo", ""}, - //{"PATCH", "/repos/:owner/:repo", ""}, + + {"PATCH", "/repos/:owner/:repo", ""}, + {"GET", "/repos/:owner/:repo/contributors", ""}, {"GET", "/repos/:owner/:repo/languages", ""}, {"GET", "/repos/:owner/:repo/teams", ""}, @@ -359,19 +394,26 @@ var ( {"GET", "/repos/:owner/:repo/commits/:sha/comments", ""}, {"POST", "/repos/:owner/:repo/commits/:sha/comments", ""}, {"GET", "/repos/:owner/:repo/comments/:id", ""}, - //{"PATCH", "/repos/:owner/:repo/comments/:id", ""}, + + {"PATCH", "/repos/:owner/:repo/comments/:id", ""}, + {"DELETE", "/repos/:owner/:repo/comments/:id", ""}, {"GET", "/repos/:owner/:repo/commits", ""}, {"GET", "/repos/:owner/:repo/commits/:sha", ""}, {"GET", "/repos/:owner/:repo/readme", ""}, + //{"GET", "/repos/:owner/:repo/contents/*path", ""}, //{"PUT", "/repos/:owner/:repo/contents/*path", ""}, //{"DELETE", "/repos/:owner/:repo/contents/*path", ""}, - //{"GET", "/repos/:owner/:repo/:archive_format/:ref", ""}, + + {"GET", "/repos/:owner/:repo/:archive_format/:ref", ""}, + {"GET", "/repos/:owner/:repo/keys", ""}, {"GET", "/repos/:owner/:repo/keys/:id", ""}, {"POST", "/repos/:owner/:repo/keys", ""}, - //{"PATCH", "/repos/:owner/:repo/keys/:id", ""}, + + {"PATCH", "/repos/:owner/:repo/keys/:id", ""}, + {"DELETE", "/repos/:owner/:repo/keys/:id", ""}, {"GET", "/repos/:owner/:repo/downloads", ""}, {"GET", "/repos/:owner/:repo/downloads/:id", ""}, @@ -381,14 +423,18 @@ var ( {"GET", "/repos/:owner/:repo/hooks", ""}, {"GET", "/repos/:owner/:repo/hooks/:id", ""}, {"POST", "/repos/:owner/:repo/hooks", ""}, - //{"PATCH", "/repos/:owner/:repo/hooks/:id", ""}, + + {"PATCH", "/repos/:owner/:repo/hooks/:id", ""}, + {"POST", "/repos/:owner/:repo/hooks/:id/tests", ""}, {"DELETE", "/repos/:owner/:repo/hooks/:id", ""}, {"POST", "/repos/:owner/:repo/merges", ""}, {"GET", "/repos/:owner/:repo/releases", ""}, {"GET", "/repos/:owner/:repo/releases/:id", ""}, {"POST", "/repos/:owner/:repo/releases", ""}, - //{"PATCH", "/repos/:owner/:repo/releases/:id", ""}, + + {"PATCH", "/repos/:owner/:repo/releases/:id", ""}, + {"DELETE", "/repos/:owner/:repo/releases/:id", ""}, {"GET", "/repos/:owner/:repo/releases/:id/assets", ""}, {"GET", "/repos/:owner/:repo/stats/contributors", ""}, @@ -412,7 +458,9 @@ var ( // Users {"GET", "/users/:user", ""}, {"GET", "/user", ""}, - //{"PATCH", "/user", ""}, + + {"PATCH", "/user", ""}, + {"GET", "/users", ""}, {"GET", "/user/emails", ""}, {"POST", "/user/emails", ""}, @@ -429,7 +477,9 @@ var ( {"GET", "/user/keys", ""}, {"GET", "/user/keys/:id", ""}, {"POST", "/user/keys", ""}, - //{"PATCH", "/user/keys/:id", ""}, + + {"PATCH", "/user/keys/:id", ""}, + {"DELETE", "/user/keys/:id", ""}, } @@ -500,6 +550,88 @@ var ( {"DELETE", "/moments/:id", ""}, } + paramAndAnyAPI = []*Route{ + {"GET", "/root/:first/foo/*", ""}, + {"GET", "/root/:first/:second/*", ""}, + {"GET", "/root/:first/bar/:second/*", ""}, + {"GET", "/root/:first/qux/:second/:third/:fourth", ""}, + {"GET", "/root/:first/qux/:second/:third/:fourth/*", ""}, + {"GET", "/root/*", ""}, + + {"POST", "/root/:first/foo/*", ""}, + {"POST", "/root/:first/:second/*", ""}, + {"POST", "/root/:first/bar/:second/*", ""}, + {"POST", "/root/:first/qux/:second/:third/:fourth", ""}, + {"POST", "/root/:first/qux/:second/:third/:fourth/*", ""}, + {"POST", "/root/*", ""}, + + {"PUT", "/root/:first/foo/*", ""}, + {"PUT", "/root/:first/:second/*", ""}, + {"PUT", "/root/:first/bar/:second/*", ""}, + {"PUT", "/root/:first/qux/:second/:third/:fourth", ""}, + {"PUT", "/root/:first/qux/:second/:third/:fourth/*", ""}, + {"PUT", "/root/*", ""}, + + {"DELETE", "/root/:first/foo/*", ""}, + {"DELETE", "/root/:first/:second/*", ""}, + {"DELETE", "/root/:first/bar/:second/*", ""}, + {"DELETE", "/root/:first/qux/:second/:third/:fourth", ""}, + {"DELETE", "/root/:first/qux/:second/:third/:fourth/*", ""}, + {"DELETE", "/root/*", ""}, + } + + paramAndAnyAPIToFind = []*Route{ + {"GET", "/root/one/foo/after/the/asterisk", ""}, + {"GET", "/root/one/foo/path/after/the/asterisk", ""}, + {"GET", "/root/one/two/path/after/the/asterisk", ""}, + {"GET", "/root/one/bar/two/after/the/asterisk", ""}, + {"GET", "/root/one/qux/two/three/four", ""}, + {"GET", "/root/one/qux/two/three/four/after/the/asterisk", ""}, + + {"POST", "/root/one/foo/after/the/asterisk", ""}, + {"POST", "/root/one/foo/path/after/the/asterisk", ""}, + {"POST", "/root/one/two/path/after/the/asterisk", ""}, + {"POST", "/root/one/bar/two/after/the/asterisk", ""}, + {"POST", "/root/one/qux/two/three/four", ""}, + {"POST", "/root/one/qux/two/three/four/after/the/asterisk", ""}, + + {"PUT", "/root/one/foo/after/the/asterisk", ""}, + {"PUT", "/root/one/foo/path/after/the/asterisk", ""}, + {"PUT", "/root/one/two/path/after/the/asterisk", ""}, + {"PUT", "/root/one/bar/two/after/the/asterisk", ""}, + {"PUT", "/root/one/qux/two/three/four", ""}, + {"PUT", "/root/one/qux/two/three/four/after/the/asterisk", ""}, + + {"DELETE", "/root/one/foo/after/the/asterisk", ""}, + {"DELETE", "/root/one/foo/path/after/the/asterisk", ""}, + {"DELETE", "/root/one/two/path/after/the/asterisk", ""}, + {"DELETE", "/root/one/bar/two/after/the/asterisk", ""}, + {"DELETE", "/root/one/qux/two/three/four", ""}, + {"DELETE", "/root/one/qux/two/three/four/after/the/asterisk", ""}, + } + + missesAPI = []*Route{ + {"GET", "/missOne", ""}, + {"GET", "/miss/two", ""}, + {"GET", "/miss/three/levels", ""}, + {"GET", "/miss/four/levels/nooo", ""}, + + {"POST", "/missOne", ""}, + {"POST", "/miss/two", ""}, + {"POST", "/miss/three/levels", ""}, + {"POST", "/miss/four/levels/nooo", ""}, + + {"PUT", "/missOne", ""}, + {"PUT", "/miss/two", ""}, + {"PUT", "/miss/three/levels", ""}, + {"PUT", "/miss/four/levels/nooo", ""}, + + {"DELETE", "/missOne", ""}, + {"DELETE", "/miss/two", ""}, + {"DELETE", "/miss/three/levels", ""}, + {"DELETE", "/miss/four/levels/nooo", ""}, + } + // handlerHelper created a function that will set a context key for assertion handlerHelper = func(key string, value int) func(c Context) error { return func(c Context) error { @@ -1298,6 +1430,43 @@ func TestRouterParam1466(t *testing.T) { assert.Equal(t, 0, c.response.Status) } +// Issue #1655 +func TestRouterFindNotPanicOrLoopsWhenContextSetParamValuesIsCalledWithLessValuesThanEchoMaxParam(t *testing.T) { + e := New() + r := e.router + + v0 := e.Group("/:version") + v0.GET("/admin", func(c Context) error { + c.SetParamNames("version") + c.SetParamValues("v1") + return nil + }) + + v0.GET("/images/view/:id", handlerHelper("iv", 1)) + v0.GET("/images/:id", handlerHelper("i", 1)) + v0.GET("/view/*", handlerHelper("v", 1)) + + //If this API is called before the next two one panic the other loops ( of course without my fix ;) ) + c := e.NewContext(nil, nil) + r.Find(http.MethodGet, "/v1/admin", c) + c.Handler()(c) + assert.Equal(t, "v1", c.Param("version")) + + //panic + c = e.NewContext(nil, nil) + r.Find(http.MethodGet, "/v1/view/same-data", c) + c.Handler()(c) + assert.Equal(t, "same-data", c.Param("*")) + assert.Equal(t, 1, c.Get("v")) + + //looping + c = e.NewContext(nil, nil) + r.Find(http.MethodGet, "/v1/images/view", c) + c.Handler()(c) + assert.Equal(t, "view", c.Param("id")) + assert.Equal(t, 1, c.Get("i")) +} + // Issue #1653 func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) { e := New() @@ -1332,7 +1501,7 @@ func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) { assert.Equal(t, http.StatusNotFound, he.Code) } -func benchmarkRouterRoutes(b *testing.B, routes []*Route) { +func benchmarkRouterRoutes(b *testing.B, routes []*Route, routesToFind []*Route) { e := New() r := e.router b.ReportAllocs() @@ -1344,9 +1513,12 @@ func benchmarkRouterRoutes(b *testing.B, routes []*Route) { }) } + // Routes adding are performed just once, so it doesn't make sense to see that in the benchmark + b.ResetTimer() + // Find routes for i := 0; i < b.N; i++ { - for _, route := range gitHubAPI { + for _, route := range routesToFind { c := e.pool.Get().(*context) r.Find(route.Method, route.Path, c) e.pool.Put(c) @@ -1355,28 +1527,56 @@ func benchmarkRouterRoutes(b *testing.B, routes []*Route) { } func BenchmarkRouterStaticRoutes(b *testing.B) { - benchmarkRouterRoutes(b, staticRoutes) + benchmarkRouterRoutes(b, staticRoutes, staticRoutes) +} + +func BenchmarkRouterStaticRoutesMisses(b *testing.B) { + benchmarkRouterRoutes(b, staticRoutes, missesAPI) } func BenchmarkRouterGitHubAPI(b *testing.B) { - benchmarkRouterRoutes(b, gitHubAPI) + benchmarkRouterRoutes(b, gitHubAPI, gitHubAPI) +} + +func BenchmarkRouterGitHubAPIMisses(b *testing.B) { + benchmarkRouterRoutes(b, gitHubAPI, missesAPI) } func BenchmarkRouterParseAPI(b *testing.B) { - benchmarkRouterRoutes(b, parseAPI) + benchmarkRouterRoutes(b, parseAPI, parseAPI) +} + +func BenchmarkRouterParseAPIMisses(b *testing.B) { + benchmarkRouterRoutes(b, parseAPI, missesAPI) } func BenchmarkRouterGooglePlusAPI(b *testing.B) { - benchmarkRouterRoutes(b, googlePlusAPI) + benchmarkRouterRoutes(b, googlePlusAPI, googlePlusAPI) +} + +func BenchmarkRouterGooglePlusAPIMisses(b *testing.B) { + benchmarkRouterRoutes(b, googlePlusAPI, missesAPI) +} + +func BenchmarkRouterParamsAndAnyAPI(b *testing.B) { + benchmarkRouterRoutes(b, paramAndAnyAPI, paramAndAnyAPIToFind) } func (n *node) printTree(pfx string, tail bool) { p := prefix(tail, pfx, "└── ", "├── ") fmt.Printf("%s%s, %p: type=%d, parent=%p, handler=%v, pnames=%v\n", p, n.prefix, n, n.kind, n.parent, n.methodHandler, n.pnames) - children := n.children - l := len(children) p = prefix(tail, pfx, " ", "│ ") + + children := n.staticChildrens + l := len(children) + + if n.paramChildren != nil { + n.paramChildren.printTree(p, n.anyChildren == nil && l == 0) + } + if n.anyChildren != nil { + n.anyChildren.printTree(p, l == 0) + } for i := 0; i < l-1; i++ { children[i].printTree(p, false) }