fix: preserve [\s\S] instead of expanding to [\x00-\x{10ffff}] for PCRE compatibility#291
[\s\S] instead of expanding to [\x00-\x{10ffff}] for PCRE compatibility#291Conversation
…ibility Co-authored-by: fzipi <[email protected]>
[\s\S] instead of expanding to [\x00-\x{10ffff}] for PCRE compatibility
…ticcheck QF1012 Co-authored-by: fzipi <[email protected]>
Fixed in 2ed7fa7 — replaced both |
fzipi
changed the title
[\s\S] instead of expanding to [\x00-\x{10ffff}] for PCRE compatibility[\s\S] instead of expanding to [\x00-\x{10ffff}] for PCRE compatibility
There was a problem hiding this comment.
Pull request overview
Adjusts the regex assembly cleanup pipeline to preserve the cross-engine “match any char including newline” idiom [\s\S], avoiding PCRE non-UTF compilation failures and preventing silent newline-matching regressions.
Changes:
- Add a post-processing step to normalize rassemble-go outputs like
[\x00-\x{10ffff}]and(?s:.)back to[\s\S]before flag stripping. - Extend/update assembler tests to cover standalone, quantified, and embedded
[\s\S]cases (and update an existing flag-group test expectation). - Refactor
useHexEscapesto usefmt.Fprintfinstead ofsb.WriteString(fmt.Sprintf(...))(staticcheck QF1012).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| regex/operators/assembler.go | Adds replaceAnyCharClass cleanup step and refactors hex-escape writing. |
| regex/operators/assembler_test.go | Adds coverage for [\s\S] preservation and updates expected output where newline matching must be retained. |
| go.mod | Tidies indirect requirements. |
| go.sum | Updates dependency checksums consistent with module tidy/update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
Where did you get this? The toolchain usually generates |
|
This only happens because of the quantifier after the character class. The quick fix here is obviously to use |
|
Replaced by #300 |
[\s\S]in.rafiles was being expanded to[\x00-\x{10ffff}]by rassemble-go, which is valid RE2 but illegal in PCRE non-UTF mode (ModSecurity v2/Apache).what
replaceAnyCharClasspost-processing step inassembler.gothat normalizes rassemble-go's two output forms back to[\s\S]:[\x00-\x{10ffff}]→[\s\S](produced for quantified patterns, e.g.[\s\S]+)(?s:.)→[\s\S](produced for a standalone[\s\S]; previously became.after flag stripping, silently dropping newline matching)dontUseFlagsForMetaCharactersin the cleanup pipelinesb.WriteString(fmt.Sprintf(...))withfmt.Fprintf(&sb, ...)inuseHexEscapesto satisfy staticcheck QF1012why
[\x00-\x{10ffff}]uses\x{...}wide-char syntax which PCRE rejects in non-UTF mode, causing Apache/ModSecurity v2 to fail on rule compilation[\s\S]→(?s:.)→.path was also semantically wrong:.without thesflag does not match newlines[\s\S]is valid in both PCRE and RE2 and is the idiomatic cross-engine "any char including newline" classreferences
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.