diff --git a/caddyhttp/browse/browse.go b/caddyhttp/browse/browse.go index 36064c1d..9da9b860 100644 --- a/caddyhttp/browse/browse.go +++ b/caddyhttp/browse/browse.go @@ -299,7 +299,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { // Browsing navigation gets messed up if browsing a directory // that doesn't end in "/" (which it should, anyway) if !strings.HasSuffix(r.URL.Path, "/") { - staticfiles.Redirect(w, r, r.URL.Path+"/", http.StatusTemporaryRedirect) + staticfiles.RedirectToDir(w, r) return 0, nil } diff --git a/caddyhttp/browse/browse_test.go b/caddyhttp/browse/browse_test.go index c3f945c9..500c6e16 100644 --- a/caddyhttp/browse/browse_test.go +++ b/caddyhttp/browse/browse_test.go @@ -1,6 +1,7 @@ package browse import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -362,3 +363,68 @@ func isReversed(data sort.Interface) bool { } return true } + +func TestBrowseRedirect(t *testing.T) { + testCases := []struct { + url string + statusCode int + returnCode int + location string + }{ + { + "http://www.example.com/photos", + http.StatusMovedPermanently, + 0, + "http://www.example.com/photos/", + }, + { + "/photos", + http.StatusMovedPermanently, + 0, + "/photos/", + }, + } + + for i, tc := range testCases { + b := Browse{ + Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) { + t.Fatalf("Test %d - Next shouldn't be called", i) + return 0, nil + }), + Configs: []Config{ + { + PathScope: "/photos", + Fs: staticfiles.FileServer{ + Root: http.Dir("./testdata"), + }, + }, + }, + } + + req, err := http.NewRequest("GET", tc.url, nil) + u, _ := url.Parse(tc.url) + ctx := context.WithValue(req.Context(), staticfiles.URLPathCtxKey, u.Path) + req = req.WithContext(ctx) + if err != nil { + t.Fatalf("Test %d - could not create HTTP request: %v", i, err) + } + + rec := httptest.NewRecorder() + + returnCode, _ := b.ServeHTTP(rec, req) + if returnCode != tc.returnCode { + t.Fatalf("Test %d - wrong return code, expected %d, got %d", + i, tc.returnCode, returnCode) + } + + if got := rec.Code; got != tc.statusCode { + t.Errorf("Test %d - wrong status, expected %d, got %d", + i, tc.statusCode, got) + } + + if got := rec.Header().Get("Location"); got != tc.location { + t.Errorf("Test %d - wrong Location header, expected %s, got %s", + i, tc.location, got) + } + } +} diff --git a/caddyhttp/staticfiles/fileserver.go b/caddyhttp/staticfiles/fileserver.go index 091090fc..b8bbc73b 100644 --- a/caddyhttp/staticfiles/fileserver.go +++ b/caddyhttp/staticfiles/fileserver.go @@ -96,30 +96,14 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name stri // Ensure / at end of directory url. If the original URL path is // used then ensure / exists as well. if !strings.HasSuffix(r.URL.Path, "/") { - toURL, _ := url.Parse(r.URL.String()) - - path, ok := r.Context().Value(URLPathCtxKey).(string) - if ok && !strings.HasSuffix(path, "/") { - toURL.Path = path - } - toURL.Path += "/" - - http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) + RedirectToDir(w, r) return http.StatusMovedPermanently, nil } } else { // Ensure no / at end of file url. If the original URL path is // used then ensure no / exists as well. if strings.HasSuffix(r.URL.Path, "/") { - toURL, _ := url.Parse(r.URL.String()) - - path, ok := r.Context().Value(URLPathCtxKey).(string) - if ok && strings.HasSuffix(path, "/") { - toURL.Path = path - } - toURL.Path = strings.TrimSuffix(toURL.Path, "/") - - http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) + RedirectToFile(w, r) return http.StatusMovedPermanently, nil } } @@ -234,14 +218,34 @@ func (fs FileServer) IsHidden(d os.FileInfo) bool { return false } -// Redirect sends an HTTP redirect to the client but will preserve -// the query string for the new path. Based on http.localRedirect -// from the Go standard library. -func Redirect(w http.ResponseWriter, r *http.Request, newPath string, statusCode int) { - if q := r.URL.RawQuery; q != "" { - newPath += "?" + q +// RedirectToDir replies to the request with a redirect to the URL in r, which +// has been transformed to indicate that the resource being requested is a +// directory. +func RedirectToDir(w http.ResponseWriter, r *http.Request) { + toURL, _ := url.Parse(r.URL.String()) + + path, ok := r.Context().Value(URLPathCtxKey).(string) + if ok && !strings.HasSuffix(path, "/") { + toURL.Path = path } - http.Redirect(w, r, newPath, statusCode) + toURL.Path += "/" + + http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) +} + +// RedirectToFile replies to the request with a redirect to the URL in r, which +// has been transformed to indicate that the resource being requested is a +// file. +func RedirectToFile(w http.ResponseWriter, r *http.Request) { + toURL, _ := url.Parse(r.URL.String()) + + path, ok := r.Context().Value(URLPathCtxKey).(string) + if ok && strings.HasSuffix(path, "/") { + toURL.Path = path + } + toURL.Path = strings.TrimSuffix(toURL.Path, "/") + + http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) } // IndexPages is a list of pages that may be understood as