Repository: caddyserver/caddy (language: go)

Issue:
fileserver: Breadcrumbs are broken if directory name contains percent sign %
Breadcrumbs are generated without text if directory name contains a percent sign.
The bug is present in both

```
> caddy version
v2.6.1 h1:EDqo59TyYWhXQnfde93Mmv4FJfYe00dO60zMiEt+pzo=
```

and also the latest master 6bad878a22e048762262d6fabe2144cefaf4ca81

To replicate:  
Take this file tree 

```
> tree
.
└── dir with % in the name
    └── another
        └── with %
            └── sub
```

and run caddy with `caddy file-server --browse`

it creates this HTML:

```html
<header>
	<h1>
		<a href="../../../../">/</a><a href="../../../"></a>/<a href="../../">another</a>/<a href="../"></a>/<a href="">sub</a>/
	</h1>
</header>
```

Note the missing link text for directories with `%` signs in their name.

A test case to replicate the issue:

```golang
// put this in
// modules/caddyhttp/fileserver/browsetplcontext_test.go:21

func TestBreadcrumbs(t *testing.T) {
	testdata := []struct {
		path     string
		expected []crumb
	}{
		{"folder/with % sign/sub", []crumb{
			{Link: "../../", Text: "folder"},
			{Link: "../", Text: "with % sign"}, // .Text == "" here
			{Link: "", Text: "sub"},
		}},
```



Discussion / hints:
The problem seems to be caused by the use of `url.PathUnescape`. 

> It returns an error if any % is not followed by two hexadecimal digits

https://pkg.go.dev/net/url#PathUnescape

and it returns a non-nil err, but it is ignored.

https://github.com/caddyserver/caddy/blob/6bad878a22e048762262d6fabe2144cefaf4ca81/modules/caddyhttp/fileserver/browsetplcontext.go#L160
I don't have a solution in mind at the moment, but I can submit a fix for this issue when I have the time.
(I need to work on my force-push addiction.)
The same problem also exists here:

https://github.com/caddyserver/caddy/blob/6bad878a22e048762262d6fabe2144cefaf4ca81/modules/caddyhttp/fileserver/browsetplcontext.go#L95-L97

and it causes title to be rendered as `<title>.</title>`

Fix patch:
diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go
index e1a08942112..7cb6e407761 100644
--- a/modules/caddyhttp/fileserver/browse.go
+++ b/modules/caddyhttp/fileserver/browse.go
@@ -82,8 +82,8 @@ 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(r.Context(), dir.(fs.ReadDirFile), root, path.Clean(r.URL.Path), repl)
+	// TODO: not entirely sure if path.Clean() is necessary here but seems like a safe plan (i.e. /%2e%2e%2f) - someone could verify this
+	listing, err := fsrv.loadDirectoryContents(r.Context(), dir.(fs.ReadDirFile), root, path.Clean(r.URL.EscapedPath()), repl)
 	switch {
 	case os.IsPermission(err):
 		return caddyhttp.Error(http.StatusForbidden, err)
diff --git a/modules/caddyhttp/fileserver/browse.html b/modules/caddyhttp/fileserver/browse.html
index c893b640172..2afea5ee4c0 100644
--- a/modules/caddyhttp/fileserver/browse.html
+++ b/modules/caddyhttp/fileserver/browse.html
@@ -850,11 +850,11 @@ <h1>
 
 		<script>
 			const filterEl = document.getElementById('filter');
-			filterEl.focus({ preventScroll: true });
+			filterEl?.focus({ preventScroll: true });
 
 			function initPage() {
 				// populate and evaluate filter
-				if (!filterEl.value) {
+				if (!filterEl?.value) {
 					const filterParam = new URL(window.location.href).searchParams.get('filter');
 					if (filterParam) {
 						filterEl.value = filterParam;
@@ -874,6 +874,7 @@ <h1>
 			}
 
 			function filter() {
+				if (!filterEl) return;
 				const q = filterEl.value.trim().toLowerCase();
 				document.querySelectorAll('tr.file').forEach(function(el) {
 					if (!q) {


Test patch:
diff --git a/modules/caddyhttp/fileserver/browsetplcontext_test.go b/modules/caddyhttp/fileserver/browsetplcontext_test.go
index 9f0d08e1b2b..184196fa8c4 100644
--- a/modules/caddyhttp/fileserver/browsetplcontext_test.go
+++ b/modules/caddyhttp/fileserver/browsetplcontext_test.go
@@ -25,6 +25,45 @@ func TestBreadcrumbs(t *testing.T) {
 	}{
 		{"", []crumb{}},
 		{"/", []crumb{{Text: "/"}}},
+		{"/foo/", []crumb{
+			{Link: "../", Text: "/"},
+			{Link: "", Text: "foo"},
+		}},
+		{"/foo/bar/", []crumb{
+			{Link: "../../", Text: "/"},
+			{Link: "../", Text: "foo"},
+			{Link: "", Text: "bar"},
+		}},
+		{"/foo bar/", []crumb{
+			{Link: "../", Text: "/"},
+			{Link: "", Text: "foo bar"},
+		}},
+		{"/foo bar/baz/", []crumb{
+			{Link: "../../", Text: "/"},
+			{Link: "../", Text: "foo bar"},
+			{Link: "", Text: "baz"},
+		}},
+		{"/100%25 test coverage/is a lie/", []crumb{
+			{Link: "../../", Text: "/"},
+			{Link: "../", Text: "100% test coverage"},
+			{Link: "", Text: "is a lie"},
+		}},
+		{"/AC%2FDC/", []crumb{
+			{Link: "../", Text: "/"},
+			{Link: "", Text: "AC/DC"},
+		}},
+		{"/foo/%2e%2e%2f/bar", []crumb{
+			{Link: "../../../", Text: "/"},
+			{Link: "../../", Text: "foo"},
+			{Link: "../", Text: "../"},
+			{Link: "", Text: "bar"},
+		}},
+		{"/foo/../bar", []crumb{
+			{Link: "../../../", Text: "/"},
+			{Link: "../../", Text: "foo"},
+			{Link: "../", Text: ".."},
+			{Link: "", Text: "bar"},
+		}},
 		{"foo/bar/baz", []crumb{
 			{Link: "../../", Text: "foo"},
 			{Link: "../", Text: "bar"},
@@ -51,16 +90,16 @@ func TestBreadcrumbs(t *testing.T) {
 		}},
 	}
 
-	for _, d := range testdata {
+	for testNum, d := range testdata {
 		l := browseTemplateContext{Path: d.path}
 		actual := l.Breadcrumbs()
 		if len(actual) != len(d.expected) {
-			t.Errorf("wrong size output, got %d elements but expected %d", len(actual), len(d.expected))
+			t.Errorf("Test %d: Got %d components but expected %d; got: %+v", testNum, len(actual), len(d.expected), actual)
 			continue
 		}
 		for i, c := range actual {
 			if c != d.expected[i] {
-				t.Errorf("got %#v but expected %#v at index %d", c, d.expected[i], i)
+				t.Errorf("Test %d crumb %d: got %#v but expected %#v at index %d", testNum, i, c, d.expected[i], i)
 			}
 		}
 	}