From 0c4acb0c6261b3ecc99ea8385076b86bd9874102 Mon Sep 17 00:00:00 2001
From: Will Norris <will@tailscale.com>
Date: Mon, 13 Nov 2023 14:58:34 -0800
Subject: [PATCH] golink: don't modify URL path when resolving link

Both http.ServeMux as well as the http.Redirect method pass the request
URL through `cleanPath` which, among other things, collapses double
slashes `//` to a single slash `/`. Most of the time this is fine, since
most servers treat those as identical anyway. But some destination
servers need the original path unmodified. Since we're just redirecting,
we don't need to be concerned with the additional benefits of
`cleanPath` such as eliminating `../` path components, since that is the
responsibility of the destination server to clean if needed.

This change adds a separate root http.Handler for golink requests. It
still uses http.ServeMux for internal endpoints, but serves golinks
directly without passing the request through ServeMux. Additionally,
this sets the redirect status and Location header directly rather than
calling http.Redirect, since that also modifies the URL it is given.

Fixes #89
---
 golink.go      | 40 +++++++++++++++++++++++++++++-----------
 golink_test.go |  8 +++++++-
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/golink.go b/golink.go
index 7c499a0..757c45b 100644
--- a/golink.go
+++ b/golink.go
@@ -139,15 +139,6 @@ func Run() error {
 	// flush stats periodically
 	go flushStatsLoop()
 
-	http.HandleFunc("/", serveGo)
-	http.HandleFunc("/.detail/", serveDetail)
-	http.HandleFunc("/.export", serveExport)
-	http.HandleFunc("/.help", serveHelp)
-	http.HandleFunc("/.opensearch", serveOpenSearch)
-	http.HandleFunc("/.all", serveAll)
-	http.HandleFunc("/.delete/", serveDelete)
-	http.Handle("/.static/", http.StripPrefix("/.", http.FileServer(http.FS(embeddedFS))))
-
 	if *dev != "" {
 		// override default hostname for dev mode
 		if *hostname == defaultHostname {
@@ -160,7 +151,7 @@ func Run() error {
 		}
 
 		log.Printf("Running in dev mode on %s ...", *dev)
-		log.Fatal(http.ListenAndServe(*dev, nil))
+		log.Fatal(http.ListenAndServe(*dev, serveHandler()))
 	}
 
 	if *hostname == "" {
@@ -295,6 +286,29 @@ func deleteLinkStats(link *Link) {
 	db.DeleteStats(link.Short)
 }
 
+// serverHandler returns the main http.Handler for serving all requests.
+func serveHandler() http.Handler {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/.detail/", serveDetail)
+	mux.HandleFunc("/.export", serveExport)
+	mux.HandleFunc("/.help", serveHelp)
+	mux.HandleFunc("/.opensearch", serveOpenSearch)
+	mux.HandleFunc("/.all", serveAll)
+	mux.HandleFunc("/.delete/", serveDelete)
+	mux.Handle("/.static/", http.StripPrefix("/.", http.FileServer(http.FS(embeddedFS))))
+
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// all internal URLs begin with a leading "."; any other URL is treated as a go link.
+		// Serve go links directly without passing through the ServeMux,
+		// which sometimes modifies the request URL path, which we don't want.
+		if !strings.HasPrefix(r.URL.Path, "/.") {
+			serveGo(w, r)
+			return
+		}
+		mux.ServeHTTP(w, r)
+	})
+}
+
 func serveHome(w http.ResponseWriter, short string) {
 	var clicks []visitData
 
@@ -408,7 +422,11 @@ func serveGo(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	http.Redirect(w, r, target.String(), http.StatusFound)
+
+	// http.Redirect always cleans the redirect URL, which we don't always want.
+	// Instead, manually set status and Location header.
+	w.WriteHeader(http.StatusFound)
+	w.Header().Set("Location", target.String())
 }
 
 // acceptHTML returns whether the request can accept a text/html response.
diff --git a/golink_test.go b/golink_test.go
index e67d502..9506add 100644
--- a/golink_test.go
+++ b/golink_test.go
@@ -69,6 +69,12 @@ func TestServeGo(t *testing.T) {
 			wantStatus: http.StatusFound,
 			wantLink:   "http://who/p?q=1",
 		},
+		{
+			name:       "simple link with double slash in path",
+			link:       "/who/http://host",
+			wantStatus: http.StatusFound,
+			wantLink:   "http://who/http://host",
+		},
 		{
 			name:       "user link",
 			link:       "/me",
@@ -105,7 +111,7 @@ func TestServeGo(t *testing.T) {
 
 			r := httptest.NewRequest("GET", tt.link, nil)
 			w := httptest.NewRecorder()
-			serveGo(w, r)
+			serveHandler().ServeHTTP(w, r)
 
 			if w.Code != tt.wantStatus {
 				t.Errorf("serveGo(%q) = %d; want %d", tt.link, w.Code, tt.wantStatus)