Implementing custom PathClean function to allow masking, closes #1298 (#1317)

* Added path cleanup functions with masking to preserve certain patterns + unit tests, #1298

* Use custom PathClean function instead of path.Clean to apply masks to preserve protocol separator in the path

* Indentation corrected in the test data map to pass the lint

* Fixing ineffassign of a temporary string variable

* Improved variable naming and documentation

* Improved variable naming

* Added benchmarks and improved variable naming in tests

* Removed unnecessary value capture when iterating over a map for keys

* A typo correction

caddyhttp/httpserver/pathcleaner.go

@@ -0,0 +1,76 @@
+package httpserver
+
+import (
+	"math/rand"
+	"path"
+	"strings"
+	"time"
+)
+
+// CleanMaskedPath prevents one or more of the path cleanup operations:
+//   - collapse multiple slashes into one
+//   - eliminate "/." (current directory)
+//   - eliminate "<parent_directory>/.."
+// by masking certain patterns in the path with a temporary random string.
+// This could be helpful when certain patterns in the path are desired to be preserved
+// that would otherwise be changed by path.Clean().
+// One such use case is the presence of the double slashes as protocol separator
+// (e.g., /api/endpoint/http://example.com).
+// This is a common pattern in many applications to allow passing URIs as path argument.
+func CleanMaskedPath(reqPath string, masks ...string) string {
+	var replacerVal string
+	maskMap := make(map[string]string)
+
+	// Iterate over supplied masks and create temporary replacement strings
+	// only for the masks that are present in the path, then replace all occurrences
+	for _, mask := range masks {
+		if strings.Index(reqPath, mask) >= 0 {
+			replacerVal = "/_caddy" + generateRandomString() + "__"
+			maskMap[mask] = replacerVal
+			reqPath = strings.Replace(reqPath, mask, replacerVal, -1)
+		}
+	}
+
+	reqPath = path.Clean(reqPath)
+
+	// Revert the replaced masks after path cleanup
+	for mask, replacerVal := range maskMap {
+		reqPath = strings.Replace(reqPath, replacerVal, mask, -1)
+	}
+	return reqPath
+}
+
+// CleanPath calls CleanMaskedPath() with the default mask of "://"
+// to preserve double slashes of protocols
+// such as "http://", "https://", and "ftp://" etc.
+func CleanPath(reqPath string) string {
+	return CleanMaskedPath(reqPath, "://")
+}
+
+// An efficient and fast method for random string generation.
+// Inspired by http://stackoverflow.com/a/31832326.
+const randomStringLength = 4
+const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+const (
+	letterIdxBits = 6
+	letterIdxMask = 1<<letterIdxBits - 1
+	letterIdxMax  = 63 / letterIdxBits
+)
+
+var src = rand.NewSource(time.Now().UnixNano())
+
+func generateRandomString() string {
+	b := make([]byte, randomStringLength)
+	for i, cache, remain := randomStringLength-1, src.Int63(), letterIdxMax; i >= 0; {
+		if remain == 0 {
+			cache, remain = src.Int63(), letterIdxMax
+		}
+		if idx := int(cache & letterIdxMask); idx < len(letterBytes) {
+			b[i] = letterBytes[idx]
+			i--
+		}
+		cache >>= letterIdxBits
+		remain--
+	}
+	return string(b)
+}

caddyhttp/httpserver/pathcleaner_test.go

@@ -0,0 +1,120 @@
+package httpserver
+
+import (
+	"path"
+	"testing"
+)
+
+var paths = map[string]map[string]string{
+	"/../a/b/../././/c": {
+		"preserve_all":      "/../a/b/../././/c",
+		"preserve_protocol": "/a/c",
+		"preserve_slashes":  "/a//c",
+		"preserve_dots":     "/../a/b/../././c",
+		"clean_all":         "/a/c",
+	},
+	"/path/https://www.google.com": {
+		"preserve_all":      "/path/https://www.google.com",
+		"preserve_protocol": "/path/https://www.google.com",
+		"preserve_slashes":  "/path/https://www.google.com",
+		"preserve_dots":     "/path/https:/www.google.com",
+		"clean_all":         "/path/https:/www.google.com",
+	},
+	"/a/b/../././/c/http://example.com/foo//bar/../blah": {
+		"preserve_all":      "/a/b/../././/c/http://example.com/foo//bar/../blah",
+		"preserve_protocol": "/a/c/http://example.com/foo/blah",
+		"preserve_slashes":  "/a//c/http://example.com/foo/blah",
+		"preserve_dots":     "/a/b/../././c/http:/example.com/foo/bar/../blah",
+		"clean_all":         "/a/c/http:/example.com/foo/blah",
+	},
+}
+
+func assertEqual(t *testing.T, expected, received string) {
+	if expected != received {
+		t.Errorf("\tExpected: %s\n\t\t\tRecieved: %s", expected, received)
+	}
+}
+
+func maskedTestRunner(t *testing.T, variation string, masks ...string) {
+	for reqPath, transformation := range paths {
+		assertEqual(t, transformation[variation], CleanMaskedPath(reqPath, masks...))
+	}
+}
+
+// No need to test the built-in path.Clean() function.
+// However, it could be useful to cross-examine the test dataset.
+func TestPathClean(t *testing.T) {
+	for reqPath, transformation := range paths {
+		assertEqual(t, transformation["clean_all"], path.Clean(reqPath))
+	}
+}
+
+func TestCleanAll(t *testing.T) {
+	maskedTestRunner(t, "clean_all")
+}
+
+func TestPreserveAll(t *testing.T) {
+	maskedTestRunner(t, "preserve_all", "//", "/..", "/.")
+}
+
+func TestPreserveProtocol(t *testing.T) {
+	maskedTestRunner(t, "preserve_protocol", "://")
+}
+
+func TestPreserveSlashes(t *testing.T) {
+	maskedTestRunner(t, "preserve_slashes", "//")
+}
+
+func TestPreserveDots(t *testing.T) {
+	maskedTestRunner(t, "preserve_dots", "/..", "/.")
+}
+
+func TestDefaultMask(t *testing.T) {
+	for reqPath, transformation := range paths {
+		assertEqual(t, transformation["preserve_protocol"], CleanPath(reqPath))
+	}
+}
+
+func maskedBenchmarkRunner(b *testing.B, masks ...string) {
+	for n := 0; n < b.N; n++ {
+		for reqPath := range paths {
+			CleanMaskedPath(reqPath, masks...)
+		}
+	}
+}
+
+func BenchmarkPathClean(b *testing.B) {
+	for n := 0; n < b.N; n++ {
+		for reqPath := range paths {
+			path.Clean(reqPath)
+		}
+	}
+}
+
+func BenchmarkCleanAll(b *testing.B) {
+	maskedBenchmarkRunner(b)
+}
+
+func BenchmarkPreserveAll(b *testing.B) {
+	maskedBenchmarkRunner(b, "//", "/..", "/.")
+}
+
+func BenchmarkPreserveProtocol(b *testing.B) {
+	maskedBenchmarkRunner(b, "://")
+}
+
+func BenchmarkPreserveSlashes(b *testing.B) {
+	maskedBenchmarkRunner(b, "//")
+}
+
+func BenchmarkPreserveDots(b *testing.B) {
+	maskedBenchmarkRunner(b, "/..", "/.")
+}
+
+func BenchmarkDefaultMask(b *testing.B) {
+	for n := 0; n < b.N; n++ {
+		for reqPath := range paths {
+			CleanPath(reqPath)
+		}
+	}
+}

caddyhttp/httpserver/server.go

@@ -9,7 +9,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path"
 	"runtime"
 	"strings"
 	"sync"
@@ -351,7 +350,7 @@ func sanitizePath(r *http.Request) {
 	if r.URL.Path == "/" {
 		return
 	}
-	cleanedPath := path.Clean(r.URL.Path)
+	cleanedPath := CleanPath(r.URL.Path)
 	if cleanedPath == "." {
 		r.URL.Path = "/"
 	} else {