perf(gotenberg): match allow and deny patterns without recompiling them

pkg/gotenberg/filter.go

@@ -1,70 +0,0 @@
-package gotenberg
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"time"
-
-	"github.com/dlclark/regexp2"
-)
-
-// ErrFiltered happens if a value is filtered by the [FilterDeadline] function.
-var ErrFiltered = errors.New("value filtered")
-
-// FilterDeadline checks if the given value is allowed and not denied according
-// to regex patterns. The allowed list uses OR semantics (value must match at
-// least one pattern). The denied list uses OR semantics (value is denied if it
-// matches any pattern). It returns a [context.DeadlineExceeded] if it takes
-// too long to process.
-func FilterDeadline(allowed, denied []*regexp2.Regexp, s string, deadline time.Time) error {
-	if len(allowed) > 0 {
-		matched := false
-
-		for _, pattern := range allowed {
-			// FIXME: not ideal to compile everytime, but is there another way to create a clone?
-			clone := regexp2.MustCompile(pattern.String(), 0)
-			clone.MatchTimeout = time.Until(deadline)
-
-			ok, err := clone.MatchString(s)
-			if err != nil {
-				if time.Now().After(deadline) {
-					return context.DeadlineExceeded
-				}
-
-				return fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), s, err)
-			}
-
-			if ok {
-				matched = true
-				break
-			}
-		}
-
-		if !matched {
-			return fmt.Errorf("'%s' does not match any expression from the allowed list: %w", s, ErrFiltered)
-		}
-	}
-
-	if len(denied) > 0 {
-		for _, pattern := range denied {
-			clone := regexp2.MustCompile(pattern.String(), 0)
-			clone.MatchTimeout = time.Until(deadline)
-
-			ok, err := clone.MatchString(s)
-			if err != nil {
-				if time.Now().After(deadline) {
-					return context.DeadlineExceeded
-				}
-
-				return fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), s, err)
-			}
-
-			if ok {
-				return fmt.Errorf("'%s' matches the expression from the denied list: %w", s, ErrFiltered)
-			}
-		}
-	}
-
-	return nil
-}

pkg/gotenberg/filter_test.go

@@ -1,117 +0,0 @@
-package gotenberg
-
-import (
-	"context"
-	"errors"
-	"testing"
-	"time"
-
-	"github.com/dlclark/regexp2"
-)
-
-func TestFilterDeadline(t *testing.T) {
-	for _, tc := range []struct {
-		scenario      string
-		allowed       []*regexp2.Regexp
-		denied        []*regexp2.Regexp
-		s             string
-		deadline      time.Time
-		expectError   bool
-		expectedError error
-	}{
-		{
-			scenario:      "DeadlineExceeded (allowed)",
-			allowed:       []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
-			denied:        nil,
-			s:             "foo",
-			deadline:      time.Now().Add(time.Duration(-1) * time.Hour),
-			expectError:   true,
-			expectedError: context.DeadlineExceeded,
-		},
-		{
-			scenario:      "ErrFiltered (allowed, no match)",
-			allowed:       []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
-			denied:        nil,
-			s:             "bar",
-			deadline:      time.Now().Add(time.Duration(5) * time.Second),
-			expectError:   true,
-			expectedError: ErrFiltered,
-		},
-		{
-			scenario:      "DeadlineExceeded (denied)",
-			allowed:       nil,
-			denied:        []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
-			s:             "foo",
-			deadline:      time.Now().Add(time.Duration(-1) * time.Hour),
-			expectError:   true,
-			expectedError: context.DeadlineExceeded,
-		},
-		{
-			scenario:      "ErrFiltered (denied)",
-			allowed:       nil,
-			denied:        []*regexp2.Regexp{regexp2.MustCompile("foo", 0)},
-			s:             "foo",
-			deadline:      time.Now().Add(time.Duration(5) * time.Second),
-			expectError:   true,
-			expectedError: ErrFiltered,
-		},
-		{
-			scenario:    "success (empty lists)",
-			allowed:     nil,
-			denied:      nil,
-			s:           "foo",
-			deadline:    time.Now().Add(time.Duration(5) * time.Second),
-			expectError: false,
-		},
-		{
-			scenario:    "multi-pattern allow list, second matches",
-			allowed:     []*regexp2.Regexp{regexp2.MustCompile("^https://", 0), regexp2.MustCompile("^file:///tmp/", 0)},
-			denied:      nil,
-			s:           "file:///tmp/abc/index.html",
-			deadline:    time.Now().Add(time.Duration(5) * time.Second),
-			expectError: false,
-		},
-		{
-			scenario:      "multi-pattern allow list, none matches",
-			allowed:       []*regexp2.Regexp{regexp2.MustCompile("^https://", 0), regexp2.MustCompile("^ftp://", 0)},
-			denied:        nil,
-			s:             "file:///tmp/abc/index.html",
-			deadline:      time.Now().Add(time.Duration(5) * time.Second),
-			expectError:   true,
-			expectedError: ErrFiltered,
-		},
-		{
-			scenario:      "multi-pattern deny list, second matches",
-			allowed:       nil,
-			denied:        []*regexp2.Regexp{regexp2.MustCompile("^ftp://", 0), regexp2.MustCompile("^file:.*", 0)},
-			s:             "file:///etc/passwd",
-			deadline:      time.Now().Add(time.Duration(5) * time.Second),
-			expectError:   true,
-			expectedError: ErrFiltered,
-		},
-		{
-			scenario:    "https URL passes deny list targeting file://",
-			allowed:     nil,
-			denied:      []*regexp2.Regexp{regexp2.MustCompile("^file:.*", 0)},
-			s:           "https://example.com",
-			deadline:    time.Now().Add(time.Duration(5) * time.Second),
-			expectError: false,
-		},
-	} {
-		t.Run(tc.scenario, func(t *testing.T) {
-			err := FilterDeadline(tc.allowed, tc.denied, tc.s, tc.deadline)
-
-			if tc.expectError && err == nil {
-				t.Fatal("expected an error but got none")
-			}
-
-			if !tc.expectError && err != nil {
-				t.Fatalf("expected no error but got: %v", err)
-			}
-
-			if tc.expectedError != nil && !errors.Is(err, tc.expectedError) {
-				t.Fatalf("expected error %v but got: %v", tc.expectedError, err)
-			}
-		})
-	}
-}

pkg/gotenberg/flags.go

@@ -207,15 +207,38 @@ func (f *ParsedFlags) MustDeprecatedHumanReadableBytes(deprecated string, newNam
 	return f.MustHumanReadableBytes(newName)
 }
 
+// PatternMatchTimeout bounds a single match against an operator-supplied
+// allow-list or deny-list pattern.
+//
+// regexp2 backtracks, and the strings matched against these patterns are
+// client-controlled: a request URL, a CONNECT host. A pattern that backtracks
+// catastrophically would otherwise burn a core for as long as the caller's
+// deadline allows, which is --api-timeout (env API_TIMEOUT), 30 seconds by
+// default. The ceiling mirrors the one the Chromium module already applies to
+// the per-request extraHttpHeaders scope pattern.
+//
+// [ParsedFlags.MustRegexp] and [ParsedFlags.MustRegexpSlice] stamp this onto
+// every pattern they compile, which is how all four production lists are
+// built. Patterns compiled any other way keep regexp2's default of
+// math.MaxInt64, which it treats as no timeout at all, so a hand-built slice
+// must set this itself before reaching [DecideOutbound].
+const PatternMatchTimeout = 250 * time.Millisecond
+
 // MustRegexp returns the regular expression of a flag given by name.
 // It panics if an error occurs.
+//
+// The returned expression carries [PatternMatchTimeout] and is safe to match
+// on concurrently: callers must not compile a private copy per match.
 func (f *ParsedFlags) MustRegexp(name string) *regexp2.Regexp {
 	val, err := f.GetString(name)
 	if err != nil {
 		panic(err)
 	}
 
-	return regexp2.MustCompile(val, 0)
+	re := regexp2.MustCompile(val, 0)
+	re.MatchTimeout = PatternMatchTimeout
+
+	return re
 }
 
 // MustDeprecatedRegexp returns the regular expression of a deprecated flag if
@@ -235,6 +258,9 @@ func (f *ParsedFlags) MustDeprecatedRegexp(deprecated string, newName string) *r
 //
 // Every allow-list and deny-list in Gotenberg is read through this method, so
 // it is also where allow-list patterns are audited. See [AuditAllowList].
+//
+// The returned expressions carry [PatternMatchTimeout] and are safe to match
+// on concurrently: callers must not compile a private copy per match.
 func (f *ParsedFlags) MustRegexpSlice(name string) []*regexp2.Regexp {
 	vals := f.MustStringSlice(name)
 
@@ -246,7 +272,10 @@ func (f *ParsedFlags) MustRegexpSlice(name string) []*regexp2.Regexp {
 			continue
 		}
 
-		regexps = append(regexps, regexp2.MustCompile(val, 0))
+		re := regexp2.MustCompile(val, 0)
+		re.MatchTimeout = PatternMatchTimeout
+
+		regexps = append(regexps, re)
 	}
 
 	return regexps

pkg/gotenberg/flags_test.go

@@ -1054,3 +1054,36 @@ func TestEnvVarName(t *testing.T) {
 		})
 	}
 }
+
+func TestParsedFlags_RegexpMatchTimeout(t *testing.T) {
+	// [DecideOutbound] matches on these patterns directly instead of compiling
+	// a private copy per call, so the bound has to come from here. regexp2's
+	// own default is math.MaxInt64, which it treats as no
+	// timeout at all, so a pattern built without this stamp runs unbounded
+	// against a client-controlled string.
+	fs := flag.NewFlagSet("tests", flag.ContinueOnError)
+	fs.StringSlice("some-deny-list", []string{`^file:`, `^https?://`}, "")
+	fs.String("some-pattern", `^file:`, "")
+
+	err := fs.Parse(nil)
+	if err != nil {
+		t.Fatalf("expected no error but got: %v", err)
+	}
+
+	parsedFlags := ParsedFlags{FlagSet: fs}
+
+	regexps := parsedFlags.MustRegexpSlice("some-deny-list")
+	if len(regexps) != 2 {
+		t.Fatalf("expected 2 patterns but got %d", len(regexps))
+	}
+
+	for _, re := range regexps {
+		if re.MatchTimeout != PatternMatchTimeout {
+			t.Fatalf("pattern '%s' has MatchTimeout %s, expected %s", re.String(), re.MatchTimeout, PatternMatchTimeout)
+		}
+	}
+
+	if got := parsedFlags.MustRegexp("some-pattern").MatchTimeout; got != PatternMatchTimeout {
+		t.Fatalf("expected MustRegexp MatchTimeout %s but got %s", PatternMatchTimeout, got)
+	}
+}

pkg/gotenberg/outbound.go

@@ -27,6 +27,12 @@ import (
 // example [::ffff:127.0.0.1]).
 var ErrNonPublicIP = errors.New("non-public IP")
 
+// ErrFiltered happens when a value is rejected by an allow-list or a
+// deny-list, or when it cannot be validated and [DecideOutbound] fails closed.
+// Callers map it to a generic 403: the specific reason stays in the operator
+// logs so a client cannot probe the lists.
+var ErrFiltered = errors.New("value filtered")
+
 // ErrPublicIP indicates that an outbound URL targets an IP address that is
 // reachable on the public internet. It is returned when a caller opts
 // into denying public destinations via [WithDenyPublicIPs]; typical use
@@ -291,6 +297,15 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
 		opt(&cfg)
 	}
 
+	// Each match is bounded by [PatternMatchTimeout] rather than by the
+	// remaining budget, so an already-spent deadline no longer surfaces from
+	// the match itself. Schemes that resolve a host still learn about it from
+	// resolveHost, but a non-matching file:// or data: URL returns before that
+	// point, so check it here to keep failing closed on every path.
+	if !time.Now().Before(deadline) {
+		return OutboundDecision{}, context.DeadlineExceeded
+	}
+
 	parsed, err := url.Parse(rawURL)
 	if err != nil {
 		return OutboundDecision{}, fmt.Errorf("parse URL %q: %w", rawURL, ErrFiltered)
@@ -314,15 +329,12 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
 	allowMatched := false
 	if len(allowList) > 0 {
 		for _, pattern := range allowList {
-			clone := regexp2.MustCompile(pattern.String(), 0)
+			ok, err := pattern.MatchString(normalized)
-			clone.MatchTimeout = time.Until(deadline)
-
-			ok, err := clone.MatchString(normalized)
 			if err != nil {
 				if time.Now().After(deadline) {
 					return OutboundDecision{}, context.DeadlineExceeded
 				}
-				return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), normalized, err)
+				return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", pattern.String(), normalized, err)
 			}
 
 			if ok {
@@ -337,15 +349,12 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
 	}
 
 	for _, pattern := range denyList {
-		clone := regexp2.MustCompile(pattern.String(), 0)
+		ok, err := pattern.MatchString(normalized)
-		clone.MatchTimeout = time.Until(deadline)
-
-		ok, err := clone.MatchString(normalized)
 		if err != nil {
 			if time.Now().After(deadline) {
 				return OutboundDecision{}, context.DeadlineExceeded
 			}
-			return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), normalized, err)
+			return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", pattern.String(), normalized, err)
 		}
 
 		if ok {
@@ -392,9 +401,8 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
 }
 
 // FilterOutboundURL validates that rawURL is acceptable for an outbound
-// request from Gotenberg. It is the URL-aware replacement for
+// request from Gotenberg. Prefer it for any new code that filters a URL
-// [FilterDeadline] and should be preferred for any new code that filters
+// before issuing or instructing an outbound request.
-// a URL before issuing or instructing an outbound request.
 //
 // The default behavior is permissive: the URL passes as long as it clears
 // the regex allow-list and deny-list. Callers that need IP-class checks

pkg/gotenberg/outbound_test.go

@@ -715,3 +715,54 @@ func TestNewOutboundHttpClient_NonPositiveTimeout(t *testing.T) {
 		t.Fatalf("timeout for a negative budget = %s, want a positive value so the client fails closed", got)
 	}
 }
+
+func TestDecideOutboundExpiredDeadline(t *testing.T) {
+	// Patterns are matched under the fixed PatternMatchTimeout rather than
+	// under the caller's remaining budget, so an expired deadline no longer
+	// surfaces from the match itself. Every scheme must still fail closed,
+	// including the ones that return before a host is resolved.
+	expired := time.Now().Add(-time.Second)
+
+	for _, rawURL := range []string{
+		"https://example.com/",
+		"file:///tmp/foo.html",
+		"data:text/html,hello",
+	} {
+		_, err := DecideOutbound(context.Background(), rawURL, nil, nil, expired)
+		if !errors.Is(err, context.DeadlineExceeded) {
+			t.Fatalf("DecideOutbound(%q) with an expired deadline = %v, want context.DeadlineExceeded", rawURL, err)
+		}
+	}
+}
+
+func TestDecideOutboundBoundsCatastrophicPatterns(t *testing.T) {
+	// A deny-list pattern that backtracks catastrophically, matched against a
+	// client-controlled URL. Before PatternMatchTimeout the ceiling was the
+	// caller's whole budget, so a 30s API_TIMEOUT bought a 30s CPU burn.
+	// The trailing "!" makes the match fail only after the nested quantifier
+	// has explored every way to split the run of "a"s.
+	pattern := regexp2.MustCompile(`^https://example\.com/(a+)+$`, 0)
+	pattern.MatchTimeout = PatternMatchTimeout
+
+	rawURL := "https://example.com/" + strings.Repeat("a", 40) + "!"
+
+	start := time.Now()
+	_, err := DecideOutbound(
+		context.Background(),
+		rawURL,
+		nil,
+		[]*regexp2.Regexp{pattern},
+		time.Now().Add(30*time.Second),
+	)
+	elapsed := time.Since(start)
+
+	if err == nil {
+		t.Fatal("expected an error from a catastrophic deny-list pattern")
+	}
+
+	// Generous headroom over the 250ms ceiling, still far below the 30s
+	// deadline the match would otherwise have been allowed to consume.
+	if elapsed > 5*time.Second {
+		t.Fatalf("match took %s, want it aborted near PatternMatchTimeout (%s)", elapsed, PatternMatchTimeout)
+	}
+}