From 04bee0f36d218edf0593b2c2254974f35d00276e Mon Sep 17 00:00:00 2001 From: Sawood Alam Date: Sat, 31 Dec 2016 22:29:14 -0500 Subject: [PATCH] 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 | 76 ++++++++++++++ caddyhttp/httpserver/pathcleaner_test.go | 120 +++++++++++++++++++++++ caddyhttp/httpserver/server.go | 3 +- 3 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 caddyhttp/httpserver/pathcleaner.go create mode 100644 caddyhttp/httpserver/pathcleaner_test.go diff --git a/caddyhttp/httpserver/pathcleaner.go b/caddyhttp/httpserver/pathcleaner.go new file mode 100644 index 000000000..cf5f1aa06 --- /dev/null +++ b/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 "/.." +// 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<= 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) +} diff --git a/caddyhttp/httpserver/pathcleaner_test.go b/caddyhttp/httpserver/pathcleaner_test.go new file mode 100644 index 000000000..34f395902 --- /dev/null +++ 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) + } + } +} diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go index 49956ab3f..7cc360402 100644 --- a/caddyhttp/httpserver/server.go +++ b/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 {