From 3b1ecf970065e0c44f388ff77c8cec1b6351db21 Mon Sep 17 00:00:00 2001 From: Ole Bulbuk Date: Thu, 30 Mar 2017 12:05:24 +0200 Subject: [PATCH 1/3] Tested and fixed bug with reused context (path parameters). --- context.go | 5 ++++- context_test.go | 9 +++++++++ router.go | 34 +++++++++++++++++----------------- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/context.go b/context.go index 798bcc240..cf79e2f15 100644 --- a/context.go +++ b/context.go @@ -275,7 +275,7 @@ func (c *context) SetParamNames(names ...string) { } func (c *context) ParamValues() []string { - return c.pvalues + return c.pvalues[:len(c.pnames)] } func (c *context) SetParamValues(values ...string) { @@ -553,4 +553,7 @@ func (c *context) Reset(r *http.Request, w http.ResponseWriter) { c.query = nil c.handler = NotFoundHandler c.store = nil + c.pnames = nil + // WARNING: Don't reset because it has to have length c.echo.maxParam at all times: c.pvalues = nil + c.path = "" } diff --git a/context_test.go b/context_test.go index 892b02ebc..ad4cf892c 100644 --- a/context_test.go +++ b/context_test.go @@ -190,7 +190,16 @@ func TestContext(t *testing.T) { assert.Equal(t, http.StatusInternalServerError, rec.Code) // Reset + c.SetParamNames("foo") + c.SetParamValues("bar") + c.Set("foe", "ban") + c.query = url.Values(map[string][]string{"fon": []string{"baz"}}) c.Reset(req, httptest.NewRecorder()) + assert.Equal(t, 0, len(c.ParamValues())) + assert.Equal(t, 0, len(c.ParamNames())) + assert.Equal(t, 0, len(c.store)) + assert.Equal(t, "", c.Path()) + assert.Equal(t, 0, len(c.QueryParams())) } func TestContextCookie(t *testing.T) { diff --git a/router.go b/router.go index a8d1f1aa0..933a91a70 100644 --- a/router.go +++ b/router.go @@ -296,18 +296,18 @@ func (n *node) checkMethodNotAllowed() HandlerFunc { // - Get context from `Echo#AcquireContext()` // - Reset it `Context#Reset()` // - Return it `Echo#ReleaseContext()`. -func (r *Router) Find(method, path string, context Context) { - context.SetPath(path) +func (r *Router) Find(method, path string, ctx Context) { + ctx.SetPath(path) cn := r.tree // Current node as root var ( search = path - c *node // Child node - n int // Param counter - nk kind // Next kind - nn *node // Next node - ns string // Next search - pvalues = context.ParamValues() + c *node // Child node + n int // Param counter + nk kind // Next kind + nn *node // Next node + ns string // Next search + pvalues = ctx.(*context).pvalues // use the internal slice so the interface can keep the illusion of a dynamic slice ) // Search order static > param > any @@ -409,13 +409,13 @@ func (r *Router) Find(method, path string, context Context) { } End: - context.SetHandler(cn.findHandler(method)) - context.SetPath(cn.ppath) - context.SetParamNames(cn.pnames...) + ctx.SetHandler(cn.findHandler(method)) + ctx.SetPath(cn.ppath) + ctx.SetParamNames(cn.pnames...) // NOTE: Slow zone... - if context.Handler() == nil { - context.SetHandler(cn.checkMethodNotAllowed()) + if ctx.Handler() == nil { + ctx.SetHandler(cn.checkMethodNotAllowed()) // Dig further for any, might have an empty value for *, e.g. // serving a directory. Issue #207. @@ -423,12 +423,12 @@ End: return } if h := cn.findHandler(method); h != nil { - context.SetHandler(h) + ctx.SetHandler(h) } else { - context.SetHandler(cn.checkMethodNotAllowed()) + ctx.SetHandler(cn.checkMethodNotAllowed()) } - context.SetPath(cn.ppath) - context.SetParamNames(cn.pnames...) + ctx.SetPath(cn.ppath) + ctx.SetParamNames(cn.pnames...) pvalues[len(cn.pnames)-1] = "" } From ec7427435be3c6a60f2ad51ff685d0b759e00a32 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 10 Apr 2017 13:10:31 -0700 Subject: [PATCH 2/3] Using *context instead of Context in Router#Find() Signed-off-by: Vishal Rana --- context.go | 5 +++-- router.go | 43 ++++++++++++++++++++++--------------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/context.go b/context.go index cf79e2f15..681f564e9 100644 --- a/context.go +++ b/context.go @@ -553,7 +553,8 @@ func (c *context) Reset(r *http.Request, w http.ResponseWriter) { c.query = nil c.handler = NotFoundHandler c.store = nil - c.pnames = nil - // WARNING: Don't reset because it has to have length c.echo.maxParam at all times: c.pvalues = nil c.path = "" + c.pnames = nil + // NOTE: Don't reset because it has to have length c.echo.maxParam at all times + // c.pvalues = nil } diff --git a/router.go b/router.go index 933a91a70..876fc5d8a 100644 --- a/router.go +++ b/router.go @@ -296,18 +296,19 @@ func (n *node) checkMethodNotAllowed() HandlerFunc { // - Get context from `Echo#AcquireContext()` // - Reset it `Context#Reset()` // - Return it `Echo#ReleaseContext()`. -func (r *Router) Find(method, path string, ctx Context) { - ctx.SetPath(path) +func (r *Router) Find(method, path string, c Context) { + ctx := c.(*context) + ctx.path = path cn := r.tree // Current node as root var ( search = path - c *node // Child node - n int // Param counter - nk kind // Next kind - nn *node // Next node - ns string // Next search - pvalues = ctx.(*context).pvalues // use the internal slice so the interface can keep the illusion of a dynamic slice + child *node // Child node + n int // Param counter + nk kind // Next kind + nn *node // Next node + ns string // Next search + pvalues = ctx.pvalues // Use the internal slice so the interface can keep the illusion of a dynamic slice ) // Search order static > param > any @@ -352,20 +353,20 @@ func (r *Router) Find(method, path string, ctx Context) { } // Static node - if c = cn.findChild(search[0], skind); c != nil { + if child = cn.findChild(search[0], skind); child != nil { // Save next if cn.prefix[len(cn.prefix)-1] == '/' { // Issue #623 nk = pkind nn = cn ns = search } - cn = c + cn = child continue } // Param node Param: - if c = cn.findChildByKind(pkind); c != nil { + if child = cn.findChildByKind(pkind); child != nil { // Issue #378 if len(pvalues) == n { continue @@ -378,7 +379,7 @@ func (r *Router) Find(method, path string, ctx Context) { ns = search } - cn = c + cn = child i, l := 0, len(search) for ; i < l && search[i] != '/'; i++ { } @@ -409,13 +410,13 @@ func (r *Router) Find(method, path string, ctx Context) { } End: - ctx.SetHandler(cn.findHandler(method)) - ctx.SetPath(cn.ppath) - ctx.SetParamNames(cn.pnames...) + ctx.handler = cn.findHandler(method) + ctx.path = cn.ppath + ctx.pnames = cn.pnames // NOTE: Slow zone... - if ctx.Handler() == nil { - ctx.SetHandler(cn.checkMethodNotAllowed()) + if ctx.handler == nil { + ctx.handler = cn.checkMethodNotAllowed() // Dig further for any, might have an empty value for *, e.g. // serving a directory. Issue #207. @@ -423,12 +424,12 @@ End: return } if h := cn.findHandler(method); h != nil { - ctx.SetHandler(h) + ctx.handler = h } else { - ctx.SetHandler(cn.checkMethodNotAllowed()) + ctx.handler = cn.checkMethodNotAllowed() } - ctx.SetPath(cn.ppath) - ctx.SetParamNames(cn.pnames...) + ctx.path = cn.ppath + ctx.pnames = cn.pnames pvalues[len(cn.pnames)-1] = "" } From ae7bcaff622574df4898ce92892932d11665138a Mon Sep 17 00:00:00 2001 From: YongHaoHu Date: Wed, 12 Apr 2017 18:28:34 +0800 Subject: [PATCH 3/3] middleware/test: Use string literal instead of escape char. --- middleware/logger_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/middleware/logger_test.go b/middleware/logger_test.go index f8b09cfdd..ee8e79bca 100644 --- a/middleware/logger_test.go +++ b/middleware/logger_test.go @@ -122,10 +122,10 @@ func TestLoggerTemplate(t *testing.T) { "hexvalue": false, "GET": true, "127.0.0.1": true, - "\"path\":\"/\"": true, - "\"uri\":\"/\"": true, - "\"status\":200": true, - "\"bytes_in\":0": true, + `"path":"/"`: true, + `"uri":"/"`: true, + `"status":200`: true, + `"bytes_in":0`: true, "google.com": true, "echo-tests-agent": true, "6ba7b810-9dad-11d1-80b4-00c04fd430c8": true,