feat(regex): add deadline-aware Regex::new_with_deadline API#25
Draft
youichi-uda wants to merge 1 commit intoquickwit-inc:masterfrom
Draft
feat(regex): add deadline-aware Regex::new_with_deadline API#25youichi-uda wants to merge 1 commit intoquickwit-inc:masterfrom
youichi-uda wants to merge 1 commit intoquickwit-inc:masterfrom
Conversation
Pathological regex patterns can take multiple seconds in the NFA-to-DFA
expansion before the existing STATE_LIMIT/CompiledTooBig guards reject
them. Existing limits bound *memory* but not *wall-clock time*. For
example:
\w{0,5}\w{0,5}\w{0,5}\w{0,5} -> 2.4s before TooManyStates(1000)
\w{0,4}\w{0,4}\w{0,4}\w{0,4}\w{0,4}\w{0,4} -> 3.8s
\w{0,3} repeated 8 times -> 5.0s
For services that compile user-supplied regex patterns on a request
thread (full-text search _search endpoints, log query DSLs, etc), this
is a low-payload DoS vector that caller-side caps cannot fully fix
because the expensive work happens *inside* Regex::new before any
caller cap can intervene.
This adds Regex::new_with_deadline(re, deadline) that takes an
Instant deadline and aborts DFA construction with a new
Error::DeadlineExceeded variant. The check is batched (every 1024
transitions) so overhead on well-behaved patterns is well under 1%.
Backwards-compatible: existing Regex::new is unchanged. The internal
DfaBuilder gains build_with_deadline(Option<Instant>) and the original
build() now delegates with None.
Measurements (release build, x86_64 Linux):
Easy patterns (n=1000):
"hello.*world" no-deadline 238 us with-deadline 243 us (+2%)
"[a-zA-Z0-9]+" no-deadline 10 us with-deadline 10 us (~0%)
"(foo|bar|baz|qux)+" no-deadline 20 us with-deadline 20 us (~0%)
Pathological pattern \w{0,5}\w{0,5}\w{0,5}\w{0,5}:
Regex::new -> 2447ms / TooManyStates
new_with_deadline (50ms deadline) -> 55ms / DeadlineExceeded
new_with_deadline (250ms deadline) -> 253ms / DeadlineExceeded
Tests added in src/regex/mod.rs:
- deadline_aborts_pathological_pattern: 50ms deadline aborts
SLOW_PATTERN well under the 2.4s baseline
- deadline_does_not_affect_easy_pattern: long deadline + small
pattern compiles cleanly
- new_unchanged_for_easy_pattern: backwards-compat smoke
- deadline_exceeded_error_is_distinct: Display/Debug formatting
Full suite: 141 passed, 5 ignored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Pathological regex patterns can take multiple seconds in the NFA-to-DFA expansion before the existing
STATE_LIMIT/CompiledTooBigguards reject them. The existing limits bound memory but not wall-clock time. For example, on a release build (x86_64 Linux):\w{0,5}\w{0,5}\w{0,5}\w{0,5}Err(TooManyStates(1000))\w{0,4}\w{0,4}\w{0,4}\w{0,4}\w{0,4}\w{0,4}Err(TooManyStates(1000))\w{0,3}× 8Err(TooManyStates(1000))For services that compile user-supplied regex patterns on a request thread (full-text search
_searchendpoints, log query DSLs, etc.), this is a low-payload DoS vector that caller-side caps cannot fully fix, because the expensive work happens insideRegex::newbefore any caller cap can intervene.We hit this in ferrosearch three times in three days through fuzzing of a
_searchregex DSL endpoint. Each time we tightened a caller-side input cap (commitsabbf6c5,ae0a3d4,207841c) the next fuzz wave found a new escape vector — pattern length, alternation count, and nesting depth are all loosely correlated with build time, so input-shape caps are an arms race. A wall-clock deadline is the only durable fix.API design
Design notes:
Regex::new,with_size_limit, the publicErrorenum (additive), and theAutomatonimpl are unchanged for existing callers. The only API surface added isRegex::new_with_deadlineandError::DeadlineExceeded.Instantdeadline (notDuration): lets callers compose with an outer request deadline (req_deadline.min(per_query_budget)) without bookkeeping. Matches the convention used bytokio::time::timeout_at,std::sync::Condvar::wait_timeout_until, etc.DfaBuilder::build()now delegates tobuild_with_deadline(None), which is where the new check lives.DEADLINE_CHECK_INTERVAL = 1024transitions. This is the hot loop where theSTATE_LIMITguard already lives, and is by far the dominant cost on adversarial patterns.I considered alternatives:
Compiler::c(NFA build) — the NFA build is mostly linear in HIR size and already bounded byCompiledTooBig. Empirically the trap is in DFA expansion, not compile, so adding it there would be dead code. Happy to add a symmetric check there if reviewers prefer.with_deadlinebuilder pattern — would let us also expose the size limit (currently hardcoded to10 << 20). Out of scope for this PR but a natural follow-up; this PR keeps the new API minimal so it can be reviewed and shipped quickly.Backwards compatibility
Regex::newis unchanged (delegates throughwith_size_limit_and_deadline(size, re, None)).Erroris#[derive(Debug)]only, no#[non_exhaustive]. Adding a variant to a public non-#[non_exhaustive]enum is technically a minor breaking change for downstreammatches without_arms. Given (a) tantivy-fst's README explicitly steers new users to the upstreamfstcrate, (b)Errorispub enumwithout exhaustiveness guarantees in any docs, and (c) the practical alternative is leaving callers vulnerable to the DoS, I think this is the right tradeoff. If reviewers prefer, I can add#[non_exhaustive]toErrorin the same release as a hardening pass.Performance measurement
Microbench (1000 iterations, release build, x86_64 Linux):
Regex::newnew_with_deadline(60s)hello.*world[a-zA-Z0-9]+(foo|bar|baz|qux)+Pathological pattern
\w{0,5}\w{0,5}\w{0,5}\w{0,5}:Regex::new(...)Err(TooManyStates(1000))Regex::new_with_deadline(50 ms)Err(DeadlineExceeded)Regex::new_with_deadline(250 ms)Err(DeadlineExceeded)44× latency reduction on the adversarial input; deadline overshoot is ~5 ms (one check interval).
Test results
Added 4 unit tests in
src/regex/mod.rs:deadline_aborts_pathological_pattern: 50 ms deadline aborts the slow pattern in well under the 2.4 s baseline.deadline_does_not_affect_easy_pattern: long deadline + small pattern compiles cleanly.new_unchanged_for_easy_pattern: backwards-compat smoke for the existing API.deadline_exceeded_error_is_distinct:Display/Debugformatting for the new variant.Full suite:
No existing tests modified.
Defense in depth
The 3 caller-side input-shape caps in ferrosearch will remain even after this lands. The deadline is the floor; caller caps still serve as cheap early-rejection so we don't even hand obvious-garbage to the regex engine. I think both layers are valuable and the deadline is the durable one.
Filed as draft for design review on the API shape (especially the
non_exhaustivequestion and whether to expose a builder). Happy to iterate.