diff --git a/caddy.go b/caddy.go index 0df59fb7..2e769857 100644 --- a/caddy.go +++ b/caddy.go @@ -869,3 +869,11 @@ var ( // by default if no other file is specified. DefaultConfigFile = "Caddyfile" ) + +// CtxKey is a value for use with context.WithValue. +type CtxKey string + +// URLPathCtxKey is a context key. It can be used in HTTP handlers with +// context.WithValue to access the original request URI that accompanied the +// server request. The associated value will be of type string. +const URLPathCtxKey CtxKey = "url_path" diff --git a/caddyhttp/basicauth/basicauth.go b/caddyhttp/basicauth/basicauth.go index 57ab563c..a55ef17e 100644 --- a/caddyhttp/basicauth/basicauth.go +++ b/caddyhttp/basicauth/basicauth.go @@ -19,6 +19,7 @@ import ( "sync" "github.com/jimstudt/http-authentication/basic" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" ) @@ -65,7 +66,7 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error // let upstream middleware (e.g. fastcgi and cgi) know about authenticated // user; this replaces the request with a wrapped instance r = r.WithContext(context.WithValue(r.Context(), - httpserver.CtxKey("remote_user"), username)) + caddy.CtxKey("remote_user"), username)) } } diff --git a/caddyhttp/basicauth/basicauth_test.go b/caddyhttp/basicauth/basicauth_test.go index c90bf12b..64dd4592 100644 --- a/caddyhttp/basicauth/basicauth_test.go +++ b/caddyhttp/basicauth/basicauth_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "testing" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" ) @@ -18,7 +19,7 @@ func TestBasicAuth(t *testing.T) { // This handler is registered for tests in which the only authorized user is // "okuser" upstreamHandler := func(w http.ResponseWriter, r *http.Request) (int, error) { - remoteUser, _ := r.Context().Value(httpserver.CtxKey("remote_user")).(string) + remoteUser, _ := r.Context().Value(caddy.CtxKey("remote_user")).(string) if remoteUser != "okuser" { t.Errorf("Test %d: expecting remote user 'okuser', got '%s'", i, remoteUser) } diff --git a/caddyhttp/fastcgi/fastcgi.go b/caddyhttp/fastcgi/fastcgi.go index 4bc48cca..ed051efa 100644 --- a/caddyhttp/fastcgi/fastcgi.go +++ b/caddyhttp/fastcgi/fastcgi.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" ) @@ -222,7 +223,7 @@ func (h Handler) buildEnv(r *http.Request, rule Rule, fpath string) (map[string] // Retrieve name of remote user that was set by some downstream middleware, // possibly basicauth. - remoteUser, _ := r.Context().Value(httpserver.CtxKey("remote_user")).(string) // Blank if not set + remoteUser, _ := r.Context().Value(caddy.CtxKey("remote_user")).(string) // Blank if not set // Some variables are unused but cleared explicitly to prevent // the parent environment from interfering. diff --git a/caddyhttp/httpserver/context.go b/caddyhttp/httpserver/context.go index 549ced88..8a8cec93 100644 --- a/caddyhttp/httpserver/context.go +++ b/caddyhttp/httpserver/context.go @@ -14,6 +14,7 @@ import ( "os" + "github.com/mholt/caddy" "github.com/russross/blackfriday" ) @@ -325,10 +326,8 @@ func (c Context) Files(name string) ([]string, error) { // IsMITM returns true if it seems likely that the TLS connection // is being intercepted. func (c Context) IsMITM() bool { - if val, ok := c.Req.Context().Value(CtxKey("mitm")).(bool); ok { + if val, ok := c.Req.Context().Value(caddy.CtxKey("mitm")).(bool); ok { return val } return false } - -type CtxKey string diff --git a/caddyhttp/httpserver/mitm.go b/caddyhttp/httpserver/mitm.go index 58dd38ff..a4086509 100644 --- a/caddyhttp/httpserver/mitm.go +++ b/caddyhttp/httpserver/mitm.go @@ -9,6 +9,8 @@ import ( "net/http" "strings" "sync" + + "github.com/mholt/caddy" ) // tlsHandler is a http.Handler that will inject a value @@ -72,7 +74,7 @@ func (h *tlsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if checked { - r = r.WithContext(context.WithValue(r.Context(), CtxKey("mitm"), mitm)) + r = r.WithContext(context.WithValue(r.Context(), caddy.CtxKey("mitm"), mitm)) } if mitm && h.closeOnMITM { diff --git a/caddyhttp/httpserver/mitm_test.go b/caddyhttp/httpserver/mitm_test.go index 860ac982..a262a1af 100644 --- a/caddyhttp/httpserver/mitm_test.go +++ b/caddyhttp/httpserver/mitm_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "reflect" "testing" + + "github.com/mholt/caddy" ) func TestParseClientHello(t *testing.T) { @@ -285,7 +287,7 @@ func TestHeuristicFunctionsAndHandler(t *testing.T) { want := ch.interception handler := &tlsHandler{ next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - got, checked = r.Context().Value(CtxKey("mitm")).(bool) + got, checked = r.Context().Value(caddy.CtxKey("mitm")).(bool) }), listener: newTLSListener(nil, nil), } diff --git a/caddyhttp/httpserver/replacer.go b/caddyhttp/httpserver/replacer.go index 395aaacc..dcb5e177 100644 --- a/caddyhttp/httpserver/replacer.go +++ b/caddyhttp/httpserver/replacer.go @@ -13,6 +13,8 @@ import ( "strconv" "strings" "time" + + "github.com/mholt/caddy" ) // requestReplacer is a strings.Replacer which is used to @@ -299,7 +301,7 @@ func (r *replacer) getSubstitution(key string) string { } return requestReplacer.Replace(r.requestBody.String()) case "{mitm}": - if val, ok := r.request.Context().Value(CtxKey("mitm")).(bool); ok { + if val, ok := r.request.Context().Value(caddy.CtxKey("mitm")).(bool); ok { if val { return "likely" } else { diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go index c4067462..ec4f0af3 100644 --- a/caddyhttp/httpserver/server.go +++ b/caddyhttp/httpserver/server.go @@ -9,6 +9,7 @@ import ( "log" "net" "net/http" + "net/url" "os" "runtime" "strings" @@ -284,6 +285,8 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { }() w.Header().Set("Server", "Caddy") + c := context.WithValue(r.Context(), caddy.URLPathCtxKey, r.URL.Path) + r = r.WithContext(c) sanitizePath(r) @@ -340,6 +343,14 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error) } } + // URL fields other than Path and RawQuery will be empty for most server + // requests. Hence, the request URL is updated with the scheme and host + // from the virtual host's site address. + if vhostURL, err := url.Parse(vhost.Addr.String()); err == nil { + r.URL.Scheme = vhostURL.Scheme + r.URL.Host = vhostURL.Host + } + // Apply the path-based request body size limit // The error returned by MaxBytesReader is meant to be handled // by whichever middleware/plugin that receives it when calling @@ -398,10 +409,10 @@ func (s *Server) Stop() error { return nil } -// sanitizePath collapses any ./ ../ /// madness -// which helps prevent path traversal attacks. -// Note to middleware: use URL.RawPath If you need -// the "original" URL.Path value. +// sanitizePath collapses any ./ ../ /// madness which helps prevent +// path traversal attacks. Note to middleware: use the value within the +// request's context at key caddy.URLPathContextKey to access the +// "original" URL.Path value. func sanitizePath(r *http.Request) { if r.URL.Path == "/" { return diff --git a/caddyhttp/staticfiles/fileserver.go b/caddyhttp/staticfiles/fileserver.go index 57df52ea..c96cc37a 100644 --- a/caddyhttp/staticfiles/fileserver.go +++ b/caddyhttp/staticfiles/fileserver.go @@ -3,12 +3,14 @@ package staticfiles import ( "math/rand" "net/http" + "net/url" "os" - "path" "path/filepath" "runtime" "strconv" "strings" + + "github.com/mholt/caddy" ) // FileServer implements a production-ready file server @@ -90,17 +92,34 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request, name stri } // redirect to canonical path - url := r.URL.Path if d.IsDir() { - // Ensure / at end of directory url - if !strings.HasSuffix(url, "/") { - Redirect(w, r, path.Base(url)+"/", http.StatusMovedPermanently) + // Ensure / at end of directory url. If the original URL path is + // used then ensure / exists as well. + if !strings.HasSuffix(r.URL.Path, "/") { + toURL, _ := url.Parse(r.URL.String()) + + path, ok := r.Context().Value(caddy.URLPathCtxKey).(string) + if ok && !strings.HasSuffix(path, "/") { + toURL.Path = path + } + toURL.Path += "/" + + http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) return http.StatusMovedPermanently, nil } } else { - // Ensure no / at end of file url - if strings.HasSuffix(url, "/") { - Redirect(w, r, "../"+path.Base(url), http.StatusMovedPermanently) + // Ensure no / at end of file url. If the original URL path is + // used then ensure no / exists as well. + if strings.HasSuffix(r.URL.Path, "/") { + toURL, _ := url.Parse(r.URL.String()) + + path, ok := r.Context().Value(caddy.URLPathCtxKey).(string) + if ok && strings.HasSuffix(path, "/") { + toURL.Path = path + } + toURL.Path = strings.TrimSuffix(toURL.Path, "/") + + http.Redirect(w, r, toURL.String(), http.StatusMovedPermanently) return http.StatusMovedPermanently, nil } } diff --git a/caddyhttp/staticfiles/fileserver_test.go b/caddyhttp/staticfiles/fileserver_test.go index 1182dde2..058ad7cf 100644 --- a/caddyhttp/staticfiles/fileserver_test.go +++ b/caddyhttp/staticfiles/fileserver_test.go @@ -1,6 +1,7 @@ package staticfiles import ( + "context" "errors" "net/http" "net/http/httptest" @@ -10,6 +11,8 @@ import ( "strings" "testing" "time" + + "github.com/mholt/caddy" ) var ( @@ -20,16 +23,17 @@ var ( ) var ( - webrootFile1Html = filepath.Join("webroot", "file1.html") - webrootDirFile2Html = filepath.Join("webroot", "dir", "file2.html") - webrootDirHiddenHtml = filepath.Join("webroot", "dir", "hidden.html") - webrootDirwithindexIndeHtml = filepath.Join("webroot", "dirwithindex", "index.html") - webrootSubGzippedHtml = filepath.Join("webroot", "sub", "gzipped.html") - webrootSubGzippedHtmlGz = filepath.Join("webroot", "sub", "gzipped.html.gz") - webrootSubGzippedHtmlBr = filepath.Join("webroot", "sub", "gzipped.html.br") - webrootSubBrotliHtml = filepath.Join("webroot", "sub", "brotli.html") - webrootSubBrotliHtmlGz = filepath.Join("webroot", "sub", "brotli.html.gz") - webrootSubBrotliHtmlBr = filepath.Join("webroot", "sub", "brotli.html.br") + webrootFile1Html = filepath.Join("webroot", "file1.html") + webrootDirFile2Html = filepath.Join("webroot", "dir", "file2.html") + webrootDirHiddenHtml = filepath.Join("webroot", "dir", "hidden.html") + webrootDirwithindexIndeHtml = filepath.Join("webroot", "dirwithindex", "index.html") + webrootSubGzippedHtml = filepath.Join("webroot", "sub", "gzipped.html") + webrootSubGzippedHtmlGz = filepath.Join("webroot", "sub", "gzipped.html.gz") + webrootSubGzippedHtmlBr = filepath.Join("webroot", "sub", "gzipped.html.br") + webrootSubBrotliHtml = filepath.Join("webroot", "sub", "brotli.html") + webrootSubBrotliHtmlGz = filepath.Join("webroot", "sub", "brotli.html.gz") + webrootSubBrotliHtmlBr = filepath.Join("webroot", "sub", "brotli.html.br") + webrootSubBarDirWithIndexIndexHTML = filepath.Join("webroot", "bar", "dirwithindex", "index.html") ) // testFiles is a map with relative paths to test files as keys and file content as values. @@ -44,17 +48,18 @@ var ( // '------ file2.html // '------ hidden.html var testFiles = map[string]string{ - "unreachable.html": "

must not leak

", - webrootFile1Html: "

file1.html

", - webrootDirFile2Html: "

dir/file2.html

", - webrootDirwithindexIndeHtml: "

dirwithindex/index.html

", - webrootDirHiddenHtml: "

dir/hidden.html

", - webrootSubGzippedHtml: "

gzipped.html

", - webrootSubGzippedHtmlGz: "1.gzipped.html.gz", - webrootSubGzippedHtmlBr: "2.gzipped.html.br", - webrootSubBrotliHtml: "3.brotli.html", - webrootSubBrotliHtmlGz: "4.brotli.html.gz", - webrootSubBrotliHtmlBr: "5.brotli.html.br", + "unreachable.html": "

must not leak

", + webrootFile1Html: "

file1.html

", + webrootDirFile2Html: "

dir/file2.html

", + webrootDirwithindexIndeHtml: "

dirwithindex/index.html

", + webrootDirHiddenHtml: "

dir/hidden.html

", + webrootSubGzippedHtml: "

gzipped.html

", + webrootSubGzippedHtmlGz: "1.gzipped.html.gz", + webrootSubGzippedHtmlBr: "2.gzipped.html.br", + webrootSubBrotliHtml: "3.brotli.html", + webrootSubBrotliHtmlGz: "4.brotli.html.gz", + webrootSubBrotliHtmlBr: "5.brotli.html.br", + webrootSubBarDirWithIndexIndexHTML: "

bar/dirwithindex/index.html

", } // TestServeHTTP covers positive scenarios when serving files. @@ -71,9 +76,10 @@ func TestServeHTTP(t *testing.T) { movedPermanently := "Moved Permanently" tests := []struct { - url string - acceptEncoding string - + url string + cleanedPath string + acceptEncoding string + expectedLocation string expectedStatus int expectedBodyContent string expectedEtag string @@ -108,6 +114,7 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/dirwithindex", expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/dirwithindex/", expectedBodyContent: movedPermanently, }, // Test 5 - access folder without index file @@ -119,12 +126,14 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/dir", expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/dir/", expectedBodyContent: movedPermanently, }, // Test 7 - access file with trailing slash { url: "https://foo/file1.html/", expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/file1.html", expectedBodyContent: movedPermanently, }, // Test 8 - access not existing path @@ -148,6 +157,7 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/dir?param1=val", expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/dir/?param1=val", expectedBodyContent: movedPermanently, }, // Test 12 - attempt to bypass hidden file @@ -216,11 +226,39 @@ func TestServeHTTP(t *testing.T) { url: "https://foo/file1.html/other", expectedStatus: http.StatusNotFound, }, + // Test 20 - access folder with index file without trailing slash, with + // cleaned path + { + url: "https://foo/bar/dirwithindex", + cleanedPath: "/dirwithindex", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/bar/dirwithindex/", + expectedBodyContent: movedPermanently, + }, + // Test 21 - access folder with index file without trailing slash, with + // cleaned path and query params + { + url: "https://foo/bar/dirwithindex?param1=val", + cleanedPath: "/dirwithindex", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/bar/dirwithindex/?param1=val", + expectedBodyContent: movedPermanently, + }, + // Test 22 - access file with trailing slash with cleaned path + { + url: "https://foo/bar/file1.html/", + cleanedPath: "file1.html/", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/bar/file1.html", + expectedBodyContent: movedPermanently, + }, } for i, test := range tests { responseRecorder := httptest.NewRecorder() request, err := http.NewRequest("GET", test.url, nil) + ctx := context.WithValue(request.Context(), caddy.URLPathCtxKey, request.URL.Path) + request = request.WithContext(ctx) request.Header.Add("Accept-Encoding", test.acceptEncoding) @@ -231,6 +269,12 @@ func TestServeHTTP(t *testing.T) { if u, _ := url.Parse(test.url); u.RawPath != "" { request.URL.Path = u.RawPath } + // Caddy may trim a request's URL path. Overwrite the path with + // the cleanedPath to test redirects when the path has been + // modified. + if test.cleanedPath != "" { + request.URL.Path = test.cleanedPath + } status, err := fileserver.ServeHTTP(responseRecorder, request) etag := responseRecorder.Header().Get("Etag") body := responseRecorder.Body.String() @@ -266,6 +310,13 @@ func TestServeHTTP(t *testing.T) { if !strings.Contains(body, test.expectedBodyContent) { t.Errorf("Test %d: Expected body to contain %q, found %q", i, test.expectedBodyContent, body) } + + if test.expectedLocation != "" { + l := responseRecorder.Header().Get("Location") + if test.expectedLocation != l { + t.Errorf("Test %d: Expected Location header %q, found %q", i, test.expectedLocation, l) + } + } } }