From db309081bc5c137b2aa15701ef53f7f19788da25 Mon Sep 17 00:00:00 2001 From: Jacob McSwain Date: Fri, 27 Feb 2026 09:33:46 -0600 Subject: [PATCH 1/5] chore(logger): allow skipping query string output (#4547) This is useful for APIs that might have sensitive information in the query string, such as API keys. This patch does not change the default behavior of the code unless the new `SkipQueryString` config option is passed in. The "skip" term is a bit of a misnomer here, as this doesn't actually skip that log, but modifies the output. I'm open to suggestions for a more appropriate name. Co-authored-by: Bo-Yi Wu --- docs/doc.md | 15 +++++++++++++++ logger.go | 7 ++++++- logger_test.go | 14 ++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/docs/doc.md b/docs/doc.md index 449c8d02..7201df5c 100644 --- a/docs/doc.md +++ b/docs/doc.md @@ -22,6 +22,7 @@ - [How to write log file](#how-to-write-log-file) - [Custom Log Format](#custom-log-format) - [Controlling Log output coloring](#controlling-log-output-coloring) + - [Avoid logging query strings](#avoid-loging-query-strings) - [Model binding and validation](#model-binding-and-validation) - [Custom Validators](#custom-validators) - [Only Bind Query String](#only-bind-query-string) @@ -592,6 +593,20 @@ func main() { } ``` +### Avoid logging query strings + +```go +func main() { + router := gin.New() + + // SkipQueryString indicates that the logger should not log the query string. + // For example, /path?q=1 will be logged as /path + loggerConfig := gin.LoggerConfig{SkipQueryString: true} + + router.Use(gin.LoggerWithConfig(loggerConfig)) +} +``` + ### Model binding and validation To bind a request body into a type, use model binding. We currently support binding of JSON, XML, YAML, TOML and standard form values (foo=bar&boo=baz). diff --git a/logger.go b/logger.go index 6441f7ea..cf92553a 100644 --- a/logger.go +++ b/logger.go @@ -48,6 +48,11 @@ type LoggerConfig struct { // Optional. SkipPaths []string + // SkipQueryString indicates that query strings should not be written + // for cases such as when API keys are passed via query strings. + // Optional. Default value is false. + SkipQueryString bool + // Skip is a Skipper that indicates which logs should not be written. // Optional. Skip Skipper @@ -298,7 +303,7 @@ func LoggerWithConfig(conf LoggerConfig) HandlerFunc { param.BodySize = c.Writer.Size() - if raw != "" { + if raw != "" && !conf.SkipQueryString { path = path + "?" + raw } diff --git a/logger_test.go b/logger_test.go index 53d0df95..8099a894 100644 --- a/logger_test.go +++ b/logger_test.go @@ -471,3 +471,17 @@ func TestForceConsoleColor(t *testing.T) { // reset console color mode. consoleColorMode = autoColor } + +func TestLoggerWithConfigSkipQueryString(t *testing.T) { + buffer := new(strings.Builder) + router := New() + router.Use(LoggerWithConfig(LoggerConfig{ + Output: buffer, + SkipQueryString: true, + })) + router.GET("/logged", func(c *Context) { c.Status(http.StatusOK) }) + + PerformRequest(router, "GET", "/logged?a=21") + assert.Contains(t, buffer.String(), "200") + assert.NotContains(t, buffer.String(), "a=21") +} From 5c00df8afadd06cc5be530dde00fe6d9fa4a2e4a Mon Sep 17 00:00:00 2001 From: Denis Galeev <11516397+dengaleev@users.noreply.github.com> Date: Sat, 28 Feb 2026 09:07:31 +0700 Subject: [PATCH 2/5] fix(render): write content length in Data.Render (#4206) * init test * fix test * fix assert.EqualValues usage --------- Co-authored-by: Bo-Yi Wu --- render/data.go | 8 +++++++- render/render_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/render/data.go b/render/data.go index a653ea30..2c0ad5e3 100644 --- a/render/data.go +++ b/render/data.go @@ -4,7 +4,10 @@ package render -import "net/http" +import ( + "net/http" + "strconv" +) // Data contains ContentType and bytes data. type Data struct { @@ -15,6 +18,9 @@ type Data struct { // Render (Data) writes data with custom ContentType. func (r Data) Render(w http.ResponseWriter) (err error) { r.WriteContentType(w) + if len(r.Data) > 0 { + w.Header().Set("Content-Length", strconv.Itoa(len(r.Data))) + } _, err = w.Write(r.Data) return } diff --git a/render/render_test.go b/render/render_test.go index 40e7e7be..9c3019eb 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -8,6 +8,7 @@ import ( "encoding/xml" "errors" "html/template" + "io" "net" "net/http" "net/http/httptest" @@ -453,6 +454,36 @@ func TestRenderData(t *testing.T) { require.NoError(t, err) assert.Equal(t, "#!PNG some raw data", w.Body.String()) assert.Equal(t, "image/png", w.Header().Get("Content-Type")) + assert.Equal(t, "19", w.Header().Get("Content-Length")) +} + +func TestRenderDataContentLength(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + size, err := strconv.Atoi(r.URL.Query().Get("size")) + assert.NoError(t, err) + + data := Data{ + ContentType: "application/octet-stream", + Data: make([]byte, size), + } + assert.NoError(t, data.Render(w)) + })) + t.Cleanup(srv.Close) + + for _, size := range []int{0, 1, 100, 100_000} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + resp, err := http.Get(srv.URL + "?size=" + strconv.Itoa(size)) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, "application/octet-stream", resp.Header.Get("Content-Type")) + assert.Equal(t, strconv.Itoa(size), resp.Header.Get("Content-Length")) + + actual, err := io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + assert.EqualValues(t, size, actual) + }) + } } func TestRenderString(t *testing.T) { From 6f1d5fe3cdb171a08928c3c9dd3fbcfc9ee1b521 Mon Sep 17 00:00:00 2001 From: Amirhf Date: Sat, 28 Feb 2026 05:41:57 +0330 Subject: [PATCH 3/5] test(render): add comprehensive error handling tests (#4541) * test(render): add comprehensive error handling tests Add error case tests for XML, Data, BSON, and HTML renderers to improve test coverage and ensure proper error handling: - TestRenderXMLError: validates XML marshal error handling for unsupported types - TestRenderDataError: validates Data write error handling - TestRenderBSONError: validates BSON marshal error handling for unsupported types - TestRenderBSONWriteError: validates BSON write error handling - TestRenderHTMLTemplateError: validates HTML template execution error with invalid field access - TestRenderHTMLTemplateExecuteError: validates HTML template execution error with invalid nested field All tests pass and maintain 100% coverage for the render package. * test(render): improve robustness of error handling tests based on PR feedback --------- Co-authored-by: Bo-Yi Wu Co-authored-by: AmirHossein Fallah --- render/render_test.go | 146 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 126 insertions(+), 20 deletions(-) diff --git a/render/render_test.go b/render/render_test.go index 9c3019eb..b48ab3d3 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -16,7 +16,6 @@ import ( "strings" "testing" - "github.com/gin-gonic/gin/codec/json" testdata "github.com/gin-gonic/gin/testdata/protoexample" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -24,9 +23,6 @@ import ( "google.golang.org/protobuf/proto" ) -// TODO unit tests -// test errors - func TestRenderJSON(t *testing.T) { w := httptest.NewRecorder() data := map[string]any{ @@ -141,19 +137,44 @@ func TestRenderJsonpJSON(t *testing.T) { } type errorWriter struct { - bufString string + bufString string + ErrThreshold int // 1-based threshold. If 1, errors on the 1st Write call. + writeCount int *httptest.ResponseRecorder } var _ http.ResponseWriter = (*errorWriter)(nil) +func (w *errorWriter) Header() http.Header { + if w.ResponseRecorder == nil { + w.ResponseRecorder = httptest.NewRecorder() + } + return w.ResponseRecorder.Header() +} + +func (w *errorWriter) WriteHeader(statusCode int) { + if w.ResponseRecorder == nil { + w.ResponseRecorder = httptest.NewRecorder() + } + w.ResponseRecorder.WriteHeader(statusCode) +} + func (w *errorWriter) Write(buf []byte) (int, error) { - if string(buf) == w.bufString { - return 0, errors.New(`write "` + w.bufString + `" error`) + w.writeCount++ + if (w.bufString != "" && string(buf) == w.bufString) || (w.ErrThreshold > 0 && w.writeCount >= w.ErrThreshold) { + return 0, errors.New(`write error`) + } + if w.ResponseRecorder == nil { + w.ResponseRecorder = httptest.NewRecorder() } return w.ResponseRecorder.Write(buf) } +func (w *errorWriter) reset() { + w.writeCount = 0 + w.ResponseRecorder = httptest.NewRecorder() +} + func TestRenderJsonpJSONError(t *testing.T) { ew := &errorWriter{ ResponseRecorder: httptest.NewRecorder(), @@ -166,23 +187,33 @@ func TestRenderJsonpJSONError(t *testing.T) { }, } - cb := template.JSEscapeString(jsonpJSON.Callback) - ew.bufString = cb - err := jsonpJSON.Render(ew) // error was returned while writing callback - assert.Equal(t, `write "`+cb+`" error`, err.Error()) + // error was returned while writing callback + ew.reset() + ew.ErrThreshold = 1 + err := jsonpJSON.Render(ew) + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) - ew.bufString = `(` + // error was returned while writing "(" + ew.reset() + ew.ErrThreshold = 2 err = jsonpJSON.Render(ew) - assert.Equal(t, `write "`+`(`+`" error`, err.Error()) + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) - data, _ := json.API.Marshal(jsonpJSON.Data) // error was returned while writing data - ew.bufString = string(data) + // error was returned while writing data + ew.reset() + ew.ErrThreshold = 3 err = jsonpJSON.Render(ew) - assert.Equal(t, `write "`+string(data)+`" error`, err.Error()) + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) - ew.bufString = `);` + // error was returned while writing ");" + ew.reset() + ew.ErrThreshold = 4 err = jsonpJSON.Render(ew) - assert.Equal(t, `write "`+`);`+`" error`, err.Error()) + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) } func TestRenderJsonpJSONError2(t *testing.T) { @@ -386,6 +417,30 @@ func TestRenderBSON(t *testing.T) { assert.Equal(t, "application/bson", w.Header().Get("Content-Type")) } +func TestRenderBSONError(t *testing.T) { + w := httptest.NewRecorder() + data := make(chan int) + + err := (BSON{data}).Render(w) + require.Error(t, err) +} + +func TestRenderBSONWriteError(t *testing.T) { + type testStruct struct { + Value string + } + data := &testStruct{Value: "test"} + + ew := &errorWriter{ + ErrThreshold: 1, + ResponseRecorder: httptest.NewRecorder(), + } + + err := (BSON{data}).Render(ew) + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) +} + func TestRenderXML(t *testing.T) { w := httptest.NewRecorder() data := xmlmap{ @@ -402,6 +457,15 @@ func TestRenderXML(t *testing.T) { assert.Equal(t, "application/xml; charset=utf-8", w.Header().Get("Content-Type")) } +func TestRenderXMLError(t *testing.T) { + w := httptest.NewRecorder() + data := make(chan int) + + err := (XML{data}).Render(w) + require.Error(t, err) + assert.Contains(t, err.Error(), "xml: unsupported type: chan int") +} + func TestRenderRedirect(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "/test-redirect", nil) require.NoError(t, err) @@ -486,6 +550,22 @@ func TestRenderDataContentLength(t *testing.T) { } } +func TestRenderDataError(t *testing.T) { + ew := &errorWriter{ + ErrThreshold: 1, + ResponseRecorder: httptest.NewRecorder(), + } + data := []byte("#!PNG some raw data") + + err := (Data{ + ContentType: "image/png", + Data: data, + }).Render(ew) + + require.Error(t, err) + assert.Equal(t, "write error", err.Error()) +} + func TestRenderString(t *testing.T) { w := httptest.NewRecorder() @@ -625,6 +705,32 @@ func TestRenderHTMLDebugPanics(t *testing.T) { assert.Panics(t, func() { htmlRender.Instance("", nil) }) } +func TestRenderHTMLTemplateError(t *testing.T) { + w := httptest.NewRecorder() + templ := template.Must(template.New("t").Parse(`Hello {{if .name}}{{.name.DoesNotExist}}{{end}}`)) + + htmlRender := HTMLProduction{Template: templ} + instance := htmlRender.Instance("t", map[string]any{ + "name": "alexandernyquist", + }) + + err := instance.Render(w) + require.Error(t, err) +} + +func TestRenderHTMLTemplateExecuteError(t *testing.T) { + w := httptest.NewRecorder() + templ := template.Must(template.New("t").Parse(`Hello {{.name.invalid}}`)) + + htmlRender := HTMLProduction{Template: templ} + instance := htmlRender.Instance("t", map[string]any{ + "name": "alexandernyquist", + }) + + err := instance.Render(w) + require.Error(t, err) +} + func TestRenderReader(t *testing.T) { w := httptest.NewRecorder() @@ -676,10 +782,10 @@ func TestRenderWriteError(t *testing.T) { prefix := "my-prefix:" r := SecureJSON{Data: data, Prefix: prefix} ew := &errorWriter{ - bufString: prefix, + ErrThreshold: 1, ResponseRecorder: httptest.NewRecorder(), } err := r.Render(ew) require.Error(t, err) - assert.Equal(t, `write "my-prefix:" error`, err.Error()) + assert.Equal(t, "write error", err.Error()) } From fb2583442c4d9bccb75e6d26f1aa6e7c01950db6 Mon Sep 17 00:00:00 2001 From: Mehrdad Banikian <80095851+mehrdadbn9@users.noreply.github.com> Date: Sat, 28 Feb 2026 05:43:11 +0330 Subject: [PATCH 4/5] test(context): use http.StatusContinue constant instead of magic number 100 (#4542) * refactor(context): use http.StatusContinue constant instead of magic number 100 Replace magic number 100 with http.StatusContinue constant for better code clarity and maintainability in bodyAllowedForStatus function. Also fix typo in logger_test.go: colorForLantency -> colorForLatency Fixes #4489 * test: improve coverage to meet 99% threshold - Add TestContextGetRawDataNilBody to cover nil body error case - Add SameSiteDefaultMode test case in SetCookieData - Add 400ms latency test case for LatencyColor - Add TestMarshalXMLforHSuccess for successful XML marshaling Coverage improved from 99.1% to 99.2% * fix: use require for error assertions (testifylint) --- context.go | 1 + context_test.go | 29 +++++++++++++++++++++++++++++ logger_test.go | 17 +++++++++-------- utils_test.go | 12 ++++++++++++ 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/context.go b/context.go index a00d1e55..92fb3704 100644 --- a/context.go +++ b/context.go @@ -1056,6 +1056,7 @@ func (c *Context) requestHeader(key string) string { /************************************/ // bodyAllowedForStatus is a copy of http.bodyAllowedForStatus non-exported function. +// Uses http.StatusContinue constant for better code clarity. func bodyAllowedForStatus(status int) bool { switch { case status >= http.StatusContinue && status < http.StatusOK: diff --git a/context_test.go b/context_test.go index 41ec7bd5..44db7475 100644 --- a/context_test.go +++ b/context_test.go @@ -1031,6 +1031,7 @@ func TestContextGetCookie(t *testing.T) { } func TestContextBodyAllowedForStatus(t *testing.T) { + assert.False(t, bodyAllowedForStatus(http.StatusContinue)) assert.False(t, bodyAllowedForStatus(http.StatusProcessing)) assert.False(t, bodyAllowedForStatus(http.StatusNoContent)) assert.False(t, bodyAllowedForStatus(http.StatusNotModified)) @@ -2947,6 +2948,16 @@ func TestContextGetRawData(t *testing.T) { assert.Equal(t, "Fetch binary post data", string(data)) } +func TestContextGetRawDataNilBody(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", nil) + + data, err := c.GetRawData() + assert.Nil(t, data) + require.Error(t, err) + assert.Equal(t, "cannot read nil body", err.Error()) +} + func TestContextRenderDataFromReader(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) @@ -3535,6 +3546,24 @@ func TestContextSetCookieData(t *testing.T) { setCookie := c.Writer.Header().Get("Set-Cookie") assert.Contains(t, setCookie, "SameSite=None") }) + + // Test that SameSiteDefaultMode inherits from context's sameSite + t.Run("SameSiteDefaultMode inherits context sameSite", func(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.SetSameSite(http.SameSiteStrictMode) + cookie := &http.Cookie{ + Name: "user", + Value: "gin", + Path: "/", + Domain: "localhost", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteDefaultMode, + } + c.SetCookieData(cookie) + setCookie := c.Writer.Header().Get("Set-Cookie") + assert.Contains(t, setCookie, "SameSite=Strict") + }) } func TestGetMapFromFormData(t *testing.T) { diff --git a/logger_test.go b/logger_test.go index 8099a894..395d97e6 100644 --- a/logger_test.go +++ b/logger_test.go @@ -318,20 +318,21 @@ func TestColorForStatus(t *testing.T) { } func TestColorForLatency(t *testing.T) { - colorForLantency := func(latency time.Duration) string { + colorForLatency := func(latency time.Duration) string { p := LogFormatterParams{ Latency: latency, } return p.LatencyColor() } - assert.Equal(t, white, colorForLantency(time.Duration(0)), "0 should be white") - assert.Equal(t, white, colorForLantency(time.Millisecond*20), "20ms should be white") - assert.Equal(t, green, colorForLantency(time.Millisecond*150), "150ms should be green") - assert.Equal(t, cyan, colorForLantency(time.Millisecond*250), "250ms should be cyan") - assert.Equal(t, yellow, colorForLantency(time.Millisecond*600), "600ms should be yellow") - assert.Equal(t, magenta, colorForLantency(time.Millisecond*1500), "1.5s should be magenta") - assert.Equal(t, red, colorForLantency(time.Second*3), "other things should be red") + assert.Equal(t, white, colorForLatency(time.Duration(0)), "0 should be white") + assert.Equal(t, white, colorForLatency(time.Millisecond*20), "20ms should be white") + assert.Equal(t, green, colorForLatency(time.Millisecond*150), "150ms should be green") + assert.Equal(t, cyan, colorForLatency(time.Millisecond*250), "250ms should be cyan") + assert.Equal(t, blue, colorForLatency(time.Millisecond*400), "400ms should be blue") + assert.Equal(t, yellow, colorForLatency(time.Millisecond*600), "600ms should be yellow") + assert.Equal(t, magenta, colorForLatency(time.Millisecond*1500), "1.5s should be magenta") + assert.Equal(t, red, colorForLatency(time.Second*3), "other things should be red") } func TestResetColor(t *testing.T) { diff --git a/utils_test.go b/utils_test.go index 893ebc88..e1f2c332 100644 --- a/utils_test.go +++ b/utils_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func init() { @@ -145,6 +146,17 @@ func TestMarshalXMLforH(t *testing.T) { assert.Error(t, e) } +func TestMarshalXMLforHSuccess(t *testing.T) { + h := H{ + "key1": "value1", + "key2": 123, + } + data, err := xml.Marshal(h) + require.NoError(t, err) + assert.Contains(t, string(data), "value1") + assert.Contains(t, string(data), "123") +} + func TestIsASCII(t *testing.T) { assert.True(t, isASCII("test")) assert.False(t, isASCII("๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ")) From 472d086af2acd924cb4b9d7be0525f7d790f69bc Mon Sep 17 00:00:00 2001 From: Varun Chawla <34209028+veeceey@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:15:27 -0800 Subject: [PATCH 5/5] fix(tree): panic in findCaseInsensitivePathRec with RedirectFixedPath (#4535) * fix(tree): panic in findCaseInsensitivePathRec with RedirectFixedPath enabled When RedirectFixedPath is enabled and a request doesn't match any route, findCaseInsensitivePathRec panics with "invalid node type". This happens because it accesses children[0] to find the wildcard child, but addChild() keeps the wildcard child at the end of the array (not the beginning). When a node has both static and wildcard children, children[0] is a static node, so the switch on nType falls through to the default panic case. The fix mirrors what getValue() already does correctly (line 483): use children[len(children)-1] to access the wildcard child. Fixes #2959 * Address review feedback: improve test assertions and prefer static routes in case-insensitive lookup - Assert found=false for non-matching paths instead of only checking for panics - Fix expected casing for case-insensitive static route match (/PREFIX/XXX -> /prefix/xxx) - Update findCaseInsensitivePathRec to try static children before falling back to wildcard child, ensuring static routes win over param routes in case-insensitive matching --------- Co-authored-by: Bo-Yi Wu --- tree.go | 67 ++++++++++++++++++++++++++++++++++++- tree_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/tree.go b/tree.go index 3ac0a3b1..580abbaf 100644 --- a/tree.go +++ b/tree.go @@ -818,7 +818,72 @@ walk: // Outer loop for walking the tree return nil } - n = n.children[0] + // When wildChild is true, try static children first (via indices) + // before falling back to the wildcard child. This ensures that + // case-insensitive lookups prefer static routes over param routes + // (e.g., /PREFIX/XXX should resolve to /prefix/xxx, not match :id). + if len(n.indices) > 0 { + rb = shiftNRuneBytes(rb, npLen) + + if rb[0] != 0 { + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + } else { + var rv rune + var off int + for max_ := min(npLen, 3); off < max_; off++ { + if i := npLen - off; utf8.RuneStart(oldPath[i]) { + rv, _ = utf8.DecodeRuneInString(oldPath[i:]) + break + } + } + + lo := unicode.ToLower(rv) + utf8.EncodeRune(rb[:], lo) + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + + if up := unicode.ToUpper(rv); up != lo { + utf8.EncodeRune(rb[:], up) + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + } + } + } + + // Fall back to wildcard child, which is always at the end of the array + n = n.children[len(n.children)-1] switch n.nType { case param: // Find param end (either '/' or path end) diff --git a/tree_test.go b/tree_test.go index b580007d..23339af4 100644 --- a/tree_test.go +++ b/tree_test.go @@ -1018,3 +1018,96 @@ func TestWildcardInvalidSlash(t *testing.T) { } } } + +func TestTreeFindCaseInsensitivePathWithMultipleChildrenAndWildcard(t *testing.T) { + tree := &node{} + + // Setup routes that create a node with both static children and a wildcard child. + // This configuration previously caused a panic ("invalid node type") in + // findCaseInsensitivePathRec because it accessed children[0] instead of the + // wildcard child (which is always at the end of the children array). + // See: https://github.com/gin-gonic/gin/issues/2959 + routes := [...]string{ + "/aa/aa", + "/:bb/aa", + } + + for _, route := range routes { + recv := catchPanic(func() { + tree.addRoute(route, fakeHandler(route)) + }) + if recv != nil { + t.Fatalf("panic inserting route '%s': %v", route, recv) + } + } + + // These lookups previously panicked with "invalid node type" because + // findCaseInsensitivePathRec picked children[0] (a static node) instead + // of the wildcard child at the end of the array. + out, found := tree.findCaseInsensitivePath("/aa", true) + if found { + t.Errorf("Expected no match for '/aa', but got: %s", string(out)) + } + + out, found = tree.findCaseInsensitivePath("/aa/aa/aa/aa", true) + if found { + t.Errorf("Expected no match for '/aa/aa/aa/aa', but got: %s", string(out)) + } + + // Case-insensitive lookup should match the static route /aa/aa + out, found = tree.findCaseInsensitivePath("/AA/AA", true) + if !found { + t.Error("Route '/AA/AA' not found via case-insensitive lookup") + } else if string(out) != "/aa/aa" { + t.Errorf("Wrong result for '/AA/AA': expected '/aa/aa', got: %s", string(out)) + } +} + +func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) { + tree := &node{} + + // Another variant: param route + static route under same prefix + routes := [...]string{ + "/prefix/:id", + "/prefix/xxx", + } + + for _, route := range routes { + recv := catchPanic(func() { + tree.addRoute(route, fakeHandler(route)) + }) + if recv != nil { + t.Fatalf("panic inserting route '%s': %v", route, recv) + } + } + + // Should NOT panic even for paths that don't match any route + out, found := tree.findCaseInsensitivePath("/prefix/a/b/c", true) + if found { + t.Errorf("Expected no match for '/prefix/a/b/c', but got: %s", string(out)) + } + + // Exact match should still work + out, found = tree.findCaseInsensitivePath("/prefix/xxx", true) + if !found { + t.Error("Route '/prefix/xxx' not found") + } else if string(out) != "/prefix/xxx" { + t.Errorf("Wrong result for '/prefix/xxx': %s", string(out)) + } + + // Case-insensitive match should work + out, found = tree.findCaseInsensitivePath("/PREFIX/XXX", true) + if !found { + t.Error("Route '/PREFIX/XXX' not found via case-insensitive lookup") + } else if string(out) != "/prefix/xxx" { + t.Errorf("Wrong result for '/PREFIX/XXX': expected '/prefix/xxx', got: %s", string(out)) + } + + // Param route should still match + out, found = tree.findCaseInsensitivePath("/prefix/something", true) + if !found { + t.Error("Route '/prefix/something' not found via param match") + } else if string(out) != "/prefix/something" { + t.Errorf("Wrong result for '/prefix/something': %s", string(out)) + } +}