From 5a4374bea055c49c9c38b6a7d41e43742c137341 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 6 Mar 2024 00:51:26 -0500 Subject: [PATCH 1/4] fileserver: Preserve query during canonicalization redirect (#6109) * fileserver: Preserve query during canonicalization redirect * Clarify that only a path should be passed --- modules/caddyhttp/fileserver/staticfiles.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 1f0b6a5e470..57d1bc85180 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -639,12 +639,18 @@ func calculateEtag(d os.FileInfo) string { return `"` + t + s + `"` } -func redirect(w http.ResponseWriter, r *http.Request, to string) error { - for strings.HasPrefix(to, "//") { +// redirect performs a redirect to a given path. The 'toPath' parameter +// MUST be solely a path, and MUST NOT include a query. +func redirect(w http.ResponseWriter, r *http.Request, toPath string) error { + for strings.HasPrefix(toPath, "//") { // prevent path-based open redirects - to = strings.TrimPrefix(to, "/") + toPath = strings.TrimPrefix(toPath, "/") } - http.Redirect(w, r, to, http.StatusPermanentRedirect) + // preserve the query string if present + if r.URL.RawQuery != "" { + toPath += "?" + r.URL.RawQuery + } + http.Redirect(w, r, toPath, http.StatusPermanentRedirect) return nil } From 277472d0817c04ba137bdb34f17d2bbbf2565403 Mon Sep 17 00:00:00 2001 From: huajin tong <137764712+thirdkeyword@users.noreply.github.com> Date: Wed, 6 Mar 2024 21:53:03 +0800 Subject: [PATCH 2/4] fix struct names (#6151) Signed-off-by: thirdkeyword --- modules/caddyhttp/celmatcher.go | 8 ++++---- modules/caddyhttp/reverseproxy/admin.go | 2 +- modules/caddyhttp/templates/templates.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/caddyhttp/celmatcher.go b/modules/caddyhttp/celmatcher.go index 65a0e7afa80..3761353643f 100644 --- a/modules/caddyhttp/celmatcher.go +++ b/modules/caddyhttp/celmatcher.go @@ -381,24 +381,24 @@ func CELMatcherImpl(macroName, funcName string, matcherDataTypes []*cel.Type, fa type CELMatcherFactory func(data ref.Val) (RequestMatcher, error) // matcherCELLibrary is a simplistic configurable cel.Library implementation. -type matcherCELLibary struct { +type matcherCELLibrary struct { envOptions []cel.EnvOption programOptions []cel.ProgramOption } // NewMatcherCELLibrary creates a matcherLibrary from option setes. func NewMatcherCELLibrary(envOptions []cel.EnvOption, programOptions []cel.ProgramOption) cel.Library { - return &matcherCELLibary{ + return &matcherCELLibrary{ envOptions: envOptions, programOptions: programOptions, } } -func (lib *matcherCELLibary) CompileOptions() []cel.EnvOption { +func (lib *matcherCELLibrary) CompileOptions() []cel.EnvOption { return lib.envOptions } -func (lib *matcherCELLibary) ProgramOptions() []cel.ProgramOption { +func (lib *matcherCELLibrary) ProgramOptions() []cel.ProgramOption { return lib.programOptions } diff --git a/modules/caddyhttp/reverseproxy/admin.go b/modules/caddyhttp/reverseproxy/admin.go index f64d1ecf0aa..7e72a4cdb51 100644 --- a/modules/caddyhttp/reverseproxy/admin.go +++ b/modules/caddyhttp/reverseproxy/admin.go @@ -32,7 +32,7 @@ func init() { // reverse proxy upstreams in the pool. type adminUpstreams struct{} -// upstreamResults holds the status of a particular upstream +// upstreamStatus holds the status of a particular upstream type upstreamStatus struct { Address string `json:"address"` NumRequests int `json:"num_requests"` diff --git a/modules/caddyhttp/templates/templates.go b/modules/caddyhttp/templates/templates.go index 418f09e531c..b97093b6f63 100644 --- a/modules/caddyhttp/templates/templates.go +++ b/modules/caddyhttp/templates/templates.go @@ -329,7 +329,7 @@ type Templates struct { logger *zap.Logger } -// Customfunctions is the interface for registering custom template functions. +// CustomFunctions is the interface for registering custom template functions. type CustomFunctions interface { // CustomTemplateFunctions should return the mapping from custom function names to implementations. CustomTemplateFunctions() template.FuncMap From 69290d232dde7306ddaa20172f03cb8860ea6adb Mon Sep 17 00:00:00 2001 From: Aziz Rmadi <46684200+armadi1809@users.noreply.github.com> Date: Wed, 6 Mar 2024 09:08:46 -0600 Subject: [PATCH 3/4] rewrite: Implement `uri query` operations (#6120) * Implemented basic uri query operations * Added support for query operations block * Applied Replacer on all query keys and values * Implemented rename query key opration * Rewrite struct: Changed QueryOperations field to Query and comments cleanup * Cleaned up comments, changed the order of operations and added more tests * Changed order of fields in queryOps struct to match the operations order --- caddytest/integration/caddyfile_test.go | 91 +++++++++++++++++++++++++ modules/caddyhttp/rewrite/caddyfile.go | 60 +++++++++++++++- modules/caddyhttp/rewrite/rewrite.go | 76 +++++++++++++++++++++ 3 files changed, 226 insertions(+), 1 deletion(-) diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go index 959783be7a7..5d1fa3f0898 100644 --- a/caddytest/integration/caddyfile_test.go +++ b/caddytest/integration/caddyfile_test.go @@ -497,6 +497,97 @@ func TestUriReplace(t *testing.T) { tester.AssertGetResponse("http://localhost:9080/endpoint?test={%20content%20}", 200, "test=%7B%20content%20%7D") } +func TestUriOps(t *testing.T) { + tester := caddytest.NewTester(t) + + tester.InitServer(` + { + admin localhost:2999 + http_port 9080 + } + :9080 + uri query +foo bar + uri query -baz + uri query taz test + uri query key=value example + uri query changethis>changed + + respond "{query}"`, "caddyfile") + + tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar0&baz=buz&taz=nottest&changethis=val", 200, "changed=val&foo=bar0&foo=bar&key%3Dvalue=example&taz=test") +} + +func TestSetThenAddQueryParams(t *testing.T) { + tester := caddytest.NewTester(t) + + tester.InitServer(` + { + admin localhost:2999 + http_port 9080 + } + :9080 + uri query foo bar + uri query +foo baz + + respond "{query}"`, "caddyfile") + + tester.AssertGetResponse("http://localhost:9080/endpoint", 200, "foo=bar&foo=baz") +} + +func TestSetThenDeleteParams(t *testing.T) { + tester := caddytest.NewTester(t) + + tester.InitServer(` + { + admin localhost:2999 + http_port 9080 + } + :9080 + uri query bar foo{query.foo} + uri query -foo + + respond "{query}"`, "caddyfile") + + tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "bar=foobar") +} + +func TestRenameAndOtherOps(t *testing.T) { + tester := caddytest.NewTester(t) + + tester.InitServer(` + { + admin localhost:2999 + http_port 9080 + } + :9080 + uri query foo>bar + uri query bar taz + uri query +bar baz + + respond "{query}"`, "caddyfile") + + tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "bar=taz&bar=baz") +} + +func TestUriOpsBlock(t *testing.T) { + tester := caddytest.NewTester(t) + + tester.InitServer(` + { + admin localhost:2999 + http_port 9080 + } + :9080 + uri query { + +foo bar + -baz + taz test + } + respond "{query}"`, "caddyfile") + + tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar0&baz=buz&taz=nottest", 200, "foo=bar0&foo=bar&taz=test") +} + func TestHandleErrorSimpleCodes(t *testing.T) { tester := caddytest.NewTester(t) tester.InitServer(`{ diff --git a/modules/caddyhttp/rewrite/caddyfile.go b/modules/caddyhttp/rewrite/caddyfile.go index 363dbfdb5ba..31f7e9b4859 100644 --- a/modules/caddyhttp/rewrite/caddyfile.go +++ b/modules/caddyhttp/rewrite/caddyfile.go @@ -98,7 +98,7 @@ func parseCaddyfileURI(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, err h.Next() // consume directive name args := h.RemainingArgs() - if len(args) < 2 { + if len(args) < 1 { return nil, h.ArgErr() } @@ -158,12 +158,70 @@ func parseCaddyfileURI(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, err Replace: replace, }) + case "query": + if len(args) > 4 { + return nil, h.ArgErr() + } + rewr.Query = &queryOps{} + var hasArgs bool + if len(args) > 1 { + hasArgs = true + err := applyQueryOps(h, rewr.Query, args[1:]) + if err != nil { + return nil, err + } + } + + for h.NextBlock(0) { + if hasArgs { + return nil, h.Err("Cannot specify uri query rewrites in both argument and block") + } + queryArgs := []string{h.Val()} + queryArgs = append(queryArgs, h.RemainingArgs()...) + err := applyQueryOps(h, rewr.Query, queryArgs) + if err != nil { + return nil, err + } + } + default: return nil, h.Errf("unrecognized URI manipulation '%s'", args[0]) } return rewr, nil } +func applyQueryOps(h httpcaddyfile.Helper, qo *queryOps, args []string) error { + key := args[0] + switch { + case strings.HasPrefix(key, "-"): + if len(args) != 1 { + return h.ArgErr() + } + qo.Delete = append(qo.Delete, strings.TrimLeft(key, "-")) + + case strings.HasPrefix(key, "+"): + if len(args) != 2 { + return h.ArgErr() + } + param := strings.TrimLeft(key, "+") + qo.Add = append(qo.Add, queryOpsArguments{Key: param, Val: args[1]}) + + case strings.Contains(key, ">"): + if len(args) != 1 { + return h.ArgErr() + } + renameValKey := strings.Split(key, ">") + qo.Rename = append(qo.Rename, queryOpsArguments{Key: renameValKey[0], Val: renameValKey[1]}) + + default: + if len(args) != 2 { + return h.ArgErr() + } + qo.Set = append(qo.Set, queryOpsArguments{Key: key, Val: args[1]}) + } + return nil +} + // parseCaddyfileHandlePath parses the handle_path directive. Syntax: // // handle_path [] { diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 77ef668bfa3..1859f9df2b5 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -89,6 +89,9 @@ type Rewrite struct { // Performs regular expression replacements on the URI path. PathRegexp []*regexReplacer `json:"path_regexp,omitempty"` + // Mutates the query string of the URI. + Query *queryOps `json:"query,omitempty"` + logger *zap.Logger } @@ -269,6 +272,11 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool { rep.do(r, repl) } + // apply query operations + if rewr.Query != nil { + rewr.Query.do(r, repl) + } + // update the encoded copy of the URI r.RequestURI = r.URL.RequestURI() @@ -470,5 +478,73 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) { } } +// queryOps describes the operations to perform on query keys: add, set, rename and delete. +type queryOps struct { + // Renames a query key from Key to Val, without affecting the value. + Rename []queryOpsArguments `json:"rename,omitempty"` + + // Sets query parameters; overwrites a query key with the given value. + Set []queryOpsArguments `json:"set,omitempty"` + + // Adds query parameters; does not overwrite an existing query field, + // and only appends an additional value for that key if any already exist. + Add []queryOpsArguments `json:"add,omitempty"` + + // Deletes a given query key by name. + Delete []string `json:"delete,omitempty"` +} + +func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) { + query := r.URL.Query() + + for _, renameParam := range q.Rename { + key := repl.ReplaceAll(renameParam.Key, "") + val := repl.ReplaceAll(renameParam.Val, "") + if key == "" || val == "" { + continue + } + query[val] = query[key] + delete(query, key) + } + + for _, setParam := range q.Set { + key := repl.ReplaceAll(setParam.Key, "") + if key == "" { + continue + } + val := repl.ReplaceAll(setParam.Val, "") + query[key] = []string{val} + } + + for _, addParam := range q.Add { + key := repl.ReplaceAll(addParam.Key, "") + if key == "" { + continue + } + val := repl.ReplaceAll(addParam.Val, "") + query[key] = append(query[key], val) + } + + for _, deleteParam := range q.Delete { + param := repl.ReplaceAll(deleteParam, "") + if param == "" { + continue + } + delete(query, param) + } + + r.URL.RawQuery = query.Encode() +} + +type queryOpsArguments struct { + // A key in the query string. Note that query string keys may appear multiple times. + Key string `json:"key,omitempty"` + + // The value for the given operation; for add and set, this is + // simply the value of the query, and for rename this is the + // query key to rename to. + Val string `json:"val,omitempty"` +} + // Interface guard var _ caddyhttp.MiddlewareHandler = (*Rewrite)(nil) From 258d9061401091c014c247d39d33d07bc178c66c Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 6 Mar 2024 14:41:45 -0500 Subject: [PATCH 4/4] httpcaddyfile: Add `RegisterDirectiveOrder` function for plugin authors (#5865) * httpcaddyfile: Add `RegisterDirectiveOrder` function for plugin authors * Set up Positional enum * Linter doesn't like a switch on an enum with default * Update caddyconfig/httpcaddyfile/directives.go Co-authored-by: Matt Holt --------- Co-authored-by: Matt Holt --- caddyconfig/httpcaddyfile/directives.go | 90 ++++++++++++++++++++++--- caddyconfig/httpcaddyfile/options.go | 14 ++-- 2 files changed, 89 insertions(+), 15 deletions(-) diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index 13026fa4edc..bde25031a84 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -27,18 +27,25 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) -// directiveOrder specifies the order -// to apply directives in HTTP routes. +// defaultDirectiveOrder specifies the default order +// to apply directives in HTTP routes. This must only +// consist of directives that are included in Caddy's +// standard distribution. // -// The root directive goes first in case rewrites or -// redirects depend on existence of files, i.e. the -// file matcher, which must know the root first. +// e.g. The 'root' directive goes near the start in +// case rewrites or redirects depend on existence of +// files, i.e. the file matcher, which must know the +// root first. // -// The header directive goes second so that headers -// can be manipulated before doing redirects. -var directiveOrder = []string{ +// e.g. The 'header' directive goes before 'redir' so +// that headers can be manipulated before doing redirects. +// +// e.g. The 'respond' directive is near the end because it +// writes a response and terminates the middleware chain. +var defaultDirectiveOrder = []string{ "tracing", + // set variables that may be used by other directives "map", "vars", "fs", @@ -85,6 +92,11 @@ var directiveOrder = []string{ "acme_server", } +// directiveOrder specifies the order to apply directives +// in HTTP routes, after being modified by either the +// plugins or by the user via the "order" global option. +var directiveOrder = defaultDirectiveOrder + // directiveIsOrdered returns true if dir is // a known, ordered (sorted) directive. func directiveIsOrdered(dir string) bool { @@ -131,6 +143,58 @@ func RegisterHandlerDirective(dir string, setupFunc UnmarshalHandlerFunc) { }) } +// RegisterDirectiveOrder registers the default order for a +// directive from a plugin. +// +// This is useful when a plugin has a well-understood place +// it should run in the middleware pipeline, and it allows +// users to avoid having to define the order themselves. +// +// The directive dir may be placed in the position relative +// to ('before' or 'after') a directive included in Caddy's +// standard distribution. It cannot be relative to another +// plugin's directive. +// +// EXPERIMENTAL: This API may change or be removed. +func RegisterDirectiveOrder(dir string, position Positional, standardDir string) { + // check if directive was already ordered + if directiveIsOrdered(dir) { + panic("directive '" + dir + "' already ordered") + } + + if position != Before && position != After { + panic("the 2nd argument must be either 'before' or 'after', got '" + position + "'") + } + + // check if directive exists in standard distribution, since + // we can't allow plugins to depend on one another; we can't + // guarantee the order that plugins are loaded in. + foundStandardDir := false + for _, d := range defaultDirectiveOrder { + if d == standardDir { + foundStandardDir = true + } + } + if !foundStandardDir { + panic("the 3rd argument '" + standardDir + "' must be a directive that exists in the standard distribution of Caddy") + } + + // insert directive into proper position + newOrder := directiveOrder + for i, d := range newOrder { + if d != standardDir { + continue + } + if position == Before { + newOrder = append(newOrder[:i], append([]string{dir}, newOrder[i:]...)...) + } else if position == After { + newOrder = append(newOrder[:i+1], append([]string{dir}, newOrder[i+1:]...)...) + } + break + } + directiveOrder = newOrder +} + // RegisterGlobalOption registers a unique global option opt with // an associated unmarshaling (setup) function. When the global // option opt is encountered in a Caddyfile, setupFunc will be @@ -555,6 +619,16 @@ func (sb serverBlock) isAllHTTP() bool { return true } +// Positional are the supported modes for ordering directives. +type Positional string + +const ( + Before Positional = "before" + After Positional = "after" + First Positional = "first" + Last Positional = "last" +) + type ( // UnmarshalFunc is a function which can unmarshal Caddyfile // tokens into zero or more config values using a Helper type. diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go index 9ff62d07e83..70d475d6b28 100644 --- a/caddyconfig/httpcaddyfile/options.go +++ b/caddyconfig/httpcaddyfile/options.go @@ -107,7 +107,7 @@ func parseOptOrder(d *caddyfile.Dispenser, _ any) (any, error) { if !d.Next() { return nil, d.ArgErr() } - pos := d.Val() + pos := Positional(d.Val()) newOrder := directiveOrder @@ -121,22 +121,22 @@ func parseOptOrder(d *caddyfile.Dispenser, _ any) (any, error) { // act on the positional switch pos { - case "first": + case First: newOrder = append([]string{dirName}, newOrder...) if d.NextArg() { return nil, d.ArgErr() } directiveOrder = newOrder return newOrder, nil - case "last": + case Last: newOrder = append(newOrder, dirName) if d.NextArg() { return nil, d.ArgErr() } directiveOrder = newOrder return newOrder, nil - case "before": - case "after": + case Before: + case After: default: return nil, d.Errf("unknown positional '%s'", pos) } @@ -153,9 +153,9 @@ func parseOptOrder(d *caddyfile.Dispenser, _ any) (any, error) { // insert directive into proper position for i, d := range newOrder { if d == otherDir { - if pos == "before" { + if pos == Before { newOrder = append(newOrder[:i], append([]string{dirName}, newOrder[i:]...)...) - } else if pos == "after" { + } else if pos == After { newOrder = append(newOrder[:i+1], append([]string{dirName}, newOrder[i+1:]...)...) } break