From 9d54f655aa7698ce683786dfc31b6f11df3c89d2 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 21 May 2019 13:03:52 -0600 Subject: Take care of remaining TODOs in the browse responder --- modules/caddyhttp/fileserver/browse.go | 56 ++++++++++++--------------- modules/caddyhttp/fileserver/browselisting.go | 2 +- modules/caddyhttp/fileserver/matcher.go | 4 +- modules/caddyhttp/fileserver/staticfiles.go | 4 +- 4 files changed, 28 insertions(+), 38 deletions(-) (limited to 'modules/caddyhttp/fileserver') diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index ed5ad42..9869801 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -21,6 +21,16 @@ type Browse struct { } func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *http.Request) error { + // 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 + // of "/a/b/c" - so we have to redirect in this case + if !strings.HasSuffix(r.URL.Path, "/") { + r.URL.Path += "/" + http.Redirect(w, r, r.URL.String(), http.StatusMovedPermanently) + return nil + } + dir, err := fsrv.openFile(dirPath, w) if err != nil { return err @@ -29,7 +39,8 @@ func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *ht repl := r.Context().Value(caddy2.ReplacerCtxKey).(caddy2.Replacer) - listing, err := fsrv.loadDirectoryContents(dir, r.URL.Path, repl) + // calling path.Clean here prevents weird breadcrumbs when URL paths are sketchy like /%2e%2e%2f + listing, err := fsrv.loadDirectoryContents(dir, path.Clean(r.URL.Path), repl) switch { case os.IsPermission(err): return caddyhttp.Error(http.StatusForbidden, err) @@ -58,21 +69,6 @@ func (fsrv *FileServer) serveBrowse(dirPath string, w http.ResponseWriter, r *ht buf.WriteTo(w) return nil - - // TODO: Sigh... do we have to put this here? - // // Browsing navigation gets messed up if browsing a directory - // // that doesn't end in "/" (which it should, anyway) - // u := *r.URL - // if u.Path == "" { - // u.Path = "/" - // } - // if u.Path[len(u.Path)-1] != '/' { - // u.Path += "/" - // http.Redirect(w, r, u.String(), http.StatusMovedPermanently) - // return http.StatusMovedPermanently, nil - // } - - // return b.ServeListing(w, r, requestedFilepath, bc) } func (fsrv *FileServer) loadDirectoryContents(dir *os.File, urlPath string, repl caddy2.Replacer) (browseListing, error) { @@ -138,20 +134,16 @@ func isSymlink(f os.FileInfo) bool { return f.Mode()&os.ModeSymlink != 0 } -// isSymlinkTargetDir return true if f's symbolic link target -// is a directory. Return false if not a symbolic link. -// TODO: Re-implement -func isSymlinkTargetDir(f os.FileInfo, urlPath string) bool { - // if !isSymlink(f) { - // return false - // } - - // // TODO: Ensure path is sanitized - // target:= path.Join(root, urlPath, f.Name())) - // targetInfo, err := os.Stat(target) - // if err != nil { - // return false - // } - // return targetInfo.IsDir() - return false +// isSymlinkTargetDir returns true if f's symbolic link target +// is a directory. +func isSymlinkTargetDir(f os.FileInfo, root, urlPath string) bool { + if !isSymlink(f) { + return false + } + target := sanitizedPathJoin(root, path.Join(urlPath, f.Name())) + targetInfo, err := os.Stat(target) + if err != nil { + return false + } + return targetInfo.IsDir() } diff --git a/modules/caddyhttp/fileserver/browselisting.go b/modules/caddyhttp/fileserver/browselisting.go index e17b7ae..94705b3 100644 --- a/modules/caddyhttp/fileserver/browselisting.go +++ b/modules/caddyhttp/fileserver/browselisting.go @@ -28,7 +28,7 @@ func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, urlP continue } - isDir := f.IsDir() || isSymlinkTargetDir(f, urlPath) + isDir := f.IsDir() || isSymlinkTargetDir(f, fsrv.Root, urlPath) if isDir { name += "/" diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index fd994d0..29805a2 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -3,7 +3,6 @@ package fileserver import ( "net/http" "os" - "path/filepath" "bitbucket.org/lightcodelabs/caddy2" "bitbucket.org/lightcodelabs/caddy2/modules/caddyhttp" @@ -33,8 +32,7 @@ type FileMatcher struct { // Match matches the request r against m. func (m FileMatcher) Match(r *http.Request) bool { - // TODO: sanitize path - fullPath := filepath.Join(m.Root, m.Path) + fullPath := sanitizedPathJoin(m.Root, m.Path) var match bool if len(m.Flags) > 0 { match = true diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 873c5fb..19f5f68 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -30,9 +30,9 @@ type FileServer struct { Root string `json:"root"` // default is current directory Hide []string `json:"hide"` IndexNames []string `json:"index_names"` - Files []string `json:"files"` // all relative to the root; default is request URI path - Rehandle bool `json:"rehandle"` // issue a rehandle (internal redirect) if request is rewritten + Files []string `json:"files"` // all relative to the root; default is request URI path SelectionPolicy string `json:"selection_policy"` + Rehandle bool `json:"rehandle"` // issue a rehandle (internal redirect) if request is rewritten Fallback caddyhttp.RouteList `json:"fallback"` Browse *Browse `json:"browse"` // TODO: Etag -- cgit v1.2.3