Add Context.PathParamDefault(name string, defaultValue string) string method

Improve method comments here and there.

context.go

@@ -55,6 +55,16 @@ type Context interface {
 	// PathParam returns path parameter by name.
 	PathParam(name string) string
 
+	// PathParamDefault returns the path parameter or default value for the provided name.
+	//
+	// Notes for DefaultRouter implementation:
+	// Path parameter could be empty for cases like that:
+	// * route `/release-:version/bin` and request URL is `/release-/bin`
+	// * route `/api/:version/image.jpg` and request URL is `/api//image.jpg`
+	// but not when path parameter is last part of route path
+	// * route `/download/file.:ext` will not match request `/download/file.`
+	PathParamDefault(name string, defaultValue string) string
+
 	// PathParams returns path parameter values.
 	PathParams() PathParams
 
@@ -176,6 +186,9 @@ type Context interface {
 	Redirect(code int, url string) error
 
 	// Echo returns the `Echo` instance.
+	//
+	// WARNING: Remember that Echo public fields and methods are coroutine safe ONLY when you are NOT mutating them
+	// anywhere in your code after Echo server has started.
 	Echo() *Echo
 }
 
@@ -379,7 +392,10 @@ func (c *DefaultContext) PathParam(name string) string {
 	return c.pathParams.Get(name, "")
 }
 
-// PathParamDefault does not exist as expecting empty path param makes no sense
+// PathParamDefault returns the path parameter or default value for the provided name.
+func (c *DefaultContext) PathParamDefault(name, defaultValue string) string {
+	return c.pathParams.Get(name, defaultValue)
+}
 
 // PathParams returns path parameter values.
 func (c *DefaultContext) PathParams() PathParams {
@@ -406,7 +422,8 @@ func (c *DefaultContext) QueryParam(name string) string {
 }
 
 // QueryParamDefault returns the query param or default value for the provided name.
-// Note: QueryParamDefault does not distinguish if form had no value by that name or value was empty string
+// Note: QueryParamDefault does not distinguish if query had no value by that name or value was empty string
+// This means URLs `/test?search=` and `/test` would both return `1` for `c.QueryParamDefault("search", "1")`
 func (c *DefaultContext) QueryParamDefault(name, defaultValue string) string {
 	value := c.QueryParam(name)
 	if value == "" {

context_test.go

@@ -448,22 +448,142 @@ func TestContextCookie(t *testing.T) {
 	assert.Contains(t, rec.Header().Get(HeaderSetCookie), "HttpOnly")
 }
 
-func TestContextPathParam(t *testing.T) {
-	e := New()
-	req := httptest.NewRequest(http.MethodGet, "/", nil)
-	c := e.NewContext(req, nil).(*DefaultContext)
-
-	params := &PathParams{
-		{Name: "uid", Value: "101"},
-		{Name: "fid", Value: "501"},
+func TestContext_PathParams(t *testing.T) {
+	var testCases = []struct {
+		name   string
+		given  *PathParams
+		expect PathParams
+	}{
+		{
+			name: "param exists",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+				{Name: "fid", Value: "501"},
+			},
+			expect: PathParams{
+				{Name: "uid", Value: "101"},
+				{Name: "fid", Value: "501"},
+			},
+		},
+		{
+			name:   "params is empty",
+			given:  &PathParams{},
+			expect: PathParams{},
+		},
 	}
-	// ParamNames
-	c.pathParams = params
-	assert.EqualValues(t, *params, c.PathParams())
 
-	// Param
-	assert.Equal(t, "501", c.PathParam("fid"))
-	assert.Equal(t, "", c.PathParam("undefined"))
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			c := e.NewContext(req, nil)
+
+			c.(RoutableContext).SetRawPathParams(tc.given)
+
+			assert.EqualValues(t, tc.expect, c.PathParams())
+		})
+	}
+}
+
+func TestContext_PathParam(t *testing.T) {
+	var testCases = []struct {
+		name          string
+		given         *PathParams
+		whenParamName string
+		expect        string
+	}{
+		{
+			name: "param exists",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+				{Name: "fid", Value: "501"},
+			},
+			whenParamName: "uid",
+			expect:        "101",
+		},
+		{
+			name: "multiple same param values exists - return first",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+				{Name: "uid", Value: "202"},
+				{Name: "fid", Value: "501"},
+			},
+			whenParamName: "uid",
+			expect:        "101",
+		},
+		{
+			name: "param does not exists",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+			},
+			whenParamName: "nope",
+			expect:        "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			c := e.NewContext(req, nil)
+
+			c.(RoutableContext).SetRawPathParams(tc.given)
+
+			assert.EqualValues(t, tc.expect, c.PathParam(tc.whenParamName))
+		})
+	}
+}
+
+func TestContext_PathParamDefault(t *testing.T) {
+	var testCases = []struct {
+		name             string
+		given            *PathParams
+		whenParamName    string
+		whenDefaultValue string
+		expect           string
+	}{
+		{
+			name: "param exists",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+				{Name: "fid", Value: "501"},
+			},
+			whenParamName:    "uid",
+			whenDefaultValue: "999",
+			expect:           "101",
+		},
+		{
+			name: "param exists and is empty",
+			given: &PathParams{
+				{Name: "uid", Value: ""},
+				{Name: "fid", Value: "501"},
+			},
+			whenParamName:    "uid",
+			whenDefaultValue: "999",
+			expect:           "", // <-- this is different from QueryParamDefault behaviour
+		},
+		{
+			name: "param does not exists",
+			given: &PathParams{
+				{Name: "uid", Value: "101"},
+			},
+			whenParamName:    "nope",
+			whenDefaultValue: "999",
+			expect:           "999",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			c := e.NewContext(req, nil)
+
+			c.(RoutableContext).SetRawPathParams(tc.given)
+
+			assert.EqualValues(t, tc.expect, c.PathParamDefault(tc.whenParamName, tc.whenDefaultValue))
+		})
+	}
 }
 
 func TestContextGetAndSetParam(t *testing.T) {
@@ -568,27 +688,129 @@ func TestContextFormValue(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestContextQueryParam(t *testing.T) {
-	q := make(url.Values)
-	q.Set("name", "Jon Snow")
-	q.Set("email", "jon@labstack.com")
-	req := httptest.NewRequest(http.MethodGet, "/?"+q.Encode(), nil)
-	e := New()
-	c := e.NewContext(req, nil)
+func TestContext_QueryParams(t *testing.T) {
+	var testCases = []struct {
+		name     string
+		givenURL string
+		expect   url.Values
+	}{
+		{
+			name:     "multiple values in url",
+			givenURL: "/?test=1&test=2&email=jon%40labstack.com",
+			expect: url.Values{
+				"test":  []string{"1", "2"},
+				"email": []string{"jon@labstack.com"},
+			},
+		},
+		{
+			name:     "single value  in url",
+			givenURL: "/?nope=1",
+			expect: url.Values{
+				"nope": []string{"1"},
+			},
+		},
+		{
+			name:     "no query params in url",
+			givenURL: "/?",
+			expect:   url.Values{},
+		},
+	}
 
-	// QueryParam
-	assert.Equal(t, "Jon Snow", c.QueryParam("name"))
-	assert.Equal(t, "jon@labstack.com", c.QueryParam("email"))
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.givenURL, nil)
+			e := New()
+			c := e.NewContext(req, nil)
 
-	// QueryParamDefault
-	assert.Equal(t, "Jon Snow", c.QueryParamDefault("name", "nope"))
-	assert.Equal(t, "default", c.QueryParamDefault("missing", "default"))
+			assert.Equal(t, tc.expect, c.QueryParams())
+		})
+	}
+}
 
-	// QueryParams
-	assert.Equal(t, url.Values{
-		"name":  []string{"Jon Snow"},
-		"email": []string{"jon@labstack.com"},
-	}, c.QueryParams())
+func TestContext_QueryParam(t *testing.T) {
+	var testCases = []struct {
+		name          string
+		givenURL      string
+		whenParamName string
+		expect        string
+	}{
+		{
+			name:          "value exists in url",
+			givenURL:      "/?test=1",
+			whenParamName: "test",
+			expect:        "1",
+		},
+		{
+			name:          "multiple values exists in url",
+			givenURL:      "/?test=9&test=8",
+			whenParamName: "test",
+			expect:        "9", // <-- first value in returned
+		},
+		{
+			name:          "value does not exists in url",
+			givenURL:      "/?nope=1",
+			whenParamName: "test",
+			expect:        "",
+		},
+		{
+			name:          "value is empty in url",
+			givenURL:      "/?test=",
+			whenParamName: "test",
+			expect:        "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.givenURL, nil)
+			e := New()
+			c := e.NewContext(req, nil)
+
+			assert.Equal(t, tc.expect, c.QueryParam(tc.whenParamName))
+		})
+	}
+}
+
+func TestContext_QueryParamDefault(t *testing.T) {
+	var testCases = []struct {
+		name             string
+		givenURL         string
+		whenParamName    string
+		whenDefaultValue string
+		expect           string
+	}{
+		{
+			name:             "value exists in url",
+			givenURL:         "/?test=1",
+			whenParamName:    "test",
+			whenDefaultValue: "999",
+			expect:           "1",
+		},
+		{
+			name:             "value does not exists in url",
+			givenURL:         "/?nope=1",
+			whenParamName:    "test",
+			whenDefaultValue: "999",
+			expect:           "999",
+		},
+		{
+			name:             "value is empty in url",
+			givenURL:         "/?test=",
+			whenParamName:    "test",
+			whenDefaultValue: "999",
+			expect:           "999",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.givenURL, nil)
+			e := New()
+			c := e.NewContext(req, nil)
+
+			assert.Equal(t, tc.expect, c.QueryParamDefault(tc.whenParamName, tc.whenDefaultValue))
+		})
+	}
 }
 
 func TestContextFormFile(t *testing.T) {

echo.go

@@ -55,7 +55,9 @@ import (
 )
 
 // Echo is the top-level framework instance.
-// Note: replacing/nilling public fields is not coroutine/thread-safe and can cause data-races/panics.
+//
+// Note: replacing/nilling public fields is not coroutine/thread-safe and can cause data-races/panics. This is very likely
+// to happen when you access Echo instances through Context.Echo() method.
 type Echo struct {
 	// premiddleware are middlewares that are run for every request before routing is done
 	premiddleware []MiddlewareFunc
@@ -67,8 +69,8 @@ type Echo struct {
 	routerCreator func(e *Echo) Router
 
 	contextPool sync.Pool
-	// contextPathParamAllocSize holds maximum parameter count for all added routes. This is necessary info for context
-	// creation time so we can allocate path parameter values slice.
+	// contextPathParamAllocSize holds maximum parameter count for all added routes. This is necessary info at context
+	// creation moment so we can allocate path parameter values slice with correct size.
 	contextPathParamAllocSize int
 
 	// NewContextFunc allows using custom context implementations, instead of default *echo.context

router.go

@@ -8,6 +8,15 @@ import (
 )
 
 // Router is interface for routing request contexts to registered routes.
+//
+// Contract between Echo/Context instance and the router:
+// * all routes must be added through methods on echo.Echo instance.
+//   Reason: Echo instance uses RouteInfo.Params() length to allocate slice for paths parameters (see `Echo.contextPathParamAllocSize`).
+// * Router must populate Context during Router.Route call with:
+//   * RoutableContext.SetPath
+//   * RoutableContext.SetRawPathParams (IMPORTANT! with same slice pointer that c.RawPathParams() returns)
+//   * RoutableContext.SetRouteInfo
+//   And optionally can set additional information to Context with RoutableContext.Set
 type Router interface {
 	// Add registers Routable with the Router and returns registered RouteInfo
 	Add(routable Routable) (RouteInfo, error)
@@ -19,11 +28,6 @@ type Router interface {
 	// Route searches Router for matching route and applies it to the given context. In case when no matching method
 	// was not found (405) or no matching route exists for path (404), router will return its implementation of 405/404
 	// handler function.
-	// When implementing custom Router make sure to populate Context during Router.Route call with:
-	// * RoutableContext.SetPath
-	// * RoutableContext.SetRawPathParams (IMPORTANT! with same slice pointer that c.RawPathParams() returns)
-	// * RoutableContext.SetRouteInfo
-	// And optionally can set additional information to Context with RoutableContext.Set
 	Route(c RoutableContext) HandlerFunc
 }
 

router_test.go

@@ -2242,6 +2242,64 @@ func TestRouter_Match_DifferentParamNamesForSamePlace(t *testing.T) {
 	}
 }
 
+// Issue #2164 - this test is meant to document path parameter behaviour when request url has empty value in place
+// of the path parameter. As tests show the result is different depending on where parameter exists in the route path.
+func TestDefaultRouter_PathParamsCanMatchEmptyValues(t *testing.T) {
+	var testCases = []struct {
+		name        string
+		whenURL     string
+		expectRoute string
+		expectParam map[string]string
+		expectError error
+	}{
+		{
+			name:        "ok, route is matched with even empty param is in the middle and between slashes",
+			whenURL:     "/a//b",
+			expectRoute: "/a/:id/b",
+			expectParam: map[string]string{"id": ""},
+		},
+		{
+			name:        "ok, route is matched with even empty param is in the middle",
+			whenURL:     "/a2/b",
+			expectRoute: "/a2:id/b",
+			expectParam: map[string]string{"id": ""},
+		},
+		{
+			name:        "ok, route is NOT matched with even empty param is at the end",
+			whenURL:     "/a3/",
+			expectRoute: "",
+			expectParam: map[string]string{},
+			expectError: ErrNotFound,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+
+			e.GET("/a/:id/b", handlerFunc)
+			e.GET("/a2:id/b", handlerFunc)
+			e.GET("/a3/:id", handlerFunc)
+
+			req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)
+			c := e.NewContext(req, nil).(*DefaultContext)
+			handler := e.router.Route(c)
+
+			err := handler(c)
+			if tc.expectError != nil {
+				assert.Equal(t, tc.expectError, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tc.expectRoute, c.Path())
+			for param, expectedValue := range tc.expectParam {
+				assert.Equal(t, expectedValue, c.pathParams.Get(param, ""))
+			}
+			checkUnusedParamValues(t, c, tc.expectParam)
+		})
+	}
+}
+
 // Issue #729
 func TestRouterParamAlias(t *testing.T) {
 	api := []testRoute{