Fix: is_fully_escaped does not handle consecutive backslashes correctly#14393
Conversation
The function only checked if the character immediately before a regex metacharacter was a backslash, but did not count how many consecutive backslashes preceded it. When two backslashes appear before a metacharacter (e.g. r'\\.'), the first escapes the second, leaving the metacharacter unescaped. The function incorrectly reported such strings as fully escaped. Fix by counting consecutive backslashes: an even count means the metacharacter is not escaped, an odd count means it is. Closes pytest-dev#14392
r'\\\\|' is 4 backslashes + pipe. Even count means pipe is not escaped, so is_fully_escaped should return False, not True.
4 backslashes + pipe: even count means pipe is not escaped, so is_fully_escaped should return False, not True.
alokshukla631
left a comment
There was a problem hiding this comment.
This looks good to me. Counting the run of preceding backslashes matches regex escaping rules much better than the old single-character check, and the added cases make the intended behavior easy to follow.
|
@alokshukla631 |
| return not any( | ||
| c in metacharacters and (i == 0 or s[i - 1] != "\\") for (i, c) in enumerate(s) | ||
| ) | ||
| for i, c in enumerate(s): |
There was a problem hiding this comment.
Can we use a regex for it instead? I'm thinking we can do it in 2 steps:
- Strip all escapes (
re.sub(r'\\.', '', pattern)) - Check if still contains any metacharacters (simple containment check).
Step 1 should ensure that none of the chars found in step 2 are escaped.
I think that would be shorter and a bit more obvious.
There was a problem hiding this comment.
Thanks for the review! Updated the changelog and refactored is_fully_escaped to use the regex approach — it's cleaner this way.
Co-authored-by: Ran Benita <ran@unusedvar.com>
…//github.com/EternalRights/pytest into fix-is-fully-escaped-consecutive-backslashes
Backport to 9.0.x: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply a38b20d on top of patchback/backports/9.0.x/a38b20d79039d50c504320b5d4d790c61893a90e/pr-14393 Backporting merged PR #14393 into main
🤖 @patchback |
Closes #14392
Description
The
is_fully_escapedfunction inraises.pyonly checked whether the character immediately before a regex metacharacter was a backslash. It did not count how many consecutive backslashes preceded the metacharacter.In regex, backslashes escape each other in pairs. So
\\.(two backslashes then a dot) means an escaped backslash followed by an unescaped dot. Butis_fully_escaped(r\\.)was incorrectly returningTruebecause it only checked ifs[i-1]was\.This affects
pytest.raises(match=...)error messages: when the match pattern contains escaped backslashes before a metacharacter, the function incorrectly considers the pattern fully escaped and skips showing a regex diff on match failure.Fix: Count consecutive backslashes before each metacharacter. Even count means the metacharacter is not escaped; odd count means it is.
Type of Change
Checklist