From cf959d79ff04b23b640a7b1af860f2d3bd6c6f47 Mon Sep 17 00:00:00 2001 From: Agnes-George1 Date: Mon, 24 Nov 2025 16:30:09 +0530 Subject: [PATCH] worked on review comments --- tree.go | 53 +++++++++++++++++++++++++++------------------------- tree_test.go | 16 ++++++++-------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/tree.go b/tree.go index 6d2fcebf..283cbb42 100644 --- a/tree.go +++ b/tree.go @@ -85,16 +85,37 @@ func (n *node) addChild(child *node) { } func countParams(path string) uint16 { - var n uint16 s := bytesconv.StringToBytes(path) - n += uint16(bytes.Count(s, strColon)) - n += uint16(bytes.Count(s, strStar)) - return n + colons := bytes.Count(s, strColon) + stars := bytes.Count(s, strStar) + total := colons + stars + // Cap at max uint16 to prevent overflow + if total > 0xFFFF { + return 0xFFFF + } + return uint16(total) } func countSections(path string) uint16 { s := bytesconv.StringToBytes(path) - return uint16(bytes.Count(s, strSlash)) + count := bytes.Count(s, strSlash) + // Cap at max uint16 to prevent overflow + if count > 0xFFFF { + return 0xFFFF + } + return uint16(count) +} + +// unescapePathValue unescapes a path parameter value if unescape is enabled. +// Only unescapes if the value contains percent-encoded characters or plus signs. +// This prevents double unescaping and potential path traversal vulnerabilities. +func unescapePathValue(val string, unescape bool) string { + if unescape && strings.ContainsAny(val, "%+") { + if v, err := url.QueryUnescape(val); err == nil { + return v + } + } + return val } type nodeType uint8 @@ -519,16 +540,7 @@ walk: // Outer loop for walking the tree // Expand slice within preallocated capacity i := len(*value.params) *value.params = (*value.params)[:i+1] - val := path[:end] - if unescape { - // Only unescape if the value contains percent-encoded characters or plus signs - // This prevents double unescaping and potential path traversal - if strings.ContainsAny(val, "%+") { - if v, err := url.QueryUnescape(val); err == nil { - val = v - } - } - } + val := unescapePathValue(path[:end], unescape) (*value.params)[i] = Param{ Key: n.path[1:], Value: val, @@ -576,16 +588,7 @@ walk: // Outer loop for walking the tree // Expand slice within preallocated capacity i := len(*value.params) *value.params = (*value.params)[:i+1] - val := path - if unescape { - // Only unescape if the value contains percent-encoded characters or plus signs - // This prevents double unescaping and potential path traversal - if strings.ContainsAny(path, "%+") { - if v, err := url.QueryUnescape(path); err == nil { - val = v - } - } - } + val := unescapePathValue(path, unescape) (*value.params)[i] = Param{ Key: n.path[2:], Value: val, diff --git a/tree_test.go b/tree_test.go index 51fe8c6d..09c46d27 100644 --- a/tree_test.go +++ b/tree_test.go @@ -377,16 +377,16 @@ func TestSecureParameterHandling(t *testing.T) { checkRequests(t, tree, testRequests{ // Normal case - single encoding works as expected {"/info/user%2Fprofile", false, "/info/:user", Params{Param{Key: "user", Value: "user/profile"}}}, - + // Double encoding - should only decode once {"/info/user%252Fprofile", false, "/info/:user", Params{Param{Key: "user", Value: "user%2Fprofile"}}}, - + // Triple encoding - should only decode once {"/info/user%25252Fprofile", false, "/info/:user", Params{Param{Key: "user", Value: "user%252Fprofile"}}}, - + // Mixed encoding - should only decode once {"/info/%2Fuser%252Fprofile", false, "/info/:user", Params{Param{Key: "user", Value: "/user%2Fprofile"}}}, - + // No encoding - should pass through unchanged {"/info/user", false, "/info/:user", Params{Param{Key: "user", Value: "user"}}}, }, unescape) @@ -396,16 +396,16 @@ func TestSecureParameterHandling(t *testing.T) { checkRequests(t, tree, testRequests{ // Normal case - single encoding works as expected {"/files/path%2Fto%2Ffile.txt", false, "/files/*filepath", Params{Param{Key: "filepath", Value: "/path/to/file.txt"}}}, - + // Double encoding - should only decode once {"/files/path%252Fto%252Ffile.txt", false, "/files/*filepath", Params{Param{Key: "filepath", Value: "/path%2Fto%2Ffile.txt"}}}, - + // Triple encoding - should only decode once {"/files/path%25252Fto%25252Ffile.txt", false, "/files/*filepath", Params{Param{Key: "filepath", Value: "/path%252Fto%252Ffile.txt"}}}, - + // Mixed encoding - should only decode once {"/files/%2Fpath%252Fto%2Ffile.txt", false, "/files/*filepath", Params{Param{Key: "filepath", Value: "//path%2Fto/file.txt"}}}, - + // No encoding - should pass through unchanged {"/files/normal/file.txt", false, "/files/*filepath", Params{Param{Key: "filepath", Value: "/normal/file.txt"}}}, }, unescape)