diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 750eb143e3d..3122f12b414 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -42,15 +42,28 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, zap.String("path", dirPath), zap.String("root", root)) - // navigation on the client-side gets messed up if the - // URL doesn't end in a trailing slash because hrefs like - // "/b/c" on a path like "/a" end up going to "/b/c" instead + // Navigation on the client-side gets messed up if the + // URL doesn't end in a trailing slash because hrefs to + // "b/c" at path "/a" end up going to "/b/c" instead // of "/a/b/c" - so we have to redirect in this case - if !strings.HasSuffix(r.URL.Path, "/") { - fsrv.logger.Debug("redirecting to trailing slash to preserve hrefs", zap.String("request_path", r.URL.Path)) - r.URL.Path += "/" - http.Redirect(w, r, r.URL.String(), http.StatusMovedPermanently) - return nil + // so that the path is "/a/" and the client constructs + // relative hrefs "b/c" to be "/a/b/c". + // + // Only redirect if the last element of the path (the filename) was not + // rewritten; if the admin wanted to rewrite to the canonical path, they + // would have, and we have to be very careful not to introduce unwanted + // redirects and especially redirect loops! (Redirecting using the + // original URI is necessary because that's the URI the browser knows, + // we don't want to redirect from internally-rewritten URIs.) + // See https://github.com/caddyserver/caddy/issues/4205. + origReq := r.Context().Value(caddyhttp.OriginalRequestCtxKey).(http.Request) + if path.Base(origReq.URL.Path) == path.Base(r.URL.Path) { + if !strings.HasSuffix(origReq.URL.Path, "/") { + fsrv.logger.Debug("redirecting to trailing slash to preserve hrefs", zap.String("request_path", r.URL.Path)) + origReq.URL.Path += "/" + http.Redirect(w, r, origReq.URL.String(), http.StatusMovedPermanently) + return nil + } } dir, err := fsrv.openFile(dirPath, w) diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index f151e323e87..9266332a0f5 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -20,6 +20,7 @@ import ( "mime" "net/http" "os" + "path" "path/filepath" "strconv" "strings" @@ -240,12 +241,26 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c // trailing slash - not enforcing this can break relative hrefs // in HTML (see https://github.com/caddyserver/caddy/issues/2741) if fsrv.CanonicalURIs == nil || *fsrv.CanonicalURIs { - if implicitIndexFile && !strings.HasSuffix(r.URL.Path, "/") { - fsrv.logger.Debug("redirecting to canonical URI (adding trailing slash for directory)", zap.String("path", r.URL.Path)) - return redirect(w, r, r.URL.Path+"/") - } else if !implicitIndexFile && strings.HasSuffix(r.URL.Path, "/") { - fsrv.logger.Debug("redirecting to canonical URI (removing trailing slash for file)", zap.String("path", r.URL.Path)) - return redirect(w, r, r.URL.Path[:len(r.URL.Path)-1]) + // Only redirect if the last element of the path (the filename) was not + // rewritten; if the admin wanted to rewrite to the canonical path, they + // would have, and we have to be very careful not to introduce unwanted + // redirects and especially redirect loops! + // See https://github.com/caddyserver/caddy/issues/4205. + origReq := r.Context().Value(caddyhttp.OriginalRequestCtxKey).(http.Request) + if path.Base(origReq.URL.Path) == path.Base(r.URL.Path) { + if implicitIndexFile && !strings.HasSuffix(origReq.URL.Path, "/") { + to := origReq.URL.Path + "/" + fsrv.logger.Debug("redirecting to canonical URI (adding trailing slash for directory)", + zap.String("from_path", origReq.URL.Path), + zap.String("to_path", to)) + return redirect(w, r, to) + } else if !implicitIndexFile && strings.HasSuffix(origReq.URL.Path, "/") { + to := origReq.URL.Path[:len(origReq.URL.Path)-1] + fsrv.logger.Debug("redirecting to canonical URI (removing trailing slash for file)", + zap.String("from_path", origReq.URL.Path), + zap.String("to_path", to)) + return redirect(w, r, to) + } } }