caddyhttp: Make query matcher more efficient

Only parse query string once

Matthew Holt committed Jul 13, 2022 at 12:20 UTC 04a14ee37ac6192d734518fa9082d6eb93971bc6

1 file changed +17 -1

modules/caddyhttp/matchers.go

+17 -1

@@ -103,6 +103,9 @@ type (

103 // "query": ["*"]

104 // }

105 // ```

106 + //

107 + // Invalid query strings, including those with bad escapings or illegal characters

108 + // like semicolons, will fail to parse and thus fail to match.

109 MatchQuery url.Values

110

111 // MatchHeader matches requests by header fields. The key is the field

@@ -625,9 +628,22 @@ func (m *MatchQuery) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

628 // Match returns true if r matches m. An empty m matches an empty query string.

629 func (m MatchQuery) Match(r *http.Request) bool {

630 repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)

631 +

632 + // parse query string just once, for efficiency

633 + parsed, err := url.ParseQuery(r.URL.RawQuery)

634 + if err != nil {

635 + // Illegal query string. Likely bad escape sequence or syntax.

636 + // Note that semicolons in query string have a controversial history. Summaries:

637 + // - https://github.com/golang/go/issues/50034

638 + // - https://github.com/golang/go/issues/25192

639 + // W3C recommendations are flawed and ambiguous, and different servers handle semicolons differently.

640 + // Filippo Valsorda rightly wrote: "Relying on parser alignment for security is doomed."

641 + return false

642 + }

643 +

644 for param, vals := range m {

645 param = repl.ReplaceAll(param, "")

630 - paramVal, found := r.URL.Query()[param]

646 + paramVal, found := parsed[param]

647 if found {

648 for _, v := range vals {

649 v = repl.ReplaceAll(v, "")