mirror of
https://github.com/gotenberg/gotenberg.git
synced 2026-10-16 01:03:17 +01:00
fix(gotenberg): retry pattern matches aborted by a process pause
This commit is contained in:
@@ -222,6 +222,10 @@ func (f *ParsedFlags) MustDeprecatedHumanReadableBytes(deprecated string, newNam
|
|||||||
// built. Patterns compiled any other way keep regexp2's default of
|
// 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
|
// math.MaxInt64, which it treats as no timeout at all, so a hand-built slice
|
||||||
// must set this itself before reaching [DecideOutbound].
|
// must set this itself before reaching [DecideOutbound].
|
||||||
|
//
|
||||||
|
// The ceiling is wall-clock. Match through [MatchPattern] rather than calling
|
||||||
|
// regexp2 directly: a match that never approaches the ceiling still aborts if
|
||||||
|
// the process loses the CPU at the wrong moment.
|
||||||
const PatternMatchTimeout = 250 * time.Millisecond
|
const PatternMatchTimeout = 250 * time.Millisecond
|
||||||
|
|
||||||
// MustRegexp returns the regular expression of a flag given by name.
|
// MustRegexp returns the regular expression of a flag given by name.
|
||||||
|
|||||||
@@ -336,12 +336,16 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
|
|||||||
allowMatched := false
|
allowMatched := false
|
||||||
if len(allowList) > 0 {
|
if len(allowList) > 0 {
|
||||||
for _, pattern := range allowList {
|
for _, pattern := range allowList {
|
||||||
ok, err := pattern.MatchString(normalized)
|
ok, err := MatchPattern(pattern, normalized)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if time.Now().After(deadline) {
|
if time.Now().After(deadline) {
|
||||||
return OutboundDecision{}, context.DeadlineExceeded
|
return OutboundDecision{}, context.DeadlineExceeded
|
||||||
}
|
}
|
||||||
return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", pattern.String(), normalized, err)
|
// The pattern could not be evaluated, so the URL cannot be
|
||||||
|
// cleared for the IP-check bypass an allow-list match grants.
|
||||||
|
// Fail closed like an unresolvable host does below, so the
|
||||||
|
// client gets a generic 403 rather than a 500.
|
||||||
|
return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %v: %w", pattern.String(), normalized, err, ErrFiltered)
|
||||||
}
|
}
|
||||||
|
|
||||||
if ok {
|
if ok {
|
||||||
@@ -356,12 +360,15 @@ func DecideOutbound(ctx context.Context, rawURL string, allowList, denyList []*r
|
|||||||
}
|
}
|
||||||
|
|
||||||
for _, pattern := range denyList {
|
for _, pattern := range denyList {
|
||||||
ok, err := pattern.MatchString(normalized)
|
ok, err := MatchPattern(pattern, normalized)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if time.Now().After(deadline) {
|
if time.Now().After(deadline) {
|
||||||
return OutboundDecision{}, context.DeadlineExceeded
|
return OutboundDecision{}, context.DeadlineExceeded
|
||||||
}
|
}
|
||||||
return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", pattern.String(), normalized, err)
|
// The pattern could not be evaluated, so the URL cannot be proven
|
||||||
|
// to fall outside the deny-list. Fail closed rather than letting a
|
||||||
|
// deny-list that never ran pass the request through.
|
||||||
|
return OutboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %v: %w", pattern.String(), normalized, err, ErrFiltered)
|
||||||
}
|
}
|
||||||
|
|
||||||
if ok {
|
if ok {
|
||||||
|
|||||||
@@ -793,6 +793,13 @@ func TestDecideOutboundBoundsCatastrophicPatterns(t *testing.T) {
|
|||||||
t.Fatal("expected an error from a catastrophic deny-list pattern")
|
t.Fatal("expected an error from a catastrophic deny-list pattern")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A deny-list that could not be evaluated cannot clear the URL, so the
|
||||||
|
// decision fails closed and the client gets a generic 403 rather than a
|
||||||
|
// 500 naming the pattern.
|
||||||
|
if !errors.Is(err, ErrFiltered) {
|
||||||
|
t.Fatalf("expected ErrFiltered from an unevaluable deny-list pattern but got: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
// Generous headroom over the 250ms ceiling, still far below the 30s
|
// Generous headroom over the 250ms ceiling, still far below the 30s
|
||||||
// deadline the match would otherwise have been allowed to consume.
|
// deadline the match would otherwise have been allowed to consume.
|
||||||
if elapsed > 5*time.Second {
|
if elapsed > 5*time.Second {
|
||||||
|
|||||||
55
pkg/gotenberg/pattern.go
Normal file
55
pkg/gotenberg/pattern.go
Normal file
@@ -0,0 +1,55 @@
|
|||||||
|
package gotenberg
|
||||||
|
|
||||||
|
import (
|
||||||
|
"github.com/dlclark/regexp2"
|
||||||
|
)
|
||||||
|
|
||||||
|
// patternMatchAttempts caps how many times [MatchPattern] runs one pattern
|
||||||
|
// against one string. It is what keeps a pattern that is genuinely out of
|
||||||
|
// budget from retrying forever: three attempts bound its cost at three
|
||||||
|
// [PatternMatchTimeout], which is still two orders of magnitude below the
|
||||||
|
// --api-timeout (env API_TIMEOUT) the ceiling exists to protect.
|
||||||
|
const patternMatchAttempts = 3
|
||||||
|
|
||||||
|
// MatchPattern reports whether s matches pattern. It bounds the match by the
|
||||||
|
// pattern's MatchTimeout without the false timeouts that the bound alone
|
||||||
|
// produces.
|
||||||
|
//
|
||||||
|
// regexp2 does not time a match against [time.Now]. It derives the deadline
|
||||||
|
// from a process-global clock that a background goroutine advances every
|
||||||
|
// 100ms, and it tests that deadline on the very first step of the match.
|
||||||
|
// Anything that stops the whole process, a cgroup CPU-quota throttle or a long
|
||||||
|
// stop-the-world pause, also stops that goroutine, which then advances the
|
||||||
|
// clock by the full pause in a single write. A match holding a deadline from
|
||||||
|
// before that jump aborts whatever work it had done: a 366ns match against a
|
||||||
|
// short URL reports "match timeout after 250ms". The abort lands on whichever
|
||||||
|
// match straddles the jump rather than on a match that was slow, which is why
|
||||||
|
// it fires on an idle instance and against Gotenberg's own file:///tmp/ URLs.
|
||||||
|
// See https://github.com/gotenberg/gotenberg/issues/1659.
|
||||||
|
//
|
||||||
|
// Retrying separates the two cases. Catastrophic backtracking is
|
||||||
|
// deterministic: the same pattern against the same string exhausts the same
|
||||||
|
// budget on every attempt, so a genuine runaway still aborts, and costs at
|
||||||
|
// most patternMatchAttempts ceilings to prove it. A clock-induced abort needs
|
||||||
|
// the process to lose the CPU inside one specific match, which the next
|
||||||
|
// attempt does not reproduce.
|
||||||
|
//
|
||||||
|
// Elapsed time cannot make that call instead. A match frozen mid-flight
|
||||||
|
// reports the freeze as its own cost, 806ms against a 250ms ceiling in one
|
||||||
|
// measured run, so it is indistinguishable by wall clock from a match that
|
||||||
|
// really did spend its budget. Go exposes no per-goroutine CPU time, and
|
||||||
|
// process CPU time counts every other request in flight.
|
||||||
|
func MatchPattern(pattern *regexp2.Regexp, s string) (bool, error) {
|
||||||
|
var err error
|
||||||
|
|
||||||
|
for range patternMatchAttempts {
|
||||||
|
var ok bool
|
||||||
|
|
||||||
|
ok, err = pattern.MatchString(s)
|
||||||
|
if err == nil {
|
||||||
|
return ok, nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
99
pkg/gotenberg/pattern_test.go
Normal file
99
pkg/gotenberg/pattern_test.go
Normal file
@@ -0,0 +1,99 @@
|
|||||||
|
package gotenberg
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/dlclark/regexp2"
|
||||||
|
)
|
||||||
|
|
||||||
|
// mustPattern compiles a pattern the way the production lists are built.
|
||||||
|
func mustPattern(t *testing.T, expr string) *regexp2.Regexp {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
re := regexp2.MustCompile(expr, 0)
|
||||||
|
re.MatchTimeout = PatternMatchTimeout
|
||||||
|
|
||||||
|
return re
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMatchPattern(t *testing.T) {
|
||||||
|
// The abort [MatchPattern] absorbs cannot be staged here: it needs the
|
||||||
|
// whole process to lose the CPU around one specific match, which no test
|
||||||
|
// can schedule. What is testable is the other half of the contract, that
|
||||||
|
// retrying never turns a genuine runaway into a pass.
|
||||||
|
// See https://github.com/gotenberg/gotenberg/issues/1659.
|
||||||
|
for _, tc := range []struct {
|
||||||
|
scenario string
|
||||||
|
pattern *regexp2.Regexp
|
||||||
|
s string
|
||||||
|
expectMatch bool
|
||||||
|
expectError bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
scenario: "deny-list match",
|
||||||
|
pattern: mustPattern(t, `^file:(?!//\/tmp/).*`),
|
||||||
|
s: "file:///etc/passwd",
|
||||||
|
expectMatch: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
scenario: "no match against Gotenberg's own working directory",
|
||||||
|
pattern: mustPattern(t, `^file:(?!//\/tmp/).*`),
|
||||||
|
s: "file:///tmp/1a2b3c4d/5e6f7a8b/9c0d1e2f.html",
|
||||||
|
expectMatch: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
scenario: "no match",
|
||||||
|
pattern: mustPattern(t, `^https://example\.com/`),
|
||||||
|
s: "https://example.org/",
|
||||||
|
expectMatch: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
scenario: "catastrophic backtracking still aborts",
|
||||||
|
pattern: mustPattern(t, `^https://example\.com/(a+)+$`),
|
||||||
|
s: "https://example.com/" + strings.Repeat("a", 40) + "!",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(tc.scenario, func(t *testing.T) {
|
||||||
|
ok, err := MatchPattern(tc.pattern, tc.s)
|
||||||
|
|
||||||
|
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 ok != tc.expectMatch {
|
||||||
|
t.Fatalf("expected match %t but got %t", tc.expectMatch, ok)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMatchPatternBoundsCatastrophicPattern(t *testing.T) {
|
||||||
|
// Proving a runaway is genuine costs one ceiling per attempt, so the
|
||||||
|
// worst case is patternMatchAttempts of them plus regexp2's clock period
|
||||||
|
// on each, roughly a second. The bound that matters is the one this
|
||||||
|
// replaced: before the ceiling existed, the same match was allowed to
|
||||||
|
// burn a core for the caller's whole 30s budget.
|
||||||
|
pattern := mustPattern(t, `^https://example\.com/(a+)+$`)
|
||||||
|
s := "https://example.com/" + strings.Repeat("a", 40) + "!"
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
_, err := MatchPattern(pattern, s)
|
||||||
|
elapsed := time.Since(start)
|
||||||
|
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected an error from a catastrophic pattern")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Generous headroom over the expected second keeps this stable on a
|
||||||
|
// loaded CI box while still failing if the bound is gone.
|
||||||
|
if elapsed > 5*time.Second {
|
||||||
|
t.Fatalf("match took %s, want at most %d ceilings of %s", elapsed, patternMatchAttempts, PatternMatchTimeout)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -206,7 +206,7 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio
|
|||||||
}
|
}
|
||||||
|
|
||||||
matchStart := time.Now()
|
matchStart := time.Now()
|
||||||
ok, err := header.Scope.MatchString(e.Request.URL)
|
ok, err := gotenberg.MatchPattern(header.Scope, e.Request.URL)
|
||||||
budget.consume(time.Since(matchStart))
|
budget.consume(time.Since(matchStart))
|
||||||
|
|
||||||
switch {
|
switch {
|
||||||
|
|||||||
@@ -7,6 +7,8 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/dlclark/regexp2"
|
"github.com/dlclark/regexp2"
|
||||||
|
|
||||||
|
"github.com/gotenberg/gotenberg/v8/pkg/gotenberg"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestScopeMatchBudget(t *testing.T) {
|
func TestScopeMatchBudget(t *testing.T) {
|
||||||
@@ -90,7 +92,7 @@ func TestScopeMatchBudget_BoundsCatastrophicBacktracking(t *testing.T) {
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
matchStart := time.Now()
|
matchStart := time.Now()
|
||||||
_, _ = pattern.MatchString(url)
|
_, _ = gotenberg.MatchPattern(pattern, url)
|
||||||
b.consume(time.Since(matchStart))
|
b.consume(time.Since(matchStart))
|
||||||
matched++
|
matched++
|
||||||
}
|
}
|
||||||
@@ -100,10 +102,13 @@ func TestScopeMatchBudget_BoundsCatastrophicBacktracking(t *testing.T) {
|
|||||||
t.Errorf("all %d headers were matched, want the budget to stop matching early", headers)
|
t.Errorf("all %d headers were matched, want the budget to stop matching early", headers)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Each match is separately capped at extraHttpHeaderScopeMatchTimeout, so
|
// A match that is genuinely out of budget costs a few
|
||||||
// the worst case is the budget plus one final match that started with the
|
// extraHttpHeaderScopeMatchTimeout rather than one:
|
||||||
// last of the credit. Generous slack keeps this stable on a loaded CI box.
|
// [gotenberg.MatchPattern] retries an abort to tell a real runaway from
|
||||||
ceiling := budget + extraHttpHeaderScopeMatchTimeout + time.Second
|
// one caused by the process losing the CPU. 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 + 4*extraHttpHeaderScopeMatchTimeout + time.Second
|
||||||
if elapsed > ceiling {
|
if elapsed > ceiling {
|
||||||
t.Errorf("matching took %s, want at most %s", elapsed, ceiling)
|
t.Errorf("matching took %s, want at most %s", elapsed, ceiling)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user