Update header_rewrite to use Regex (#12573)#12787
Merged
cmcfarlen merged 1 commit intoapache:10.1.xfrom Jan 14, 2026
Merged
Conversation
* Update header_rewrite to use Regex * PR review * cast to int * also move dbg for failed regex compile. (cherry picked from commit a15669b)
bryancall
approved these changes
Jan 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the header_rewrite plugin by replacing direct PCRE API usage with the new tsutil Regex wrapper class. This is a cherry-pick from commit #12573 to fix issue #12769.
Changes:
- Replaced PCRE-specific data structures (ovector, pcre pointers) with the new Regex and RegexMatches classes
- Updated regex compilation and matching to use the Regex API
- Removed direct PCRE library dependency from CMakeLists.txt
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugins/header_rewrite/resources.h | Replaced ovector-related members with RegexMatches object |
| plugins/header_rewrite/resources.cc | Removed ovector initialization code |
| plugins/header_rewrite/regex_helper.h | Updated to use Regex class instead of pcre pointers |
| plugins/header_rewrite/regex_helper.cc | Implemented regex operations using new Regex API |
| plugins/header_rewrite/matcher.h | Updated regex matching to use RegexMatches |
| plugins/header_rewrite/conditions.cc | Simplified capture group access using RegexMatches indexing |
| plugins/header_rewrite/CMakeLists.txt | Removed PCRE library dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1721
to
+1722
| if (res.matches.size() > _ix) { | ||
| s.append(res.matches[_ix]); |
There was a problem hiding this comment.
Comparing int32_t (size()) with int (_ix) can lead to signed comparison warnings. Since _ix is validated to be >= 0 in set_qualifier, consider casting _ix to size_t or int32_t for the comparison to avoid potential warnings.
Suggested change
| if (res.matches.size() > _ix) { | |
| s.append(res.matches[_ix]); | |
| if (res.matches.size() > static_cast<size_t>(_ix)) { | |
| s.append(res.matches[static_cast<size_t>(_ix)]); |
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.
This cherry-picks #12573 in order to fix #12769
Update header_rewrite to use Regex
PR review
cast to int
also move dbg for failed regex compile.
(cherry picked from commit a15669b)