Merge pull request #1815 from wader/browse-abs-recursive-dir-symlink

browse: Support absolute and recursive directory symlinks

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.

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)
+			}
+		}()
+	}
+}