Repository: caddyserver/caddy (language: go)

Issue:
Stop `stat`ing files in dir when request context is cancelled
I am using Caddy to serve a dir using `file_server` middleware. The directory is hosted on a network drive. So, `stat`ing files aren't really cheap, because each stat requires performing a network IO, and it adds up when listing a directory with hundreds of files.

Caddy doesn't pass down the request context to directory listing

https://github.com/caddyserver/caddy/blob/b4e28af953aa02fb44f88c155956488b1f473348/modules/caddyhttp/fileserver/browse.go#L85

This means if I close the tab before the page loads (before caddy finishes `stat`ing all files in directory), caddy keeps on stating and it slows down the disk access for everything else.

https://github.com/caddyserver/caddy/blob/b4e28af953aa02fb44f88c155956488b1f473348/modules/caddyhttp/fileserver/browse.go#L139-L148

It should check if context is cancelled, and stop doing needless disk IO.




Discussion / hints:
I feel like passing request context down to this method and bailing out if it's cancelled should work:

```golang
func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
	// ...
	listing, err := fsrv.loadDirectoryContents(r.Context(), ...)
	// ...
}


func (fsrv *FileServer) directoryListing(ctx context.Context, entries []fs.DirEntry, canGoUp bool, root, urlPath string, repl *caddy.Replacer) browseTemplateContext {
	filesToHide := fsrv.transformHidePaths(repl)

	var dirCount, fileCount int
	fileInfos := []fileInfo{}

	for _, entry := range entries {
		if err := ctx.Err(); err != nil {
			break // or return err?
		}
```

but I'm not sure about how this affects the performance:

```golang
files, err := dir.ReadDir(10000) // TODO: this limit should probably be configurable 
```

How does walking the dir with `filepath.WalkDir()` compare to this?
Also, I can take over this issue and submit a PR with the solution I outlined above, unless you have something in mind.
@abdusco Yes, this sounds like a great idea. Would you like to submit a PR then? Your proposal above sounds like how I'd do it. (For some reason I didn't realize you could just check ctx.Err() to see if it was done, I always selected on ctx.Done() but checking the Err() is even easier!)

You could compare using WalkDir() if you want, I don't have a network drive setup handy to test with right now. I'd be curious which one is faster!

Fix patch:
diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go
index 6a72e155a6f..a8f5e8ab5f9 100644
--- a/modules/caddyhttp/fileserver/browse.go
+++ b/modules/caddyhttp/fileserver/browse.go
@@ -16,6 +16,7 @@ package fileserver
 
 import (
 	"bytes"
+	"context"
 	_ "embed"
 	"encoding/json"
 	"fmt"
@@ -82,7 +83,7 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter,
 	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 
 	// calling path.Clean here prevents weird breadcrumbs when URL paths are sketchy like /%2e%2e%2f
-	listing, err := fsrv.loadDirectoryContents(dir.(fs.ReadDirFile), root, path.Clean(r.URL.Path), repl)
+	listing, err := fsrv.loadDirectoryContents(r.Context(), dir.(fs.ReadDirFile), root, path.Clean(r.URL.Path), repl)
 	switch {
 	case os.IsPermission(err):
 		return caddyhttp.Error(http.StatusForbidden, err)
@@ -136,7 +137,7 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter,
 	return nil
 }
 
-func (fsrv *FileServer) loadDirectoryContents(dir fs.ReadDirFile, root, urlPath string, repl *caddy.Replacer) (browseTemplateContext, error) {
+func (fsrv *FileServer) loadDirectoryContents(ctx context.Context, dir fs.ReadDirFile, root, urlPath string, repl *caddy.Replacer) (browseTemplateContext, error) {
 	files, err := dir.ReadDir(10000) // TODO: this limit should probably be configurable
 	if err != nil && err != io.EOF {
 		return browseTemplateContext{}, err
@@ -145,7 +146,7 @@ func (fsrv *FileServer) loadDirectoryContents(dir fs.ReadDirFile, root, urlPath
 	// user can presumably browse "up" to parent folder if path is longer than "/"
 	canGoUp := len(urlPath) > 1
 
-	return fsrv.directoryListing(files, canGoUp, root, urlPath, repl), nil
+	return fsrv.directoryListing(ctx, files, canGoUp, root, urlPath, repl), nil
 }
 
 // browseApplyQueryParams applies query parameters to the listing.
diff --git a/modules/caddyhttp/fileserver/browsetplcontext.go b/modules/caddyhttp/fileserver/browsetplcontext.go
index cd24fc232b5..172fa505a56 100644
--- a/modules/caddyhttp/fileserver/browsetplcontext.go
+++ b/modules/caddyhttp/fileserver/browsetplcontext.go
@@ -15,6 +15,7 @@
 package fileserver
 
 import (
+	"context"
 	"io/fs"
 	"net/url"
 	"os"
@@ -30,13 +31,17 @@ import (
 	"go.uber.org/zap"
 )
 
-func (fsrv *FileServer) directoryListing(entries []fs.DirEntry, canGoUp bool, root, urlPath string, repl *caddy.Replacer) browseTemplateContext {
+func (fsrv *FileServer) directoryListing(ctx context.Context, entries []fs.DirEntry, canGoUp bool, root, urlPath string, repl *caddy.Replacer) browseTemplateContext {
 	filesToHide := fsrv.transformHidePaths(repl)
 
 	var dirCount, fileCount int
 	fileInfos := []fileInfo{}
 
 	for _, entry := range entries {
+		if err := ctx.Err(); err != nil {
+			break
+		}
+
 		name := entry.Name()
 
 		if fileHidden(name, filesToHide) {


Test patch: