fix(tree): correctly expand the capacity of params (#3502)

gin-gonic · Dec 7, 2023 · 386d244 · 386d244
1 parent 44d0dd7
commit 386d244
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 4 deletions.
diff --git a/routes_test.go b/routes_test.go
@@ -337,6 +337,45 @@ func TestRouteParamsByNameWithExtraSlash(t *testing.T) {
 	assert.Equal(t, "/is/super/great", wild)
 }
 
+// TestRouteParamsNotEmpty tests that context parameters will be set
+// even if a route with params/wildcards is registered after the context
+// initialisation (which happened in a previous requets).
+func TestRouteParamsNotEmpty(t *testing.T) {
+	name := ""
+	lastName := ""
+	wild := ""
+	router := New()
+
+	w := PerformRequest(router, http.MethodGet, "/test/john/smith/is/super/great")
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+
+	router.GET("/test/:name/:last_name/*wild", func(c *Context) {
+		name = c.Params.ByName("name")
+		lastName = c.Params.ByName("last_name")
+		var ok bool
+		wild, ok = c.Params.Get("wild")
+
+		assert.True(t, ok)
+		assert.Equal(t, name, c.Param("name"))
+		assert.Equal(t, lastName, c.Param("last_name"))
+
+		assert.Empty(t, c.Param("wtf"))
+		assert.Empty(t, c.Params.ByName("wtf"))
+
+		wtf, ok := c.Params.Get("wtf")
+		assert.Empty(t, wtf)
+		assert.False(t, ok)
+	})
+
+	w = PerformRequest(router, http.MethodGet, "/test/john/smith/is/super/great")
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, "john", name)
+	assert.Equal(t, "smith", lastName)
+	assert.Equal(t, "/is/super/great", wild)
+}
+
 // TestHandleStaticFile - ensure the static file handles properly
 func TestRouteStaticFile(t *testing.T) {
 	// SETUP file

diff --git a/tree.go b/tree.go
@@ -497,7 +497,14 @@ walk: // Outer loop for walking the tree
 					}
 
 					// Save param value
-					if params != nil && cap(*params) > 0 {
+					if params != nil {
+						// Preallocate capacity if necessary
+						if cap(*params) < int(globalParamsCount) {
+							newParams := make(Params, len(*params), globalParamsCount)
+							copy(newParams, *params)
+							*params = newParams
+						}
+
 						if value.params == nil {
 							value.params = params
 						}
@@ -544,6 +551,13 @@ walk: // Outer loop for walking the tree
 				case catchAll:
 					// Save param value
 					if params != nil {
+						// Preallocate capacity if necessary
+						if cap(*params) < int(globalParamsCount) {
+							newParams := make(Params, len(*params), globalParamsCount)
+							copy(newParams, *params)
+							*params = newParams
+						}
+
 						if value.params == nil {
 							value.params = params
 						}

diff --git a/tree_test.go b/tree_test.go
@@ -893,9 +893,9 @@ func TestTreeInvalidNodeType(t *testing.T) {
 
 func TestTreeInvalidParamsType(t *testing.T) {
 	tree := &node{}
-	tree.wildChild = true
-	tree.children = append(tree.children, &node{})
-	tree.children[0].nType = 2
+	// add a child with wildcard
+	route := "/:path"
+	tree.addRoute(route, fakeHandler(route))
 
 	// set invalid Params type
 	params := make(Params, 0)
@@ -904,6 +904,34 @@ func TestTreeInvalidParamsType(t *testing.T) {
 	tree.getValue("/test", &params, getSkippedNodes(), false)
 }
 
+func TestTreeExpandParamsCapacity(t *testing.T) {
+	data := []struct {
+		path string
+	}{
+		{"/:path"},
+		{"/*path"},
+	}
+
+	for _, item := range data {
+		tree := &node{}
+		tree.addRoute(item.path, fakeHandler(item.path))
+		params := make(Params, 0)
+
+		value := tree.getValue("/test", &params, getSkippedNodes(), false)
+
+		if value.params == nil {
+			t.Errorf("Expected %s params to be set, but they weren't", item.path)
+			continue
+		}
+
+		if len(*value.params) != 1 {
+			t.Errorf("Wrong number of %s params: got %d, want %d",
+				item.path, len(*value.params), 1)
+			continue
+		}
+	}
+}
+
 func TestTreeWildcardConflictEx(t *testing.T) {
 	conflicts := [...]struct {
 		route        string