fileserver: speed up directory browsing for large directories (#7903)

* improve fileserver performance and allocations
* skip instead of fatal
* fix comment
* implement more suggestions
* use filesystems.OsFS instead of os.DirFS
* fmt
* remove outdated benchmarks

modules/caddyhttp/fileserver/browse.go

@@ -333,20 +333,6 @@ func (fsrv *FileServer) makeBrowseTemplate(tplCtx *templateContext) (*template.T
 	return tpl, nil
 }
 
-// isSymlinkTargetDir returns true if f's symbolic link target
-// is a directory.
-func (fsrv *FileServer) isSymlinkTargetDir(fileSystem fs.FS, f fs.FileInfo, root, urlPath string) bool {
-	if !isSymlink(f) {
-		return false
-	}
-	target := caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, f.Name()))
-	targetInfo, err := fs.Stat(fileSystem, target)
-	if err != nil {
-		return false
-	}
-	return targetInfo.IsDir()
-}
-
 // isSymlink return true if f is a symbolic link.
 func isSymlink(f fs.FileInfo) bool {
 	return f.Mode()&os.ModeSymlink != 0

modules/caddyhttp/fileserver/browsetplcontext.go

@@ -44,6 +44,9 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS,
 		Path:         urlPath,
 		CanGoUp:      canGoUp,
 		lastModified: parentModTime,
+		// preallocate, since we know the upper bound on the number of
+		// items; avoids repeated slice growth/copy for large directories
+		Items: make([]fileInfo, 0, len(entries)),
 	}
 
 	for _, entry := range entries {
@@ -71,7 +74,23 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS,
 			tplCtx.lastModified = modTime
 		}
 
-		isDir := entry.IsDir() || fsrv.isSymlinkTargetDir(fileSystem, info, root, urlPath)
+		fileIsSymlink := isSymlink(info)
+		size := info.Size()
+
+		// for a symlink, a single stat of its target tells us both whether
+		// the target is a directory and what its size is
+		var targetInfo fs.FileInfo
+		var targetPath string
+		if fileIsSymlink {
+			targetPath = caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, info.Name()))
+			// An error most likely means the symlink target doesn't exist,
+			// which isn't entirely unusual and shouldn't fail the listing.
+			// In this case, targetInfo stays nil and we fall back to
+			// treating it as a non-directory, using the symlink's own size.
+			targetInfo, _ = fs.Stat(fileSystem, targetPath)
+		}
+
+		isDir := entry.IsDir() || (targetInfo != nil && targetInfo.IsDir())
 
 		// add the slash after the escape of path to avoid escaping the slash as well
 		if isDir {
@@ -81,34 +100,24 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS,
 			tplCtx.NumFiles++
 		}
 
-		size := info.Size()
-
 		if !isDir {
 			// increase the total by the symlink's size, not the target's size,
 			// by incrementing before we follow the symlink
 			tplCtx.TotalFileSize += size
 		}
 
-		fileIsSymlink := isSymlink(info)
 		symlinkPath := ""
 		if fileIsSymlink {
-			path := caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, info.Name()))
-			fileInfo, err := fs.Stat(fileSystem, path)
-			if err == nil {
-				size = fileInfo.Size()
+			if targetInfo != nil {
+				size = targetInfo.Size()
 			}
 
 			if fsrv.Browse.RevealSymlinks {
-				symLinkTarget, err := os.Readlink(path)
+				symLinkTarget, err := os.Readlink(targetPath)
 				if err == nil {
 					symlinkPath = symLinkTarget
 				}
 			}
-
-			// An error most likely means the symlink target doesn't exist,
-			// which isn't entirely unusual and shouldn't fail the listing.
-			// In this case, just use the size of the symlink itself, which
-			// was already set above.
 		}
 
 		if !isDir {
@@ -220,6 +229,14 @@ func (l *browseTemplateContext) applySortAndLimit(sortParam, orderParam, limitPa
 	l.Sort = sortParam
 	l.Order = orderParam
 
+	// Only compute lowercase names when the selected sort comparator needs it;
+	// avoid O(n) work when sorting by time or when no sorting is requested.
+	if l.Sort == sortByName || l.Sort == sortByNameDirFirst || l.Sort == sortBySize {
+		for i := range l.Items {
+			l.Items[i].nameLower = strings.ToLower(l.Items[i].Name)
+		}
+	}
+
 	if l.Order == "desc" {
 		switch l.Sort {
 		case sortByName:
@@ -282,6 +299,11 @@ type fileInfo struct {
 
 	// a pointer to the template context is useful inside nested templates
 	Tpl *browseTemplateContext `json:"-"`
+
+	// cached lowercase Name, precomputed once by applySortAndLimit so that
+	// byName/byNameDirFirst/bySize don't each recompute it on every one of
+	// the O(n log n) comparisons sort.Sort makes; not serialized.
+	nameLower string
 }
 
 // HasExt returns true if the filename has any of the given suffixes, case-insensitive.
@@ -328,7 +350,7 @@ func (l byName) Len() int      { return len(l.Items) }
 func (l byName) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] }
 
 func (l byName) Less(i, j int) bool {
-	return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name)
+	return l.Items[i].nameLower < l.Items[j].nameLower
 }
 
 func (l byNameDirFirst) Len() int      { return len(l.Items) }
@@ -337,7 +359,7 @@ func (l byNameDirFirst) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.
 func (l byNameDirFirst) Less(i, j int) bool {
 	// sort by name if both are dir or file
 	if l.Items[i].IsDir == l.Items[j].IsDir {
-		return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name)
+		return l.Items[i].nameLower < l.Items[j].nameLower
 	}
 	// sort dir ahead of file
 	return l.Items[i].IsDir
@@ -347,7 +369,7 @@ func (l bySize) Len() int      { return len(l.Items) }
 func (l bySize) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] }
 
 func (l bySize) Less(i, j int) bool {
-	const directoryOffset = -1 << 31 // = -math.MinInt32
+	const directoryOffset = -1 << 31 // = -2147483648 (min int32)
 
 	iSize, jSize := l.Items[i].Size, l.Items[j].Size
 
@@ -361,7 +383,7 @@ func (l bySize) Less(i, j int) bool {
 		jSize = directoryOffset
 	}
 	if l.Items[i].IsDir && l.Items[j].IsDir {
-		return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name)
+		return l.Items[i].nameLower < l.Items[j].nameLower
 	}
 
 	return iSize < jSize

modules/caddyhttp/fileserver/browsetplcontext_bench_test.go

@@ -0,0 +1,224 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fileserver
+
+import (
+	"context"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"testing"
+	"time"
+
+	"go.uber.org/zap"
+
+	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/internal/filesystems"
+)
+
+// makeBenchDir populates dir with n regular files and a handful of
+// symlinks (both to files and to a subdirectory), mirroring what a
+// real, large, browsable directory tends to contain.
+func makeBenchDir(tb testing.TB, dir string, n int) {
+	tb.Helper()
+
+	subdir := filepath.Join(dir, "subdir")
+	if err := os.Mkdir(subdir, 0o755); err != nil {
+		tb.Fatal(err)
+	}
+
+	for i := range n {
+		name := filepath.Join(dir, fmt.Sprintf("file-%d.txt", i))
+		if err := os.WriteFile(name, []byte("x"), 0o600); err != nil {
+			tb.Fatal(err)
+		}
+	}
+
+	target := filepath.Join(dir, "file-0.txt")
+	if err := os.Symlink(target, filepath.Join(dir, "symlink-to-file")); err != nil {
+		tb.Skipf("symlink not supported on this platform: %v", err)
+	}
+	if err := os.Symlink(subdir, filepath.Join(dir, "symlink-to-dir")); err != nil {
+		tb.Skipf("symlink not supported on this platform: %v", err)
+	}
+}
+
+// readBenchDirEntries reads all entries of dir once, up front, so that
+// benchmark iterations measure only the cost of directoryListing itself
+// (allocations, stat calls per entry) rather than the readdir syscall.
+//
+// It deliberately opens the directory and calls the ReadDir method on the
+// resulting file (like loadDirectoryContents does in production), not
+// fs.ReadDir/os.ReadDir - the latter sort entries by filename before
+// returning them, which would silently hand the sort benchmarks
+// already-sorted input and let sort.Sort's adaptive algorithm breeze
+// through with far fewer comparisons than a real, filesystem-ordered
+// directory listing requires.
+func readBenchDirEntries(tb testing.TB, dir string) (fs.FS, []fs.DirEntry) {
+	tb.Helper()
+
+	fileSystem := filesystems.OsFS{}
+	f, err := fileSystem.Open(dir)
+	if err != nil {
+		tb.Fatal(err)
+	}
+	defer f.Close()
+
+	entries, err := f.(fs.ReadDirFile).ReadDir(-1)
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return fileSystem, entries
+}
+
+func benchFileServer() *FileServer {
+	return &FileServer{
+		Browse: &Browse{},
+		logger: zap.NewNop(),
+	}
+}
+
+func benchmarkDirectoryListing(b *testing.B, n int, listFn func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext) {
+	dir := b.TempDir()
+	makeBenchDir(b, dir, n)
+	fileSystem, entries := readBenchDirEntries(b, dir)
+	fsrv := benchFileServer()
+	ctx := context.Background()
+
+	b.ReportAllocs()
+
+	for b.Loop() {
+		listFn(fsrv, ctx, fileSystem, entries, dir)
+	}
+}
+
+func BenchmarkDirectoryListing(b *testing.B) {
+	for _, n := range []int{100, 1_000, 10_000, 50_000} {
+		b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) {
+			benchmarkDirectoryListing(b, n, func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext {
+				return fsrv.directoryListing(ctx, fileSystem, time.Time{}, entries, true, root, "/", caddy.NewReplacer())
+			})
+		})
+	}
+}
+
+// makeBenchItems builds n synthetic fileInfo items (no disk I/O), mixing
+// upper/lower case names and marking roughly 1 in 10 as directories, to
+// exercise the sort comparators - including bySize's directory name
+// tie-break - the way a real, large, mixed listing would.
+func makeBenchItems(n int) []fileInfo {
+	items := make([]fileInfo, n)
+	for i := range n {
+		name := fmt.Sprintf("file_%d.txt", i)
+		if i%2 == 0 {
+			name = fmt.Sprintf("FILE_%d.TXT", i)
+		}
+		items[i] = fileInfo{
+			Name:  name,
+			Size:  int64(i),
+			IsDir: i%10 == 0,
+		}
+	}
+	return items
+}
+
+func fillNameLower(items []fileInfo) {
+	for i := range items {
+		items[i].nameLower = strings.ToLower(items[i].Name)
+	}
+}
+
+// benchmarkSort times sortFn against a fresh, unsorted copy of a synthetic
+// item set on every iteration. The copy, and the optional prepare step
+// (e.g. filling the nameLower cache), happen outside the timed portion so
+// the benchmark isolates the cost of the sort itself.
+func benchmarkSort(b *testing.B, n int, prepare func(items []fileInfo), sortFn func(items []fileInfo)) {
+	pristine := makeBenchItems(n)
+	items := make([]fileInfo, n)
+
+	for b.Loop() {
+		b.StopTimer()
+		copy(items, pristine)
+		if prepare != nil {
+			prepare(items)
+		}
+		b.StartTimer()
+		sortFn(items)
+	}
+}
+
+func BenchmarkSortByName(b *testing.B) {
+	for _, n := range []int{100, 1_000, 10_000, 50_000} {
+		b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) {
+			benchmarkSort(b, n, fillNameLower, func(items []fileInfo) {
+				sort.Sort(byName(browseTemplateContext{Items: items}))
+			})
+		})
+	}
+}
+
+func BenchmarkSortByNameDirFirst(b *testing.B) {
+	for _, n := range []int{100, 1_000, 10_000, 50_000} {
+		b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) {
+			benchmarkSort(b, n, fillNameLower, func(items []fileInfo) {
+				sort.Sort(byNameDirFirst(browseTemplateContext{Items: items}))
+			})
+		})
+	}
+}
+
+func BenchmarkSortBySize(b *testing.B) {
+	for _, n := range []int{100, 1_000, 10_000, 50_000} {
+		b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) {
+			benchmarkSort(b, n, fillNameLower, func(items []fileInfo) {
+				sort.Sort(bySize(browseTemplateContext{Items: items}))
+			})
+		})
+	}
+}
+
+// ---------------------------------------------------------------------
+// Overall benchmark: the whole real-world browse-request path - reading
+// directory entries into a listing and then sorting it.
+// ---------------------------------------------------------------------
+
+func benchmarkDirectoryListingAndSort(b *testing.B, n int, run func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext) {
+	dir := b.TempDir()
+	makeBenchDir(b, dir, n)
+	fileSystem, entries := readBenchDirEntries(b, dir)
+	fsrv := benchFileServer()
+	ctx := context.Background()
+
+	b.ReportAllocs()
+
+	for b.Loop() {
+		run(fsrv, ctx, fileSystem, entries, dir)
+	}
+}
+
+func BenchmarkDirectoryListingAndSort(b *testing.B) {
+	for _, n := range []int{100, 1_000, 10_000, 50_000} {
+		b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) {
+			benchmarkDirectoryListingAndSort(b, n, func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext {
+				listing := fsrv.directoryListing(ctx, fileSystem, time.Time{}, entries, true, root, "/", caddy.NewReplacer())
+				listing.applySortAndLimit(sortByNameDirFirst, sortOrderAsc, "", "")
+				return listing
+			})
+		})
+	}
+}

modules/caddyhttp/reverseproxy/selectionpolicies_test.go

@@ -133,36 +133,36 @@ func TestWeightedRoundRobinPolicy(t *testing.T) {
 
 func TestWeightedRoundRobinSelection_Validate(t *testing.T) {
 	tests := []struct {
-		name        string
-		weights     []int
-		wantErr     bool
+		name    string
+		weights []int
+		wantErr bool
 	}{
 		{
-			name:        "Valid 0 2 1 case",
-			weights:     []int{0, 2, 1},
-			wantErr:     false,
+			name:    "Valid 0 2 1 case",
+			weights: []int{0, 2, 1},
+			wantErr: false,
 		},
 		{
-			name:        "Invalid 0 case (single)",
-			weights:     []int{0},
-			wantErr:     true,
+			name:    "Invalid 0 case (single)",
+			weights: []int{0},
+			wantErr: true,
 		},
 		{
-			name:        "Invalid 0 0 case (multiple)",
-			weights:     []int{0, 0},
-			wantErr:     true,
+			name:    "Invalid 0 0 case (multiple)",
+			weights: []int{0, 0},
+			wantErr: true,
 		},
 		{
-			name:        "Valid weights",
-			weights:     []int{1, 1, 1},
-			wantErr:     false,
+			name:    "Valid weights",
+			weights: []int{1, 1, 1},
+			wantErr: false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			s := &WeightedRoundRobinSelection{
-				Weights:     tt.weights,
+				Weights: tt.weights,
 			}
 			_ = s.Provision(caddy.Context{})
 			err := s.Validate()

modules/caddyhttp/templates/tplcontext_test.go

@@ -424,9 +424,9 @@ func TestStripHTML(t *testing.T) {
 			expect: ``,
 		},
 		{
-		    // false start — second '<' increments depth, single '>' only closes one level
-		    input:  `<h1<b>hi`,
-		    expect: ``,
+			// false start — second '<' increments depth, single '>' only closes one level
+			input:  `<h1<b>hi`,
+			expect: ``,
 		},
 		{
 			// XSS bypass via double opening bracket