From 03f463adf2da5a2c740a1c9259a2711a17e30912 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 11 Oct 2024 18:22:38 -0400 Subject: Move implementations to MatchWithError versions --- modules/caddyhttp/caddyhttp.go | 4 + modules/caddyhttp/celmatcher.go | 1 - modules/caddyhttp/ip_matchers.go | 10 +-- modules/caddyhttp/matchers.go | 188 ++++++++++++++++++++------------------- modules/caddyhttp/vars.go | 42 +++++---- 5 files changed, 129 insertions(+), 116 deletions(-) diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 218c0a069..8a52cb257 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -36,6 +36,10 @@ func init() { // RequestMatcher is a type that can match to a request. // A route matcher MUST NOT modify the request, with the // only exception being its context. +// +// DEPRECATED: Matchers should now implement RequestMatcherWithError. +// You may remove any interface guards for RequestMatcher +// but keep your Match() methods for backwards compatibility. type RequestMatcher interface { Match(*http.Request) bool } diff --git a/modules/caddyhttp/celmatcher.go b/modules/caddyhttp/celmatcher.go index a54cf72e8..fd6594e3b 100644 --- a/modules/caddyhttp/celmatcher.go +++ b/modules/caddyhttp/celmatcher.go @@ -756,7 +756,6 @@ const MatcherNameCtxKey = "matcher_name" // Interface guards var ( _ caddy.Provisioner = (*MatchExpression)(nil) - _ RequestMatcher = (*MatchExpression)(nil) _ RequestMatcherWithError = (*MatchExpression)(nil) _ caddyfile.Unmarshaler = (*MatchExpression)(nil) _ json.Marshaler = (*MatchExpression)(nil) diff --git a/modules/caddyhttp/ip_matchers.go b/modules/caddyhttp/ip_matchers.go index 948ee8da5..9205f53fa 100644 --- a/modules/caddyhttp/ip_matchers.go +++ b/modules/caddyhttp/ip_matchers.go @@ -145,11 +145,11 @@ func (m *MatchRemoteIP) Provision(ctx caddy.Context) error { // Match returns true if r matches m. func (m MatchRemoteIP) Match(r *http.Request) bool { - matches, err := m.MatchWithError(r) + match, err := m.MatchWithError(r) if err != nil { SetVar(r.Context(), MatcherErrorVarKey, err) } - return matches + return match } // MatchWithError returns true if r matches m. @@ -249,11 +249,11 @@ func (m *MatchClientIP) Provision(ctx caddy.Context) error { // Match returns true if r matches m. func (m MatchClientIP) Match(r *http.Request) bool { - matches, err := m.MatchWithError(r) + match, err := m.MatchWithError(r) if err != nil { SetVar(r.Context(), MatcherErrorVarKey, err) } - return matches + return match } // MatchWithError returns true if r matches m. @@ -348,13 +348,11 @@ func matchIPByCidrZones(clientIP netip.Addr, zoneID string, cidrs []*netip.Prefi // Interface guards var ( - _ RequestMatcher = (*MatchRemoteIP)(nil) _ RequestMatcherWithError = (*MatchRemoteIP)(nil) _ caddy.Provisioner = (*MatchRemoteIP)(nil) _ caddyfile.Unmarshaler = (*MatchRemoteIP)(nil) _ CELLibraryProducer = (*MatchRemoteIP)(nil) - _ RequestMatcher = (*MatchClientIP)(nil) _ RequestMatcherWithError = (*MatchClientIP)(nil) _ caddy.Provisioner = (*MatchClientIP)(nil) _ caddyfile.Unmarshaler = (*MatchClientIP)(nil) diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index d5f9e31c5..427ecf3c9 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -296,6 +296,12 @@ func (m MatchHost) Provision(_ caddy.Context) error { // Match returns true if r matches m. func (m MatchHost) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchHost) MatchWithError(r *http.Request) (bool, error) { reqHost, _, err := net.SplitHostPort(r.Host) if err != nil { // OK; probably didn't have a port @@ -315,7 +321,7 @@ func (m MatchHost) Match(r *http.Request) bool { return m[i] >= reqHost }) if pos < len(m) && m[pos] == reqHost { - return true + return true, nil } } @@ -346,18 +352,13 @@ outer: continue outer } } - return true + return true, nil } else if strings.EqualFold(reqHost, host) { - return true + return true, nil } } - return false -} - -// MatchWithError returns true if r matches m. -func (m MatchHost) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return false, nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -416,6 +417,12 @@ func (m MatchPath) Provision(_ caddy.Context) error { // Match returns true if r matches m. func (m MatchPath) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchPath) MatchWithError(r *http.Request) (bool, error) { // Even though RFC 9110 says that path matching is case-sensitive // (https://www.rfc-editor.org/rfc/rfc9110.html#section-4.2.3), // we do case-insensitive matching to mitigate security issues @@ -441,7 +448,7 @@ func (m MatchPath) Match(r *http.Request) bool { // special case: whole path is wildcard; this is unnecessary // as it matches all requests, which is the same as no matcher if matchPattern == "*" { - return true + return true, nil } // Clean the path, merge doubled slashes, etc. @@ -469,7 +476,7 @@ func (m MatchPath) Match(r *http.Request) bool { if strings.Contains(matchPattern, "%") { reqPathForPattern := CleanPath(r.URL.EscapedPath(), mergeSlashes) if m.matchPatternWithEscapeSequence(reqPathForPattern, matchPattern) { - return true + return true, nil } // doing prefix/suffix/substring matches doesn't make sense @@ -488,7 +495,7 @@ func (m MatchPath) Match(r *http.Request) bool { strings.HasPrefix(matchPattern, "*") && strings.HasSuffix(matchPattern, "*") { if strings.Contains(reqPathForPattern, matchPattern[1:len(matchPattern)-1]) { - return true + return true, nil } continue } @@ -500,7 +507,7 @@ func (m MatchPath) Match(r *http.Request) bool { // treat it as a fast suffix match if strings.HasPrefix(matchPattern, "*") { if strings.HasSuffix(reqPathForPattern, matchPattern[1:]) { - return true + return true, nil } continue } @@ -509,7 +516,7 @@ func (m MatchPath) Match(r *http.Request) bool { // treat it as a fast prefix match if strings.HasSuffix(matchPattern, "*") { if strings.HasPrefix(reqPathForPattern, matchPattern[:len(matchPattern)-1]) { - return true + return true, nil } continue } @@ -520,10 +527,10 @@ func (m MatchPath) Match(r *http.Request) bool { // because we can't handle it anyway matches, _ := path.Match(matchPattern, reqPathForPattern) if matches { - return true + return true, nil } } - return false + return false, nil } func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) bool { @@ -632,11 +639,6 @@ func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) b return matches } -// MatchWithError returns true if r matches m. -func (m MatchPath) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil -} - // CELLibrary produces options that expose this matcher for use in CEL // expression matchers. // @@ -687,6 +689,12 @@ func (MatchPathRE) CaddyModule() caddy.ModuleInfo { // Match returns true if r matches m. func (m MatchPathRE) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchPathRE) MatchWithError(r *http.Request) (bool, error) { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) // Clean the path, merges doubled slashes, etc. @@ -694,12 +702,7 @@ func (m MatchPathRE) Match(r *http.Request) bool { // the path matcher. See #4407 cleanedPath := cleanPath(r.URL.Path) - return m.MatchRegexp.Match(cleanedPath, repl) -} - -// MatchWithError returns true if r matches m. -func (m MatchPathRE) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return m.MatchRegexp.Match(cleanedPath, repl), nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -779,12 +782,13 @@ func (m *MatchMethod) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Match returns true if r matches m. func (m MatchMethod) Match(r *http.Request) bool { - return slices.Contains(m, r.Method) + match, _ := m.MatchWithError(r) + return match } // MatchWithError returns true if r matches m. func (m MatchMethod) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return slices.Contains(m, r.Method), nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -843,10 +847,17 @@ 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 { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +// An empty m matches an empty query string. +func (m MatchQuery) MatchWithError(r *http.Request) (bool, error) { // If no query keys are configured, this only // matches an empty query string. if len(m) == 0 { - return len(r.URL.Query()) == 0 + return len(r.URL.Query()) == 0, nil } repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) @@ -863,7 +874,7 @@ func (m MatchQuery) Match(r *http.Request) bool { // "Relying on parser alignment for security is doomed." Overall conclusion is that // splitting on & and rejecting ; in key=value pairs is safer than accepting raw ;. // We regard the Go team's decision as sound and thus reject malformed query strings. - return false + return false, nil } // Count the amount of matched keys, to ensure we AND @@ -874,7 +885,7 @@ func (m MatchQuery) Match(r *http.Request) bool { param = repl.ReplaceAll(param, "") paramVal, found := parsed[param] if !found { - return false + return false, nil } for _, v := range vals { v = repl.ReplaceAll(v, "") @@ -884,12 +895,7 @@ func (m MatchQuery) Match(r *http.Request) bool { } } } - return matchedKeys == len(m) -} - -// MatchWithError returns true if r matches m. -func (m MatchQuery) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return matchedKeys == len(m), nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -965,13 +971,14 @@ func (m *MatchHeader) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Match returns true if r matches m. func (m MatchHeader) Match(r *http.Request) bool { - repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - return matchHeaders(r.Header, http.Header(m), r.Host, repl) + match, _ := m.MatchWithError(r) + return match } // MatchWithError returns true if r matches m. func (m MatchHeader) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) + return matchHeaders(r.Header, http.Header(m), r.Host, repl), nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -1105,6 +1112,12 @@ func (m *MatchHeaderRE) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Match returns true if r matches m. func (m MatchHeaderRE) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchHeaderRE) MatchWithError(r *http.Request) (bool, error) { for field, rm := range m { actualFieldVals := getHeaderFieldVals(r.Header, field, r.Host) match := false @@ -1117,15 +1130,10 @@ func (m MatchHeaderRE) Match(r *http.Request) bool { } } if !match { - return false + return false, nil } } - return true -} - -// MatchWithError returns true if r matches m. -func (m MatchHeaderRE) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return true, nil } // Provision compiles m's regular expressions. @@ -1222,36 +1230,37 @@ func (MatchProtocol) CaddyModule() caddy.ModuleInfo { // Match returns true if r matches m. func (m MatchProtocol) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchProtocol) MatchWithError(r *http.Request) (bool, error) { switch string(m) { case "grpc": - return strings.HasPrefix(r.Header.Get("content-type"), "application/grpc") + return strings.HasPrefix(r.Header.Get("content-type"), "application/grpc"), nil case "https": - return r.TLS != nil + return r.TLS != nil, nil case "http": - return r.TLS == nil + return r.TLS == nil, nil case "http/1.0": - return r.ProtoMajor == 1 && r.ProtoMinor == 0 + return r.ProtoMajor == 1 && r.ProtoMinor == 0, nil case "http/1.0+": - return r.ProtoAtLeast(1, 0) + return r.ProtoAtLeast(1, 0), nil case "http/1.1": - return r.ProtoMajor == 1 && r.ProtoMinor == 1 + return r.ProtoMajor == 1 && r.ProtoMinor == 1, nil case "http/1.1+": - return r.ProtoAtLeast(1, 1) + return r.ProtoAtLeast(1, 1), nil case "http/2": - return r.ProtoMajor == 2 + return r.ProtoMajor == 2, nil case "http/2+": - return r.ProtoAtLeast(2, 0) + return r.ProtoAtLeast(2, 0), nil case "http/3": - return r.ProtoMajor == 3 + return r.ProtoMajor == 3, nil case "http/3+": - return r.ProtoAtLeast(3, 0) + return r.ProtoAtLeast(3, 0), nil } - return false -} - -// MatchWithError returns true if r matches m. -func (m MatchProtocol) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return false, nil } // UnmarshalCaddyfile implements caddyfile.Unmarshaler. @@ -1298,21 +1307,22 @@ func (MatchTLS) CaddyModule() caddy.ModuleInfo { // Match returns true if r matches m. func (m MatchTLS) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchTLS) MatchWithError(r *http.Request) (bool, error) { if r.TLS == nil { - return false + return false, nil } if m.HandshakeComplete != nil { if (!*m.HandshakeComplete && r.TLS.HandshakeComplete) || (*m.HandshakeComplete && !r.TLS.HandshakeComplete) { - return false + return false, nil } } - return true -} - -// MatchWithError returns true if r matches m. -func (m MatchTLS) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return true, nil } // UnmarshalCaddyfile parses Caddyfile tokens for this matcher. Syntax: @@ -1393,17 +1403,24 @@ func (m *MatchNot) Provision(ctx caddy.Context) error { // the embedded matchers, false is returned if any of its matcher // sets return true. func (m MatchNot) Match(r *http.Request) bool { - for _, ms := range m.MatcherSets { - if ms.Match(r) { - return false - } - } - return true + match, _ := m.MatchWithError(r) + return match } -// MatchWithError returns true if r matches m. +// MatchWithError returns true if r matches m. Since this matcher +// negates the embedded matchers, false is returned if any of its +// matcher sets return true. func (m MatchNot) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + for _, ms := range m.MatcherSets { + matches, err := ms.MatchWithError(r) + if err != nil { + return false, err + } + if matches { + return false, nil + } + } + return true, nil } // MatchRegexp is an embedable type for matching @@ -1578,26 +1595,17 @@ const MatcherErrorVarKey = "matchers.error" // Interface guards var ( - _ RequestMatcher = (*MatchHost)(nil) _ RequestMatcherWithError = (*MatchHost)(nil) _ caddy.Provisioner = (*MatchHost)(nil) - _ RequestMatcher = (*MatchPath)(nil) _ RequestMatcherWithError = (*MatchPath)(nil) - _ RequestMatcher = (*MatchPathRE)(nil) _ RequestMatcherWithError = (*MatchPathRE)(nil) _ caddy.Provisioner = (*MatchPathRE)(nil) - _ RequestMatcher = (*MatchMethod)(nil) _ RequestMatcherWithError = (*MatchMethod)(nil) - _ RequestMatcher = (*MatchQuery)(nil) _ RequestMatcherWithError = (*MatchQuery)(nil) - _ RequestMatcher = (*MatchHeader)(nil) _ RequestMatcherWithError = (*MatchHeader)(nil) - _ RequestMatcher = (*MatchHeaderRE)(nil) _ RequestMatcherWithError = (*MatchHeaderRE)(nil) _ caddy.Provisioner = (*MatchHeaderRE)(nil) - _ RequestMatcher = (*MatchProtocol)(nil) _ RequestMatcherWithError = (*MatchProtocol)(nil) - _ RequestMatcher = (*MatchNot)(nil) _ RequestMatcherWithError = (*MatchNot)(nil) _ caddy.Provisioner = (*MatchNot)(nil) _ caddy.Provisioner = (*MatchRegexp)(nil) diff --git a/modules/caddyhttp/vars.go b/modules/caddyhttp/vars.go index 280843e70..f884c0993 100644 --- a/modules/caddyhttp/vars.go +++ b/modules/caddyhttp/vars.go @@ -166,8 +166,14 @@ func (m *VarsMatcher) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Match matches a request based on variables in the context, // or placeholders if the key is not a variable. func (m VarsMatcher) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m VarsMatcher) MatchWithError(r *http.Request) (bool, error) { if len(m) == 0 { - return true + return true, nil } vars := r.Context().Value(VarsCtxKey).(map[string]any) @@ -200,16 +206,11 @@ func (m VarsMatcher) Match(r *http.Request) bool { varStr = fmt.Sprintf("%v", vv) } if varStr == matcherValExpanded { - return true + return true, nil } } } - return false -} - -// MatchWithError returns true if r matches m. -func (m VarsMatcher) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return false, nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -299,6 +300,12 @@ func (m MatchVarsRE) Provision(ctx caddy.Context) error { // Match returns true if r matches m. func (m MatchVarsRE) Match(r *http.Request) bool { + match, _ := m.MatchWithError(r) + return match +} + +// MatchWithError returns true if r matches m. +func (m MatchVarsRE) MatchWithError(r *http.Request) (bool, error) { vars := r.Context().Value(VarsCtxKey).(map[string]any) repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) for key, val := range m { @@ -327,15 +334,10 @@ func (m MatchVarsRE) Match(r *http.Request) bool { valExpanded := repl.ReplaceAll(varStr, "") if match := val.Match(valExpanded, repl); match { - return match + return match, nil } } - return false -} - -// MatchWithError returns true if r matches m. -func (m MatchVarsRE) MatchWithError(r *http.Request) (bool, error) { - return m.Match(r), nil + return false, nil } // CELLibrary produces options that expose this matcher for use in CEL @@ -445,8 +447,10 @@ func SetVar(ctx context.Context, key string, value any) { // Interface guards var ( - _ MiddlewareHandler = (*VarsMiddleware)(nil) - _ caddyfile.Unmarshaler = (*VarsMiddleware)(nil) - _ RequestMatcher = (*VarsMatcher)(nil) - _ caddyfile.Unmarshaler = (*VarsMatcher)(nil) + _ MiddlewareHandler = (*VarsMiddleware)(nil) + _ caddyfile.Unmarshaler = (*VarsMiddleware)(nil) + _ RequestMatcherWithError = (*VarsMatcher)(nil) + _ caddyfile.Unmarshaler = (*VarsMatcher)(nil) + _ RequestMatcherWithError = (*MatchVarsRE)(nil) + _ caddyfile.Unmarshaler = (*MatchVarsRE)(nil) ) -- cgit v1.2.3