Skip to content

FIX: Validate replacement templates in SearchReplaceConverter - #4079

Open
电大老baby (HhH666-ux) wants to merge 1 commit into
microsoft:mainfrom
HhH666-ux:fix-search-replace-template-validation
Open

电大老baby (HhH666-ux) wants to merge 1 commit into
microsoft:mainfrom
HhH666-ux:fix-search-replace-template-validation

Conversation

@HhH666-ux

Copy link
Copy Markdown

Description

SearchReplaceConverter.__init__ validated the compiled regex pattern but not the templates in replace. Invalid templates (bad escapes, references to missing groups, unknown named groups) only failed later inside convert_async with an unhandled re.error or IndexError.

  • Validate every template in replace against the compiled pattern during construction and raise a descriptive ValueError.
  • Add regression tests for an invalid escape, a missing group reference, and an unknown named group.

Fixes #4076

Copilot AI balanced review requested due to automatic review settings November 9, 2026 03:58

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.

🟢 Approval recommended

No active review issues are reported for the change that validates replacement templates during converter initialization.

No open issues 🎉

What changed in this PR

This PR addresses #4076 by moving replacement-template validation into SearchReplaceConverter construction, so invalid configurations raise a descriptive ValueError before conversion runs.

Changes:

  • Validate every replacement template against the compiled regex during initialization.
  • Document the new validation and add tests for invalid escapes, missing groups, unknown named groups, and a valid named-group reference.
File Description
tests/​unit/​converter/​test_search_replace_converter.py Adds invalid-template regression tests and a valid-template acceptance test.
pyrit/​converter/​search_replace_converter.py Validates replacement templates during construction and documents the resulting errors.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: SearchReplaceConverter constructor does not validate replacement templates, causing unhandled runtime crash

2 participants