Skip to content

Commit

Permalink
golink: don't modify URL path when resolving link
Browse files Browse the repository at this point in the history
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
  • Loading branch information
willnorris committed Nov 13, 2023
1 parent 2ff7d04 commit 0c4acb0
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
40 changes: 29 additions & 11 deletions golink.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 == "" {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion golink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 0c4acb0

Please sign in to comment.