Skip to content

refactor: create 941170 .ra file#4493

Merged
theseion merged 5 commits into
mainfrom
regex-assembly/941170
Jun 3, 2026
Merged

refactor: create 941170 .ra file#4493
theseion merged 5 commits into
mainfrom
regex-assembly/941170

Conversation

@fzipi

@fzipi fzipi commented Mar 1, 2026

Copy link
Copy Markdown
Member

what

why

  • maintainability: .ra files make patterns easier to review and update
  • [\s\S] expands to [\x00-\x{10ffff}], which fails toolchain validation (crs-toolchain#300) and PCRE 8-bit compilation in modsec2-apache. . is dotall-equivalent in our engines: verified . matches newlines on both modsec2-apache and modsec3-nginx via direct payload probe
  • the moz-binding [^-]*?- quantifier causes PCRE backtracking on benign inputs (1.4–2.5x regression, introduced in fix 941170 #2292) — see perf: rule 941170 moz-binding branch regex slower than v3.3 (1.4–2.5× PCRE regression) #4633

refs

ai disclosure

  • tools used: Claude (Opus 4.8)
  • assisted with: [\s\S]. conversion across the .ra, applying the moz-binding perf revert, drafting the moz-binding regression test, regenerating via crs-toolchain, this PR description
  • review performed: ran the full 941170 go-ftw suite on both modsec2-apache and modsec3-nginx (all pass), empirically verified . matches newlines (dotall) on both engines via direct payload probe, confirmed crs-toolchain regex generate/compare output, verified the generated rule contains no [\x00-\x{10ffff}] wide escapes

@fzipi
fzipi requested a review from a team March 1, 2026 20:21
@github-actions

github-actions Bot commented Mar 1, 2026

Copy link
Copy Markdown
Contributor

📊 Quantitative test results for language: eng, year: 2023, size: 10K, paranoia level: 1:
🚀 Quantitative testing did not detect new false positives

@fzipi

fzipi commented Mar 1, 2026

Copy link
Copy Markdown
Member Author

blocked by coreruleset/crs-toolchain#290 — the toolchain expands [\s\S] to [\x00-\x{10ffff}] which breaks PCRE in ModSecurity v2.

@EsadCetiner

Copy link
Copy Markdown
Member

@fzipi Have the blockers been resolved for this PR, can you pick it up and fix the tests?

I'll try and get this merged.

@fzipi

fzipi commented May 10, 2026

Copy link
Copy Markdown
Member Author

I'll take a look tomorrow.

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

📊 Quantitative test results for language: eng, year: 2023, size: 10K, paranoia level: 1:
🚀 Quantitative testing did not detect new false positives

fzipi added 3 commits June 2, 2026 13:05
replace [\s\S] with . in the .ra so crs-toolchain can generate a
PCRE-compatible pattern; [\s\S] expanded to [\x00-\x{10ffff}], which
fails toolchain validation and PCRE 8-bit compilation. . is
dotall-equivalent under modsec2-apache and modsec3-nginx (verified:
. matches newlines in both engines).

also revert the moz-binding branch quantifier [^-]*?- back to \W*?-,
the PCRE backtracking regression introduced in #2292 (see #4633), and
add test 6 covering the moz-binding branch, which had no coverage.
@fzipi
fzipi force-pushed the regex-assembly/941170 branch from f4c8ef9 to 377e388 Compare June 2, 2026 11:05
Comment thread regex-assembly/941170.ra Outdated
@fzipi
fzipi requested a review from theseion June 2, 2026 17:29
@theseion
theseion added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 721e85c Jun 3, 2026
8 checks passed
@theseion
theseion deleted the regex-assembly/941170 branch June 3, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants