From 3477bfd234523601e2788a51bb365422448278ed Mon Sep 17 00:00:00 2001
From: Andrew Lytvynov <awly@tailscale.com>
Date: Thu, 31 Oct 2024 13:12:38 -0500
Subject: [PATCH] safeweb: add support for "/" and "/foo" handler distinction
 (#13980)

By counting "/" elements in the pattern we catch many scenarios, but not
the root-level handler. If either of the patterns is "/", compare the
pattern length to pick the right one.

Updates https://github.com/tailscale/corp/issues/8027

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
---
 safeweb/http.go      | 17 ++++++++++++++++-
 safeweb/http_test.go | 10 ++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/safeweb/http.go b/safeweb/http.go
index 14c61336a..d8818d386 100644
--- a/safeweb/http.go
+++ b/safeweb/http.go
@@ -225,12 +225,27 @@ const (
 	browserHandler
 )
 
+func (h handlerType) String() string {
+	switch h {
+	case browserHandler:
+		return "browser"
+	case apiHandler:
+		return "api"
+	default:
+		return "unknown"
+	}
+}
+
 // checkHandlerType returns either apiHandler or browserHandler, depending on
 // whether apiPattern or browserPattern is more specific (i.e. which pattern
 // contains more pathname components). If they are equally specific, it returns
 // unknownHandler.
 func checkHandlerType(apiPattern, browserPattern string) handlerType {
-	c := cmp.Compare(strings.Count(path.Clean(apiPattern), "/"), strings.Count(path.Clean(browserPattern), "/"))
+	apiPattern, browserPattern = path.Clean(apiPattern), path.Clean(browserPattern)
+	c := cmp.Compare(strings.Count(apiPattern, "/"), strings.Count(browserPattern, "/"))
+	if apiPattern == "/" || browserPattern == "/" {
+		c = cmp.Compare(len(apiPattern), len(browserPattern))
+	}
 	switch {
 	case c > 0:
 		return apiHandler
diff --git a/safeweb/http_test.go b/safeweb/http_test.go
index cec14b2b9..a2e2d7644 100644
--- a/safeweb/http_test.go
+++ b/safeweb/http_test.go
@@ -527,13 +527,13 @@ func TestGetMoreSpecificPattern(t *testing.T) {
 		{
 			desc: "same prefix",
 			a:    "/foo/bar/quux",
-			b:    "/foo/bar/",
+			b:    "/foo/bar/", // path.Clean will strip the trailing slash.
 			want: apiHandler,
 		},
 		{
 			desc: "almost same prefix, but not a path component",
 			a:    "/goat/sheep/cheese",
-			b:    "/goat/sheepcheese/",
+			b:    "/goat/sheepcheese/", // path.Clean will strip the trailing slash.
 			want: apiHandler,
 		},
 		{
@@ -554,6 +554,12 @@ func TestGetMoreSpecificPattern(t *testing.T) {
 			b:    "///////",
 			want: unknownHandler,
 		},
+		{
+			desc: "root-level",
+			a:    "/latest",
+			b:    "/", // path.Clean will NOT strip the trailing slash.
+			want: apiHandler,
+		},
 	} {
 		t.Run(tt.desc, func(t *testing.T) {
 			got := checkHandlerType(tt.a, tt.b)