From 25dea2903e613534e00a2a4e305e0c8f2daeea8d Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 11 Jan 2020 13:47:42 -0700 Subject: http: A little more polish on rewrite handler and try_files directive --- modules/caddyhttp/fileserver/caddyfile.go | 28 ++++++------ .../caddyhttp/reverseproxy/fastcgi/caddyfile.go | 2 +- modules/caddyhttp/rewrite/caddyfile.go | 7 +-- modules/caddyhttp/rewrite/rewrite.go | 50 +++++++++++----------- modules/caddyhttp/rewrite/rewrite_test.go | 12 +++--- 5 files changed, 51 insertions(+), 48 deletions(-) (limited to 'modules') diff --git a/modules/caddyhttp/fileserver/caddyfile.go b/modules/caddyhttp/fileserver/caddyfile.go index aa11ba0..d26b435 100644 --- a/modules/caddyhttp/fileserver/caddyfile.go +++ b/modules/caddyhttp/fileserver/caddyfile.go @@ -107,11 +107,17 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // try_files // } // } -// rewrite @try_files {http.matchers.file.relative}{http.request.uri.query_string} +// rewrite @try_files {http.matchers.file.relative} // -// If any of the files in the list have a query string, the query string will -// be ignored when checking for file existence, but will be augmented into -// the request's URI when rewriting the request. +// This directive rewrites request paths only, preserving any other part +// of the URI, unless the part is explicitly given in the file list. For +// example, if any of the files in the list have a query string: +// +// try_files {path} index.php?{query}&p={path} +// +// then the query string will not be treated as part of the file name; and +// if that file matches, the given query string will replace any query string +// that already exists on the request URI. func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) { if !h.Next() { return nil, h.ArgErr() @@ -123,16 +129,14 @@ func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } // makeRoute returns a route that tries the files listed in try - // and then rewrites to the matched file, and appends writeURIAppend - // to the end of the query string. - makeRoute := func(try []string, writeURIAppend string) []httpcaddyfile.ConfigValue { + // and then rewrites to the matched file; userQueryString is + // appended to the rewrite rule. + makeRoute := func(try []string, userQueryString string) []httpcaddyfile.ConfigValue { handler := rewrite.Rewrite{ - URI: "{http.matchers.file.relative}{http.request.uri.query_string}" + writeURIAppend, + URI: "{http.matchers.file.relative}" + userQueryString, } matcherSet := caddy.ModuleMap{ - "file": h.JSON(MatchFile{ - TryFiles: try, - }), + "file": h.JSON(MatchFile{TryFiles: try}), } return h.NewRoute(matcherSet, handler) } @@ -149,7 +153,7 @@ func parseTryFiles(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) result = append(result, makeRoute(try, "")...) try = []string{} } - result = append(result, makeRoute([]string{item[:idx]}, "&"+item[idx+1:])...) + result = append(result, makeRoute([]string{item[:idx]}, item[idx:])...) continue } // accumulate consecutive non-query-string parameters diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index ca0d6cc..dee6eb5 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -149,7 +149,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error }), } rewriteHandler := rewrite.Rewrite{ - URI: "{http.matchers.file.relative}?{http.request.uri.query}", + URI: "{http.matchers.file.relative}", } rewriteRoute := caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{rewriteMatcherSet}, diff --git a/modules/caddyhttp/rewrite/caddyfile.go b/modules/caddyhttp/rewrite/caddyfile.go index 368db15..6ec51c0 100644 --- a/modules/caddyhttp/rewrite/caddyfile.go +++ b/modules/caddyhttp/rewrite/caddyfile.go @@ -32,7 +32,8 @@ func init() { // // rewrite [] // -// The parameter becomes the new URI. +// Only URI components which are given in will be set in the resulting URI. +// See the docs for the rewrite handler for more information. func parseCaddyfileRewrite(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { var rewr Rewrite for h.Next() { @@ -58,7 +59,7 @@ func parseCaddyfileStripPrefix(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHand if !h.NextArg() { return nil, h.ArgErr() } - rewr.StripPrefix = h.Val() + rewr.StripPathPrefix = h.Val() if h.NextArg() { return nil, h.ArgErr() } @@ -77,7 +78,7 @@ func parseCaddyfileStripSuffix(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHand if !h.NextArg() { return nil, h.ArgErr() } - rewr.StripSuffix = h.Val() + rewr.StripPathSuffix = h.Val() if h.NextArg() { return nil, h.ArgErr() } diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index c069db9..abcaa84 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -17,7 +17,6 @@ package rewrite import ( "net/http" "net/url" - "strconv" "strings" "github.com/caddyserver/caddy/v2" @@ -31,32 +30,40 @@ func init() { // Rewrite is a middleware which can rewrite HTTP requests. // -// These rewrite properties are applied to a request in this order: -// Method, URI, StripPrefix, StripSuffix, URISubstring. -// -// TODO: This module is still a WIP and may experience breaking changes. +// The Method and URI properties are "setters": the request URI +// will be set to the given values. Other properties are "modifiers": +// they modify existing files but do not explicitly specify what the +// result will be. It is atypical to combine the use of setters and +// modifiers in a single rewrite. type Rewrite struct { // Changes the request's HTTP verb. Method string `json:"method,omitempty"` - // Changes the request's URI (path, query string, and fragment if present). + // Changes the request's URI, which consists of path and query string. // Only components of the URI that are specified will be changed. + // For example, a value of "/foo.html" or "foo.html" will only change + // the path and will preserve any existing query string. Similarly, a + // value of "?a=b" will only change the query string and will not affect + // the path. Both can also be changed: "/foo?a=b" - this sets both the + // path and query string at the same time. + // + // You can also use placeholders. For example, to preserve the existing + // query string, you might use: "?{http.request.uri.query}&a=b". Any + // key-value pairs you add to the query string will not overwrite + // existing values. + // + // To clear the query string, explicitly set an empty one: "?" URI string `json:"uri,omitempty"` // Strips the given prefix from the beginning of the URI path. - StripPrefix string `json:"strip_prefix,omitempty"` + StripPathPrefix string `json:"strip_path_prefix,omitempty"` // Strips the given suffix from the end of the URI path. - StripSuffix string `json:"strip_suffix,omitempty"` + StripPathSuffix string `json:"strip_path_suffix,omitempty"` // Performs substring replacements on the URI. URISubstring []replacer `json:"uri_substring,omitempty"` - // If set to a 3xx HTTP status code and if the URI was rewritten (changed), - // the handler will issue a simple HTTP redirect to the new URI using the - // given status code. - HTTPRedirect caddyhttp.WeakString `json:"http_redirect,omitempty"` - logger *zap.Logger } @@ -88,15 +95,6 @@ func (rewr Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddy zap.String("method", r.Method), zap.String("uri", r.RequestURI), ) - if rewr.HTTPRedirect != "" { - statusCode, err := strconv.Atoi(repl.ReplaceAll(rewr.HTTPRedirect.String(), "")) - if err != nil { - return caddyhttp.Error(http.StatusInternalServerError, err) - } - w.Header().Set("Location", r.RequestURI) - w.WriteHeader(statusCode) - return nil - } } return next.ServeHTTP(w, r) @@ -148,12 +146,12 @@ func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.L } // strip path prefix or suffix - if rewr.StripPrefix != "" { - prefix := repl.ReplaceAll(rewr.StripPrefix, "") + if rewr.StripPathPrefix != "" { + prefix := repl.ReplaceAll(rewr.StripPathPrefix, "") r.URL.Path = strings.TrimPrefix(r.URL.Path, prefix) } - if rewr.StripSuffix != "" { - suffix := repl.ReplaceAll(rewr.StripSuffix, "") + if rewr.StripPathSuffix != "" { + suffix := repl.ReplaceAll(rewr.StripPathSuffix, "") r.URL.Path = strings.TrimSuffix(r.URL.Path, suffix) } diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index de82d8d..3dbc2d6 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -150,33 +150,33 @@ func TestRewrite(t *testing.T) { }, { - rule: Rewrite{StripPrefix: "/prefix"}, + rule: Rewrite{StripPathPrefix: "/prefix"}, input: newRequest(t, "GET", "/foo/bar"), expect: newRequest(t, "GET", "/foo/bar"), }, { - rule: Rewrite{StripPrefix: "/prefix"}, + rule: Rewrite{StripPathPrefix: "/prefix"}, input: newRequest(t, "GET", "/prefix/foo/bar"), expect: newRequest(t, "GET", "/foo/bar"), }, { - rule: Rewrite{StripPrefix: "/prefix"}, + rule: Rewrite{StripPathPrefix: "/prefix"}, input: newRequest(t, "GET", "/foo/prefix/bar"), expect: newRequest(t, "GET", "/foo/prefix/bar"), }, { - rule: Rewrite{StripSuffix: "/suffix"}, + rule: Rewrite{StripPathSuffix: "/suffix"}, input: newRequest(t, "GET", "/foo/bar"), expect: newRequest(t, "GET", "/foo/bar"), }, { - rule: Rewrite{StripSuffix: "suffix"}, + rule: Rewrite{StripPathSuffix: "suffix"}, input: newRequest(t, "GET", "/foo/bar/suffix"), expect: newRequest(t, "GET", "/foo/bar/"), }, { - rule: Rewrite{StripSuffix: "/suffix"}, + rule: Rewrite{StripPathSuffix: "/suffix"}, input: newRequest(t, "GET", "/foo/suffix/bar"), expect: newRequest(t, "GET", "/foo/suffix/bar"), }, -- cgit v1.2.3