Skip to content

feat: validate regular expressions#300

Merged
fzipi merged 2 commits into
mainfrom
validate-regex
Apr 7, 2026
Merged

feat: validate regular expressions#300
fzipi merged 2 commits into
mainfrom
validate-regex

Conversation

@theseion

@theseion theseion commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

We don't have full control over what rassemble-go and regexp generate. This can lead to unexpected situations, where valid input regex becomes invalid. For example [\s\S]+ will be expanded to [\x00-\x{10FFFF}] by regexp, but ModSecurity can only handle single byte escape codes (up to \x{ff}).

This change introduces validation logic for regular expressions after initial parsing and after final optimization.

Currently, two validations have been implemented:

  • valid Unicode hex escapes (for ModSecurity (PCRE 8-bit))
  • no multi-byte characters in character classes (in a character class without UTF-8 support enabled, a multibyte character is essentially a logical disjunction of the individual bytes, instead of a sequence, which can lead to unexpected results)

Fixes #290

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds post-parse and post-optimization validation for assembled regular expressions to prevent generating patterns that are incompatible with ModSecurity/PCRE 8-bit mode (e.g., rejecting large \x{...} code points and multi-byte literals inside character classes).

Changes:

  • Introduces a new regex/validation package with codepoint and character-class validation plus unit tests.
  • Hooks validation into the regex assembler (pre- and post-processing) and into crs-toolchain regex format.
  • Simplifies several scanners by removing redundant scanner.Split(bufio.ScanLines) calls and updates parser API/tests to reflect the new Parse signature.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
util/renumber_tests.go Removes redundant scanner split setup.
regex/validation/regex_validation.go Adds core regex validation logic and custom scanner split funcs.
regex/validation/regex_validation_test.go Adds unit tests for validation behavior.
regex/parser/parser.go Changes Parse API and adjusts write handling.
regex/parser/parser_test.go Updates parser tests for new Parse signature.
regex/parser/include_with_definition_test.go Updates parser tests for new Parse signature.
regex/parser/include_test.go Updates parser tests for new Parse signature.
regex/parser/include_multiple_test.go Updates parser tests for new Parse signature.
regex/parser/include_except_test.go Updates parser tests for new Parse signature.
regex/parser/include_except_builder.go Removes redundant scanner split setup.
regex/parser/definition_test.go Updates parser tests for new Parse signature.
regex/operators/assembler.go Adds pre/post validation around assembly and propagates final-pass errors.
regex/operators/assembler_test.go Adds regression tests for invalid escapes / validation failures.
cmd/regex/format/format.go Runs validation when formatting regex-assembly files.
chore/update_copyright/update_copyright.go Removes redundant scanner split setup.
go.mod Updates indirect dependencies.
go.sum Updates dependency checksums.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread regex/validation/regex_validation.go
Comment thread regex/validation/regex_validation.go
Comment thread regex/validation/regex_validation.go
Comment thread regex/validation/regex_validation.go Outdated
Comment thread regex/validation/regex_validation.go
Comment thread regex/parser/parser.go Outdated
Comment thread regex/parser/parser.go Outdated
Comment thread regex/operators/assembler_test.go Outdated
@fzipi

fzipi commented Apr 4, 2026

Copy link
Copy Markdown
Member

Maybe creating a new type interface for validation and then register the validations into a list, one list for pre, another for post. That way you just perform runValidation by doing a range on the corresponding list and you don't need to write each call specifically, just register in the proper list?

@theseion
theseion force-pushed the validate-regex branch 2 times, most recently from d48d807 to 4215177 Compare April 5, 2026 07:21
@theseion

theseion commented Apr 5, 2026

Copy link
Copy Markdown
Contributor Author

I've added a ValidateAll() function instead, since at the moment, all validations are run regardless. The pre / post was only used because of the interface.

We don't have full control over what rassemble-go and regexp generate.
This can lead to unexpected situations, where valid input regex becomes
invalid. For example `[\s\S]+` will be expanded to `[\x00-\x{10FFFF}]`
by regexp, but ModSecurity can only handle single byte escape codes (up
to `\x{ff}`).

This change introduces validation logic for regular expressions after
initial parsing and after final optimization.

Currently, two validations have been implemented:
- valid Unicode hex escapes (for ModSecurity (PCRE 8-bit))
- no multi-byte characters in character classes (in a character class
  without UTF-8 support enabled, a multibyte character is essentially a
  logical disjunction of the individual bytes, instead of a sequence,
  which can lead to unexpected results)

Fixes #290
Comment thread regex/operators/assembler_test.go Outdated
Comment thread regex/operators/assembler_test.go
Comment thread regex/validation/regex_validation_test.go
@theseion
theseion requested a review from fzipi April 7, 2026 05:32
@fzipi
fzipi merged commit 947ac87 into main Apr 7, 2026
3 checks passed
@fzipi
fzipi deleted the validate-regex branch April 7, 2026 15:07
@theseion theseion added the enhancement New feature or request label Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crs-toolchain expands [\s\S] to [\x00-\x{10ffff}] breaking PCRE/ModSecurity v2

3 participants