added decoding and normalizing urlencoded chars when registering endpoints. Disabled decoding urlencoded slash

Author: flrdv

internal/strutil/url.go

@@ -0,0 +1,54 @@
+package strutil
+
+import (
+	"strings"
+
+	"github.com/indigo-web/indigo/internal/hexconv"
+)
+
+// IsURLUnsafeChar tells whether it's safe to decode an urlencoded character.
+func IsURLUnsafeChar(c byte) bool {
+	return c == '/'
+}
+
+// URLDecode decodes an urlencoded string and tells whether the string was properly formed.
+func URLDecode(str string) (string, bool) {
+	var b strings.Builder
+	b.Grow(len(str))
+	s := str
+
+	for len(s) > 0 {
+		percent := strings.IndexByte(s, '%')
+		if percent == -1 {
+			break
+		}
+
+		b.WriteString(s[:percent])
+		s = s[percent+1:]
+		if len(s) < 2 {
+			return "", false
+		}
+
+		c1, c2 := s[0], s[1]
+		s = s[2:]
+		x, y := hexconv.Halfbyte[c1], hexconv.Halfbyte[c2]
+		if x|y == 0xFF {
+			return "", false
+		}
+
+		char := (x << 4) | y
+		if IsASCIINonprintable(char) {
+			return "", false
+		}
+		if IsURLUnsafeChar(char) {
+			b.Write([]byte{'%', c1 | 0x20, c2 | 0x20})
+			continue
+		}
+
+		b.WriteByte(char)
+	}
+
+	b.WriteString(s)
+
+	return b.String(), true
+}

internal/strutil/url_test.go

@@ -0,0 +1,33 @@
+package strutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestURLDecode(t *testing.T) {
+	t.Run("base", func(t *testing.T) {
+		res, ok := URLDecode("%61")
+		require.True(t, ok)
+		require.Equal(t, "a", res)
+
+		for i, tc := range []string{"abc", "%61bc", "a%62c", "ab%63", "%61%62%63"} {
+			res, ok = URLDecode(tc)
+			require.True(t, ok, i)
+			require.Equal(t, "abc", res, i)
+		}
+	})
+
+	t.Run("unsafe char", func(t *testing.T) {
+		res, ok := URLDecode("%61%2f")
+		require.True(t, ok)
+		require.Equal(t, "a%2f", res)
+	})
+
+	t.Run("unsafe normalization", func(t *testing.T) {
+		res, ok := URLDecode("%61%2F")
+		require.True(t, ok)
+		require.Equal(t, "a%2f", res)
+	})
+}

router/inbuilt/inbuilt_test.go

@@ -141,6 +141,38 @@ func TestRoute(t *testing.T) {
 		t.Run("server-wide", testOPTIONS("*", "GET, HEAD, OPTIONS", false))
 		t.Run("server-wide with TRACE", testOPTIONS("*", "GET, HEAD, OPTIONS, TRACE", true))
 	})
+
+	t.Run("escaping", func(t *testing.T) {
+		testRoute := func(r router.Router, path string) func(t *testing.T) {
+			return func(t *testing.T) {
+				request := getRequest(method.GET, path)
+				resp := r.OnRequest(request)
+				require.Equal(t, 200, int(resp.Expose().Code))
+			}
+		}
+
+		newRouter := func(dynamic bool) router.Router {
+			r := New().
+				Get("/foo%2fbar", http.Respond).
+				Get("/foo%3abar", http.Respond)
+
+			if dynamic {
+				r.Get("/unreachable route/:", http.Respond)
+			}
+
+			return r.Build()
+		}
+
+		test := func(r router.Router) func(t *testing.T) {
+			return func(t *testing.T) {
+				t.Run("escaped slash", testRoute(r, "/foo%2fbar"))
+				t.Run("unescaped colon", testRoute(r, "/foo:bar"))
+			}
+		}
+
+		t.Run("static", test(newRouter(false)))
+		t.Run("dynamic", test(newRouter(true)))
+	})
 }
 
 func TestDynamic(t *testing.T) {

router/inbuilt/internal/radix/tree.go

@@ -3,9 +3,12 @@ package radix
 import (
 	"cmp"
 	"errors"
+	"fmt"
 	"slices"
+	"strconv"
 	"strings"
 
+	"github.com/indigo-web/indigo/internal/strutil"
 	"github.com/indigo-web/indigo/kv"
 )
 
@@ -130,7 +133,12 @@ func addWildcard(wildcard, value string, into *kv.Storage) {
 }
 
 func (n *Node[T]) Insert(key string, value T) error {
-	return n.insert(splitPath(key), value)
+	str, ok := strutil.URLDecode(key)
+	if !ok {
+		return fmt.Errorf("poorly encoded path: %s", strconv.Quote(key))
+	}
+
+	return n.insert(splitPath(str), value)
 }
 
 func (n *Node[T]) insert(segs []pathSegment, value T) error {
@@ -239,24 +247,13 @@ type pathSegment struct {
 }
 
 func splitPath(str string) (path []pathSegment) {
-	offset := 0
-
 	for len(str) > 0 {
-		colon := strings.IndexByte(str[offset:], ':')
+		colon := strings.IndexByte(str, ':')
 		if colon == -1 {
 			path = append(path, pathSegment{false, false, str})
 			break
 		}
 
-		colon += offset
-
-		if colon > 0 && str[colon-1] == '\\' {
-			// escaped colon: isn't a wildcard, skip
-			str = str[:colon-1] + str[colon:]
-			offset = colon
-			continue
-		}
-
 		path = append(path, pathSegment{false, false, str[:colon]})
 		str = str[colon+1:]
 

router/inbuilt/internal/radix/tree_test.go

@@ -228,40 +228,6 @@ func TestTree(t *testing.T) {
 		test(t, tree, "/prefix42", 1, "path", "42")
 		test(t, tree, "/prefixnowhere/like/this", 1, "path", "nowhere/like/this")
 	})
-
-	t.Run("escape wildcard", func(t *testing.T) {
-		t.Run("static leftmost", func(t *testing.T) {
-			tree := New[int]()
-			require.NoError(t, tree.Insert("\\:hello/\\:world", 1))
-			value, found := tree.Lookup(":hello/:world", nil)
-			require.True(t, found)
-			require.Equal(t, 1, value)
-		})
-
-		t.Run("static rightmost", func(t *testing.T) {
-			tree := New[int]()
-			require.NoError(t, tree.Insert("/\\:hello", 1))
-			value, found := tree.Lookup("/:hello", nil)
-			require.True(t, found)
-			require.Equal(t, 1, value)
-		})
-
-		t.Run("dynamic", func(t *testing.T) {
-			tree := New[int]()
-			require.NoError(t, tree.Insert("/\\:hello/:name", 1))
-			wildcards := kv.New()
-			value, found := tree.Lookup("/:hello/Pavlo", wildcards)
-			require.True(t, found)
-			require.Equal(t, 1, value)
-			require.Equal(t, "Pavlo", wildcards.Value("name"))
-		})
-	})
-
-	t.Run("path classifier", func(t *testing.T) {
-		require.False(t, IsDynamicTemplate("\\:hello/\\:world"))
-		require.False(t, IsDynamicTemplate("/\\:hello"))
-		require.True(t, IsDynamicTemplate("/\\:hello/:name"))
-	})
 }
 
 // isn't used anymore. Left just in case the tree needs to be debugged.

router/inbuilt/types.go

@@ -1,11 +1,14 @@
 package inbuilt
 
 import (
+	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/indigo-web/indigo/http"
 	"github.com/indigo-web/indigo/http/method"
 	"github.com/indigo-web/indigo/http/status"
+	"github.com/indigo-web/indigo/internal/strutil"
 	"github.com/indigo-web/indigo/router/inbuilt/internal/radix"
 )
 
@@ -26,10 +29,15 @@ type (
 )
 
 func (r routesMap) Add(path string, m method.Method, handler Handler) {
-	entry := r[path]
+	p, ok := strutil.URLDecode(path)
+	if !ok {
+		panic(fmt.Errorf("poorly encoded path: %s", strconv.Quote(path)))
+	}
+
+	entry := r[p]
 	entry.methods[m] = handler
 	entry.allow = getAllowString(entry.methods)
-	r[path] = entry
+	r[p] = entry
 }
 
 func getAllowString(methods methodLUT) (allowed string) {

router/inbuilt/uri/normalize.go

@@ -1,7 +1,6 @@
 package uri
 
-// Normalize removes trailing slashes, as all request paths are also trimmed, resulting
-// in consensus between these two.
+// Normalize eliminates a trailing slash if presented.
 func Normalize(path string) string {
 	if len(path) > 1 && path[len(path)-1] == '/' {
 		return path[:len(path)-1]