diff options
author | Toby Allen <[email protected]> | 2017-03-11 21:59:47 +0000 |
---|---|---|
committer | Matt Holt <[email protected]> | 2017-03-11 14:59:47 -0700 |
commit | cfe52084aacd11bde9d820bf38e41f26ce4aecca (patch) | |
tree | cd099a99a72ab415147bd023c07c6a84a59459e2 | |
parent | 6aa0e30af3102f63576beabce971ad1f1983c718 (diff) | |
download | caddy-cfe52084aacd11bde9d820bf38e41f26ce4aecca.tar.gz caddy-cfe52084aacd11bde9d820bf38e41f26ce4aecca.zip |
Fix issue #1346 {path} logging {uri} and add {rewrite_uri} placeholder (#1481)
* Fixed issue with {path} actually {uri}
* Test added for path rewrite
* add in uri_escaped
* added rewrite_uri and test
* fix broken test. Just checks for existance of rewrite header
* gitignore
* Use context to store uri value
* ignore .vscode
* tidy up, removal of comments and invalidated tests
* Remove commented out code.
* added comment as requested by lint
* fixed spelling mistake
* clarified code with variable name
* added context for uri and test
* added TODO comment to move consts
-rw-r--r-- | .gitignore | 4 | ||||
-rw-r--r-- | caddy.go | 6 | ||||
-rw-r--r-- | caddyhttp/fastcgi/fastcgi.go | 16 | ||||
-rw-r--r-- | caddyhttp/fastcgi/fastcgi_test.go | 21 | ||||
-rw-r--r-- | caddyhttp/httpserver/replacer.go | 30 | ||||
-rw-r--r-- | caddyhttp/httpserver/replacer_test.go | 37 | ||||
-rw-r--r-- | caddyhttp/rewrite/rewrite.go | 8 | ||||
-rw-r--r-- | caddyhttp/rewrite/to.go | 4 |
8 files changed, 78 insertions, 48 deletions
diff --git a/.gitignore b/.gitignore index e4e58fd0a..4f3845ed4 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,6 @@ access.log /*.conf Caddyfile -og_static/
\ No newline at end of file +og_static/ + +.vscode/
\ No newline at end of file @@ -871,9 +871,15 @@ var ( ) // CtxKey is a value for use with context.WithValue. +// TODO: Ideally CtxKey and consts will be moved to httpserver package. +// currently blocked by circular import with staticfiles. 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" + +// URIxRewriteCtxKey is a context key used to store original unrewritten +// URI in context.WithValue +const URIxRewriteCtxKey CtxKey = "caddy_rewrite_original_uri" diff --git a/caddyhttp/fastcgi/fastcgi.go b/caddyhttp/fastcgi/fastcgi.go index b4aeddd16..dfbd7db80 100644 --- a/caddyhttp/fastcgi/fastcgi.go +++ b/caddyhttp/fastcgi/fastcgi.go @@ -33,11 +33,6 @@ type Handler struct { ServerPort string } -// When a rewrite is performed, a header field of this name -// is added to the request -// It contains the original request URI before the rewrite. -const internalRewriteFieldName = "Caddy-Rewrite-Original-URI" - // ServeHTTP satisfies the httpserver.Handler interface. func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { for _, rule := range h.Rules { @@ -219,12 +214,12 @@ func (h Handler) buildEnv(r *http.Request, rule Rule, fpath string) (map[string] // Strip PATH_INFO from SCRIPT_NAME scriptName = strings.TrimSuffix(scriptName, pathInfo) - // Get the request URI. The request URI might be as it came in over the wire, + // Get the request URI from context. The request URI might be as it came in over the wire, // or it might have been rewritten internally by the rewrite middleware (see issue #256). - // If it was rewritten, there will be a header indicating the original URL, + // If it was rewritten, there will be a context value with the original URL, // which is needed to get the correct RequestURI value for PHP apps. reqURI := r.URL.RequestURI() - if origURI := r.Header.Get(internalRewriteFieldName); origURI != "" { + if origURI, _ := r.Context().Value(caddy.URIxRewriteCtxKey).(string); origURI != "" { reqURI = origURI } @@ -282,11 +277,8 @@ func (h Handler) buildEnv(r *http.Request, rule Rule, fpath string) (map[string] env[envVar[0]] = replacer.Replace(envVar[1]) } - // Add all HTTP headers (except Caddy-Rewrite-Original-URI ) to env variables + // Add all HTTP headers to env variables for field, val := range r.Header { - if strings.ToLower(field) == strings.ToLower(internalRewriteFieldName) { - continue - } header := strings.ToUpper(field) header = headerNameReplacer.Replace(header) env["HTTP_"+header] = strings.Join(val, ", ") diff --git a/caddyhttp/fastcgi/fastcgi_test.go b/caddyhttp/fastcgi/fastcgi_test.go index fdaf78bb7..85e451261 100644 --- a/caddyhttp/fastcgi/fastcgi_test.go +++ b/caddyhttp/fastcgi/fastcgi_test.go @@ -7,7 +7,6 @@ import ( "net/http/httptest" "net/url" "strconv" - "strings" "sync" "testing" "time" @@ -304,26 +303,6 @@ func TestBuildEnv(t *testing.T) { envExpected["CUSTOM_URI"] = "custom_uri/fgci_test.php?test=blabla" envExpected["CUSTOM_QUERY"] = "custom=true&test=blabla" testBuildEnv(r, rule, fpath, envExpected) - - // 6. Test Caddy-Rewrite-Original-URI header is not removed - r = newReq() - rule.EnvVars = [][2]string{ - {"HTTP_HOST", "{host}"}, - {"CUSTOM_URI", "custom_uri{uri}"}, - {"CUSTOM_QUERY", "custom=true&{query}"}, - } - envExpected = newEnv() - envExpected["HTTP_HOST"] = "localhost:2015" - envExpected["CUSTOM_URI"] = "custom_uri/fgci_test.php?test=blabla" - envExpected["CUSTOM_QUERY"] = "custom=true&test=blabla" - httpFieldName := strings.ToUpper(internalRewriteFieldName) - envExpected["HTTP_"+httpFieldName] = "" - r.Header.Add(internalRewriteFieldName, "/apath/torewrite/index.php") - testBuildEnv(r, rule, fpath, envExpected) - if r.Header.Get(internalRewriteFieldName) == "" { - t.Errorf("Error: Header Expected %v", internalRewriteFieldName) - } - } func TestReadTimeout(t *testing.T) { diff --git a/caddyhttp/httpserver/replacer.go b/caddyhttp/httpserver/replacer.go index 3d772b6f7..0816734dd 100644 --- a/caddyhttp/httpserver/replacer.go +++ b/caddyhttp/httpserver/replacer.go @@ -240,15 +240,23 @@ func (r *replacer) getSubstitution(key string) string { case "{path}": // if a rewrite has happened, the original URI should be used as the path // rather than the rewritten URI - path := r.request.Header.Get("Caddy-Rewrite-Original-URI") - if path == "" { + var path string + origpath, _ := r.request.Context().Value(caddy.URIxRewriteCtxKey).(string) + if origpath == "" { path = r.request.URL.Path + } else { + parsedURL, _ := url.Parse(origpath) + path = parsedURL.Path } return path case "{path_escaped}": - path := r.request.Header.Get("Caddy-Rewrite-Original-URI") - if path == "" { + var path string + origpath, _ := r.request.Context().Value(caddy.URIxRewriteCtxKey).(string) + if origpath == "" { path = r.request.URL.Path + } else { + parsedURL, _ := url.Parse(origpath) + path = parsedURL.Path } return url.QueryEscape(path) case "{rewrite_path}": @@ -276,8 +284,20 @@ func (r *replacer) getSubstitution(key string) string { } return port case "{uri}": - return r.request.URL.RequestURI() + uri, _ := r.request.Context().Value(caddy.URIxRewriteCtxKey).(string) + if uri == "" { + uri = r.request.URL.RequestURI() + } + return uri case "{uri_escaped}": + uri, _ := r.request.Context().Value(caddy.URIxRewriteCtxKey).(string) + if uri == "" { + uri = r.request.URL.RequestURI() + } + return url.QueryEscape(uri) + case "{rewrite_uri}": + return r.request.URL.RequestURI() + case "{rewrite_uri_escaped}": return url.QueryEscape(r.request.URL.RequestURI()) case "{when}": return now().Format(timeFormat) diff --git a/caddyhttp/httpserver/replacer_test.go b/caddyhttp/httpserver/replacer_test.go index 814fc01a6..70ab6d4f7 100644 --- a/caddyhttp/httpserver/replacer_test.go +++ b/caddyhttp/httpserver/replacer_test.go @@ -1,12 +1,15 @@ package httpserver import ( + "context" "net/http" "net/http/httptest" "os" "strings" "testing" "time" + + "github.com/mholt/caddy" ) func TestNewReplacer(t *testing.T) { @@ -149,6 +152,40 @@ func TestSet(t *testing.T) { } } +// Test function to test that various placeholders hold correct values after a rewrite +// has been performed. The NewRequest actually contains the rewritten value. +func TestPathRewrite(t *testing.T) { + w := httptest.NewRecorder() + recordRequest := NewResponseRecorder(w) + reader := strings.NewReader(`{"username": "dennis"}`) + + request, err := http.NewRequest("POST", "http://getcaddy.com/index.php?key=value", reader) + if err != nil { + t.Fatalf("Request Formation Failed: %s\n", err.Error()) + } + + ctx := context.WithValue(request.Context(), caddy.URIxRewriteCtxKey, "a/custom/path.php?key=value") + request = request.WithContext(ctx) + + repl := NewReplacer(request, recordRequest, "") + + if repl.Replace("This path is '{path}'") != "This path is 'a/custom/path.php'" { + t.Error("Expected host {path} replacement failed (" + repl.Replace("This path is '{path}'") + ")") + } + + if repl.Replace("This path is {rewrite_path}") != "This path is /index.php" { + t.Error("Expected host {rewrite_path} replacement failed (" + repl.Replace("This path is {rewrite_path}") + ")") + } + if repl.Replace("This path is '{uri}'") != "This path is 'a/custom/path.php?key=value'" { + t.Error("Expected host {uri} replacement failed (" + repl.Replace("This path is '{uri}'") + ")") + } + + if repl.Replace("This path is {rewrite_uri}") != "This path is /index.php?key=value" { + t.Error("Expected host {rewrite_uri} replacement failed (" + repl.Replace("This path is {rewrite_uri}") + ")") + } + +} + func TestRound(t *testing.T) { var tests = map[time.Duration]time.Duration{ // 599.935µs -> 560µs diff --git a/caddyhttp/rewrite/rewrite.go b/caddyhttp/rewrite/rewrite.go index 13fb8df53..5c8fd5ff7 100644 --- a/caddyhttp/rewrite/rewrite.go +++ b/caddyhttp/rewrite/rewrite.go @@ -65,9 +65,6 @@ func (s SimpleRule) Match(r *http.Request) bool { return s.From == r.URL.Path } // Rewrite rewrites the internal location of the current request. func (s SimpleRule) Rewrite(fs http.FileSystem, r *http.Request) Result { - // take note of this rewrite for internal use by fastcgi - // all we need is the URI, not full URL - r.Header.Set(headerFieldName, r.URL.RequestURI()) // attempt rewrite return To(fs, r, s.To, newReplacer(r)) @@ -234,8 +231,3 @@ func (r *ComplexRule) regexpMatches(rPath string) []string { func newReplacer(r *http.Request) httpserver.Replacer { return httpserver.NewReplacer(r, nil, "") } - -// When a rewrite is performed, this header is added to the request -// and is for internal use only, specifically the fastcgi middleware. -// It contains the original request URI before the rewrite. -const headerFieldName = "Caddy-Rewrite-Original-URI" diff --git a/caddyhttp/rewrite/to.go b/caddyhttp/rewrite/to.go index 660014d0f..66eea9b01 100644 --- a/caddyhttp/rewrite/to.go +++ b/caddyhttp/rewrite/to.go @@ -1,12 +1,14 @@ package rewrite import ( + "context" "log" "net/http" "net/url" "path" "strings" + "github.com/mholt/caddy" "github.com/mholt/caddy/caddyhttp/httpserver" ) @@ -49,7 +51,7 @@ func To(fs http.FileSystem, r *http.Request, to string, replacer httpserver.Repl // take note of this rewrite for internal use by fastcgi // all we need is the URI, not full URL - r.Header.Set(headerFieldName, r.URL.RequestURI()) + *r = *r.WithContext(context.WithValue(r.Context(), caddy.URIxRewriteCtxKey, r.URL.RequestURI())) // perform rewrite r.URL.Path = u.Path |