From 815f58631535e90b1734b7dcfe49607f92fc49a0 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Fri, 7 Aug 2026 16:29:55 +0200 Subject: [PATCH] fix(chromium): bound the total scope matching time per conversion --- pkg/modules/chromium/events.go | 25 +++++ pkg/modules/chromium/routes.go | 25 ++++- pkg/modules/chromium/scopebudget.go | 52 ++++++++++ pkg/modules/chromium/scopebudget_test.go | 122 +++++++++++++++++++++++ 4 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 pkg/modules/chromium/scopebudget.go create mode 100644 pkg/modules/chromium/scopebudget_test.go diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index 40d6c2b5..b01879f5 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -10,6 +10,7 @@ import ( "slices" "strings" "sync" + "time" "github.com/chromedp/cdproto/cdp" "github.com/chromedp/cdproto/fetch" @@ -62,6 +63,11 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio logger.DebugContext(ctx, fmt.Sprintf("extra HTTP headers: %+v", options.extraHttpHeaders)) } + // Shared by every scope match of this conversion, across all paused + // requests. Its lifetime is the conversion, as this function is called once + // per conversion with that conversion's context. + budget := newScopeMatchBudget(scopeMatchBudgetPerConversion) + chromedp.ListenTarget(ctx, func(ev any) { if e, ok := ev.(*fetch.EventRequestPaused); ok { go func() { @@ -127,6 +133,14 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio // First, we have to check if at least one header has to be // set for the current request. for _, header := range options.extraHttpHeaders { + // This goroutine outlives the response: nothing cancels an + // in-flight match, so stop as soon as the conversion is over. + select { + case <-ctx.Done(): + return + default: + } + if header.Scope == nil { // Non-scoped header. logger.DebugContext(ctx, fmt.Sprintf("extra HTTP header '%s' will be set for request URL '%s'", header.Name, e.Request.URL)) @@ -134,7 +148,18 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio continue } + if !budget.tryAcquire() { + // Treat the remaining scoped headers as non-matching rather + // than spending more CPU on a request the client may already + // have given up on. + logger.WarnContext(ctx, fmt.Sprintf("scope matching budget of %s exhausted, extra HTTP header '%s' and any subsequent scoped header will not be set; simplify the 'scope' patterns or reduce the number of scoped headers", scopeMatchBudgetPerConversion, header.Name)) + break + } + + matchStart := time.Now() ok, err := header.Scope.MatchString(e.Request.URL) + budget.consume(time.Since(matchStart)) + switch { case err != nil: logger.ErrorContext(ctx, fmt.Sprintf("fail to match extra HTTP header '%s' scope with URL '%s': %s", header.Name, e.Request.URL, err)) diff --git a/pkg/modules/chromium/routes.go b/pkg/modules/chromium/routes.go index 01ca7075..f7c26a4f 100644 --- a/pkg/modules/chromium/routes.go +++ b/pkg/modules/chromium/routes.go @@ -24,6 +24,20 @@ import ( "github.com/gotenberg/gotenberg/v8/pkg/modules/pdfengines" ) +// Bounds on the scoped extra HTTP headers feature. Chromium matches every +// scoped header against every paused sub-resource request, so the total +// matching work is the product of the header count and the sub-resource count. +// These caps bound the factors the client controls; [scopeMatchBudget] bounds +// the product. See https://github.com/gotenberg/gotenberg/issues/1588. +const ( + maxExtraHttpHeaders = 64 + maxExtraHttpHeaderScopeLength = 1024 + + // A scope pattern matches against a URL, which takes microseconds for any + // reasonable pattern. + extraHttpHeaderScopeMatchTimeout = 250 * time.Millisecond +) + var sameSiteRegexp = regexp2.MustCompile( `("sameSite"\s*:\s*")(?i:(lax|strict|none))(")`, regexp2.None, @@ -169,6 +183,10 @@ func FormDataChromiumOptions(ctx *api.Context) (*api.FormData, Options) { return fmt.Errorf("unmarshal extraHttpHeaders: %w", err) } + if len(headers) > maxExtraHttpHeaders { + return fmt.Errorf("too many headers, got %d, expected at most %d", len(headers), maxExtraHttpHeaders) + } + for k, v := range headers { var scope string var valueTokens []string @@ -198,12 +216,17 @@ func FormDataChromiumOptions(ctx *api.Context) (*api.FormData, Options) { var scopeRegexp *regexp2.Regexp if len(scope) > 0 { + if len(scope) > maxExtraHttpHeaderScopeLength { + err = errors.Join(err, fmt.Errorf("scope regex pattern for header '%s' is too long, got %d characters, expected at most %d", k, len(scope), maxExtraHttpHeaderScopeLength)) + continue + } + p, errCompile := regexp2.Compile(scope, regexp2.None) if errCompile != nil { err = errors.Join(err, fmt.Errorf("invalid scope regex pattern for header '%s': %w", k, errCompile)) continue } - p.MatchTimeout = 5 * time.Second + p.MatchTimeout = extraHttpHeaderScopeMatchTimeout scopeRegexp = p } diff --git a/pkg/modules/chromium/scopebudget.go b/pkg/modules/chromium/scopebudget.go new file mode 100644 index 00000000..7a5d83c1 --- /dev/null +++ b/pkg/modules/chromium/scopebudget.go @@ -0,0 +1,52 @@ +package chromium + +import ( + "sync/atomic" + "time" +) + +// scopeMatchBudgetPerConversion caps the total time a single conversion may +// spend matching scoped extra HTTP header patterns. +// +// The per-pattern MatchTimeout bounds one match, not their number: Chromium +// pauses every sub-resource request, and each paused request is matched against +// every scoped header. Without a shared budget the total is the product of the +// two, both of which the client controls. +// See https://github.com/gotenberg/gotenberg/issues/1588. +const scopeMatchBudgetPerConversion = 5 * time.Second + +// scopeMatchBudget is a time allowance shared by every scope match of a +// conversion. It is safe for concurrent use: paused requests are handled on +// their own goroutines. +type scopeMatchBudget struct { + remaining atomic.Int64 +} + +// newScopeMatchBudget returns a [scopeMatchBudget] allowing d of matching. +func newScopeMatchBudget(d time.Duration) *scopeMatchBudget { + b := new(scopeMatchBudget) + b.remaining.Store(int64(d)) + return b +} + +// tryAcquire reports whether the budget still allows a match. +func (b *scopeMatchBudget) tryAcquire() bool { + return b.remaining.Load() > 0 +} + +// consume subtracts the time a match took. It saturates at zero so that a long +// match cannot wrap the counter back into credit. +func (b *scopeMatchBudget) consume(d time.Duration) { + for { + current := b.remaining.Load() + if current <= 0 { + return + } + + next := max(current-int64(d), 0) + + if b.remaining.CompareAndSwap(current, next) { + return + } + } +} diff --git a/pkg/modules/chromium/scopebudget_test.go b/pkg/modules/chromium/scopebudget_test.go new file mode 100644 index 00000000..bb56e9ea --- /dev/null +++ b/pkg/modules/chromium/scopebudget_test.go @@ -0,0 +1,122 @@ +package chromium + +import ( + "strings" + "sync" + "testing" + "time" + + "github.com/dlclark/regexp2" +) + +func TestScopeMatchBudget(t *testing.T) { + t.Run("allows matching while credit remains", func(t *testing.T) { + b := newScopeMatchBudget(time.Second) + if !b.tryAcquire() { + t.Fatal("tryAcquire() = false on a fresh budget, want true") + } + }) + + t.Run("denies matching once exhausted", func(t *testing.T) { + b := newScopeMatchBudget(time.Second) + b.consume(time.Second) + if b.tryAcquire() { + t.Error("tryAcquire() = true after the budget was spent, want false") + } + }) + + t.Run("saturates at zero instead of wrapping into credit", func(t *testing.T) { + b := newScopeMatchBudget(time.Second) + b.consume(time.Hour) + if got := b.remaining.Load(); got != 0 { + t.Errorf("remaining = %d, want 0", got) + } + if b.tryAcquire() { + t.Error("tryAcquire() = true after an overlong match, want false") + } + }) + + t.Run("a spent budget stays spent", func(t *testing.T) { + b := newScopeMatchBudget(time.Second) + b.consume(time.Second) + b.consume(time.Millisecond) + if got := b.remaining.Load(); got != 0 { + t.Errorf("remaining = %d, want 0", got) + } + }) + + t.Run("is safe for concurrent use", func(t *testing.T) { + const goroutines = 64 + // Each goroutine spends 1ms against a budget of half that many + // milliseconds, so the total spend overshoots it. + b := newScopeMatchBudget(time.Duration(goroutines/2) * time.Millisecond) + + var wg sync.WaitGroup + for range goroutines { + wg.Go(func() { + b.tryAcquire() + b.consume(time.Millisecond) + }) + } + wg.Wait() + + if got := b.remaining.Load(); got != 0 { + t.Errorf("remaining = %d, want 0", got) + } + }) +} + +// TestScopeMatchBudget_BoundsCatastrophicBacktracking is the regression test for +// the amplification: many scoped headers matched against a hostile URL must cost +// the budget, not a multiple of it. +// See https://github.com/gotenberg/gotenberg/issues/1588. +func TestScopeMatchBudget_BoundsCatastrophicBacktracking(t *testing.T) { + const ( + headers = 16 + budget = 200 * time.Millisecond + ) + + // Nested quantifier with no possible match: classic catastrophic + // backtracking. + pattern := compileScopePattern(t, `(a+)+b`) + url := "http://example.com/" + strings.Repeat("a", 40) + + b := newScopeMatchBudget(budget) + + start := time.Now() + var matched int + for range headers { + if !b.tryAcquire() { + break + } + matchStart := time.Now() + _, _ = pattern.MatchString(url) + b.consume(time.Since(matchStart)) + matched++ + } + elapsed := time.Since(start) + + if matched == headers { + t.Errorf("all %d headers were matched, want the budget to stop matching early", headers) + } + + // Each match is separately capped at extraHttpHeaderScopeMatchTimeout, so + // the worst case is the budget plus one final match that started with the + // last of the credit. Generous slack keeps this stable on a loaded CI box. + ceiling := budget + extraHttpHeaderScopeMatchTimeout + time.Second + if elapsed > ceiling { + t.Errorf("matching took %s, want at most %s", elapsed, ceiling) + } +} + +func compileScopePattern(t *testing.T, pattern string) *regexp2.Regexp { + t.Helper() + + p, err := regexp2.Compile(pattern, regexp2.None) + if err != nil { + t.Fatalf("compile %q: %v", pattern, err) + } + p.MatchTimeout = extraHttpHeaderScopeMatchTimeout + + return p +}