diff --git a/caddyhttp/browse/browse.go b/caddyhttp/browse/browse.go index dfd34336..e1368059 100644 --- a/caddyhttp/browse/browse.go +++ b/caddyhttp/browse/browse.go @@ -246,7 +246,9 @@ func directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, config } } - if f.IsDir() { + isDir := f.IsDir() || isSymlinkTargetDir(f, urlPath, config) + + if isDir { name += "/" dirCount++ } else { @@ -260,7 +262,7 @@ func directoryListing(files []os.FileInfo, canGoUp bool, urlPath string, config url := url.URL{Path: "./" + name} // prepend with "./" to fix paths with ':' in the name fileinfos = append(fileinfos, FileInfo{ - IsDir: f.IsDir() || isSymlinkTargetDir(f, urlPath, config), + IsDir: isDir, IsSymlink: isSymlink(f), Name: f.Name(), Size: f.Size(), @@ -291,19 +293,19 @@ func isSymlinkTargetDir(f os.FileInfo, urlPath string, config *Config) bool { if !isSymlink(f) { return false } - fullPath := func(fileName string) string { - fullPath := filepath.Join(string(config.Fs.Root.(http.Dir)), urlPath, fileName) - return filepath.Clean(fullPath) - } - target, err := os.Readlink(fullPath(f.Name())) + + // a bit strange but we want Stat thru the jailed filesystem to be safe + target, err := config.Fs.Root.Open(filepath.Join(urlPath, f.Name())) if err != nil { return false } - targetInfo, err := os.Lstat(fullPath(target)) + defer target.Close() + targetInto, err := target.Stat() if err != nil { return false } - return targetInfo.IsDir() + + return targetInto.IsDir() } // ServeHTTP determines if the request is for this plugin, and if all prerequisites are met. diff --git a/caddyhttp/browse/browse_test.go b/caddyhttp/browse/browse_test.go index 6952f35c..1e666021 100644 --- a/caddyhttp/browse/browse_test.go +++ b/caddyhttp/browse/browse_test.go @@ -3,12 +3,14 @@ package browse import ( "context" "encoding/json" + "io/ioutil" "net/http" "net/http/httptest" "net/url" "os" "path/filepath" "sort" + "strings" "testing" "text/template" "time" @@ -17,6 +19,8 @@ import ( "github.com/mholt/caddy/caddyhttp/staticfiles" ) +const testDirPrefix = "caddy_browse_test" + func TestSort(t *testing.T) { // making up []fileInfo with bogus values; // to be used to make up our "listing" @@ -453,3 +457,146 @@ func TestBrowseRedirect(t *testing.T) { } } } + +func TestDirSymlink(t *testing.T) { + testCases := []struct { + source string + target string + pathScope string + url string + expectedName string + expectedURL string + }{ + // test case can expect a directory "dir" and a symlink to it called "symlink" + + {"dir", "$TMP/rel_symlink_to_dir", "/", "/", + "rel_symlink_to_dir", "./rel_symlink_to_dir/"}, + {"$TMP/dir", "$TMP/abs_symlink_to_dir", "/", "/", + "abs_symlink_to_dir", "./abs_symlink_to_dir/"}, + + {"../../dir", "$TMP/sub/dir/rel_symlink_to_dir", "/", "/sub/dir/", + "rel_symlink_to_dir", "./rel_symlink_to_dir/"}, + {"$TMP/dir", "$TMP/sub/dir/abs_symlink_to_dir", "/", "/sub/dir/", + "abs_symlink_to_dir", "./abs_symlink_to_dir/"}, + + {"../../dir", "$TMP/with/scope/rel_symlink_to_dir", "/with/scope", "/with/scope/", + "rel_symlink_to_dir", "./rel_symlink_to_dir/"}, + {"$TMP/dir", "$TMP/with/scope/abs_symlink_to_dir", "/with/scope", "/with/scope/", + "abs_symlink_to_dir", "./abs_symlink_to_dir/"}, + + {"../../../../dir", "$TMP/with/scope/sub/dir/rel_symlink_to_dir", "/with/scope", "/with/scope/sub/dir/", + "rel_symlink_to_dir", "./rel_symlink_to_dir/"}, + {"$TMP/dir", "$TMP/with/scope/sub/dir/abs_symlink_to_dir", "/with/scope", "/with/scope/sub/dir/", + "abs_symlink_to_dir", "./abs_symlink_to_dir/"}, + + {"symlink", "$TMP/rel_symlink_to_symlink", "/", "/", + "rel_symlink_to_symlink", "./rel_symlink_to_symlink/"}, + {"$TMP/symlink", "$TMP/abs_symlink_to_symlink", "/", "/", + "abs_symlink_to_symlink", "./abs_symlink_to_symlink/"}, + + {"../../symlink", "$TMP/sub/dir/rel_symlink_to_symlink", "/", "/sub/dir/", + "rel_symlink_to_symlink", "./rel_symlink_to_symlink/"}, + {"$TMP/symlink", "$TMP/sub/dir/abs_symlink_to_symlink", "/", "/sub/dir/", + "abs_symlink_to_symlink", "./abs_symlink_to_symlink/"}, + + {"../../symlink", "$TMP/with/scope/rel_symlink_to_symlink", "/with/scope", "/with/scope/", + "rel_symlink_to_symlink", "./rel_symlink_to_symlink/"}, + {"$TMP/symlink", "$TMP/with/scope/abs_symlink_to_symlink", "/with/scope", "/with/scope/", + "abs_symlink_to_symlink", "./abs_symlink_to_symlink/"}, + + {"../../../../symlink", "$TMP/with/scope/sub/dir/rel_symlink_to_symlink", "/with/scope", "/with/scope/sub/dir/", + "rel_symlink_to_symlink", "./rel_symlink_to_symlink/"}, + {"$TMP/symlink", "$TMP/with/scope/sub/dir/abs_symlink_to_symlink", "/with/scope", "/with/scope/sub/dir/", + "abs_symlink_to_symlink", "./abs_symlink_to_symlink/"}, + } + + for i, tc := range testCases { + func() { + tmpdir, err := ioutil.TempDir("", testDirPrefix) + if err != nil { + t.Fatalf("failed to create test directory: %v", err) + } + defer os.RemoveAll(tmpdir) + + if err := os.MkdirAll(filepath.Join(tmpdir, "dir"), 0755); err != nil { + t.Fatalf("failed to create test dir 'dir': %v", err) + } + if err := os.Symlink("dir", filepath.Join(tmpdir, "symlink")); err != nil { + t.Fatalf("failed to create test symlink 'symlink': %v", err) + } + + sourceResolved := strings.Replace(tc.source, "$TMP", tmpdir, -1) + targetResolved := strings.Replace(tc.target, "$TMP", tmpdir, -1) + + if err := os.MkdirAll(filepath.Dir(sourceResolved), 0755); err != nil { + t.Fatalf("failed to create source symlink dir: %v", err) + } + if err := os.MkdirAll(filepath.Dir(targetResolved), 0755); err != nil { + t.Fatalf("failed to create target symlink dir: %v", err) + } + if err := os.Symlink(sourceResolved, targetResolved); err != nil { + t.Fatalf("failed to create test symlink: %v", err) + } + + 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: tc.pathScope, + Fs: staticfiles.FileServer{ + Root: http.Dir(tmpdir), + }, + }, + }, + } + + req, err := http.NewRequest("GET", tc.url, nil) + req.Header.Add("Accept", "application/json") + 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 != http.StatusOK { + t.Fatalf("Test %d - wrong return code, expected %d, got %d", + i, http.StatusOK, returnCode) + } + + type jsonEntry struct { + Name string + IsDir bool + IsSymlink bool + URL string + } + var entries []jsonEntry + if err := json.Unmarshal(rec.Body.Bytes(), &entries); err != nil { + t.Fatalf("Test %d - failed to parse json: %v", i, err) + } + + found := false + for _, e := range entries { + if e.Name != tc.expectedName { + continue + } + found = true + if !e.IsDir { + t.Fatalf("Test %d - expected to be a dir, got %v", i, e.IsDir) + } + if !e.IsSymlink { + t.Fatalf("Test %d - expected to be a symlink, got %v", i, e.IsSymlink) + } + if e.URL != tc.expectedURL { + t.Fatalf("Test %d - wrong URL, expected %v, got %v", i, tc.expectedURL, e.URL) + } + } + if !found { + t.Fatalf("Test %d - failed, could not find name %v", i, tc.expectedName) + } + }() + } +}