diff options
author | Matthew Holt <mholt@users.noreply.github.com> | 2022-07-13 12:20:00 -0600 |
---|---|---|
committer | Matthew Holt <mholt@users.noreply.github.com> | 2022-07-13 12:20:00 -0600 |
commit | 04a14ee37ac6192d734518fa9082d6eb93971bc6 (patch) | |
tree | 5493cc81de123ad1672e24b3e6f0b7f31d97515f | |
parent | c2bbe42fc3553f6a5685cdb45453c0950fa614b2 (diff) |
caddyhttp: Make query matcher more efficient
Only parse query string once
-rw-r--r-- | modules/caddyhttp/matchers.go | 18 |
1 files changed, 17 insertions, 1 deletions
diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 268b936..4c35802 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -103,6 +103,9 @@ type ( // "query": ["*"] // } // ``` + // + // Invalid query strings, including those with bad escapings or illegal characters + // like semicolons, will fail to parse and thus fail to match. MatchQuery url.Values // MatchHeader matches requests by header fields. The key is the field @@ -625,9 +628,22 @@ func (m *MatchQuery) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Match returns true if r matches m. An empty m matches an empty query string. func (m MatchQuery) Match(r *http.Request) bool { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + + // parse query string just once, for efficiency + parsed, err := url.ParseQuery(r.URL.RawQuery) + if err != nil { + // Illegal query string. Likely bad escape sequence or syntax. + // Note that semicolons in query string have a controversial history. Summaries: + // - https://github.com/golang/go/issues/50034 + // - https://github.com/golang/go/issues/25192 + // W3C recommendations are flawed and ambiguous, and different servers handle semicolons differently. + // Filippo Valsorda rightly wrote: "Relying on parser alignment for security is doomed." + return false + } + for param, vals := range m { param = repl.ReplaceAll(param, "") - paramVal, found := r.URL.Query()[param] + paramVal, found := parsed[param] if found { for _, v := range vals { v = repl.ReplaceAll(v, "") |