caddyhttp: Smarter path matching and rewriting (caddyserver#4948)

Co-authored-by: RussellLuo <luopeng.he@gmail.com>

modules/caddyhttp/caddyhttp.go

@@ -20,6 +20,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -244,6 +245,40 @@ func SanitizedPathJoin(root, reqPath string) string {
 	return path
 }
 
+// CleanPath cleans path p according to path.Clean(), but only
+// merges repeated slashes if collapseSlashes is true, and always
+// preserves trailing slashes.
+func CleanPath(p string, collapseSlashes bool) string {
+	if collapseSlashes {
+		return cleanPath(p)
+	}
+
+	// insert an invalid/impossible URI character into each two consecutive
+	// slashes to expand empty path segments; then clean the path as usual,
+	// and then remove the remaining temporary characters.
+	const tmpCh = 0xff
+	var sb strings.Builder
+	for i, ch := range p {
+		if ch == '/' && i > 0 && p[i-1] == '/' {
+			sb.WriteByte(tmpCh)
+		}
+		sb.WriteRune(ch)
+	}
+	halfCleaned := cleanPath(sb.String())
+	halfCleaned = strings.ReplaceAll(halfCleaned, string([]byte{tmpCh}), "")
+
+	return halfCleaned
+}
+
+// cleanPath does path.Clean(p) but preserves any trailing slash.
+func cleanPath(p string) string {
+	cleaned := path.Clean(p)
+	if cleaned != "/" && strings.HasSuffix(p, "/") {
+		cleaned = cleaned + "/"
+	}
+	return cleaned
+}
+
 // tlsPlaceholderWrapper is a no-op listener wrapper that marks
 // where the TLS listener should be in a chain of listener wrappers.
 // It should only be used if another listener wrapper must be placed

modules/caddyhttp/caddyhttp_test.go

@@ -92,3 +92,60 @@ func TestSanitizedPathJoin(t *testing.T) {
 		}
 	}
 }
+
+func TestCleanPath(t *testing.T) {
+	for i, tc := range []struct {
+		input        string
+		mergeSlashes bool
+		expect       string
+	}{
+		{
+			input:  "/foo",
+			expect: "/foo",
+		},
+		{
+			input:  "/foo/",
+			expect: "/foo/",
+		},
+		{
+			input:  "//foo",
+			expect: "//foo",
+		},
+		{
+			input:        "//foo",
+			mergeSlashes: true,
+			expect:       "/foo",
+		},
+		{
+			input:        "/foo//bar/",
+			mergeSlashes: true,
+			expect:       "/foo/bar/",
+		},
+		{
+			input:  "/foo/./.././bar",
+			expect: "/bar",
+		},
+		{
+			input:  "/foo//./..//./bar",
+			expect: "/foo//bar",
+		},
+		{
+			input:  "/foo///./..//./bar",
+			expect: "/foo///bar",
+		},
+		{
+			input:  "/foo///./..//.",
+			expect: "/foo//",
+		},
+		{
+			input:  "/foo//./bar",
+			expect: "/foo//bar",
+		},
+	} {
+		actual := CleanPath(tc.input, tc.mergeSlashes)
+		if actual != tc.expect {
+			t.Errorf("Test %d [input='%s' mergeSlashes=%t]: Got '%s', expected '%s'",
+				i, tc.input, tc.mergeSlashes, actual, tc.expect)
+		}
+	}
+}

modules/caddyhttp/fileserver/staticfiles.go

@@ -23,7 +23,6 @@ import (
 	weakrand "math/rand"
 	"mime"
 	"net/http"
-	"net/url"
 	"os"
 	"path"
 	"path/filepath"
@@ -236,16 +235,7 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
 	filesToHide := fsrv.transformHidePaths(repl)
 
 	root := repl.ReplaceAll(fsrv.Root, ".")
-	// PathUnescape returns an error if the escapes aren't well-formed,
-	// meaning the count % matches the RFC. Return early if the escape is
-	// improper.
-	if _, err := url.PathUnescape(r.URL.Path); err != nil {
-		fsrv.logger.Debug("improper path escape",
-			zap.String("site_root", root),
-			zap.String("request_path", r.URL.Path),
-			zap.Error(err))
-		return err
-	}
+
 	filename := caddyhttp.SanitizedPathJoin(root, r.URL.Path)
 
 	fsrv.logger.Debug("sanitized path join",