-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix: is_fully_escaped does not handle consecutive backslashes correctly #14393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
5ba5467
ed7bbc5
046455f
4e88fdf
77f91ac
59ee7a0
66b18b2
7570db4
5b839bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Fixed ``is_fully_escaped`` not handling consecutive backslashes correctly: an escaped backslash before a metacharacter (e.g. ``\\\\.``) was incorrectly treated as escaping the metacharacter itself, causing ``pytest.raises(match=...)`` to skip the regex diff display when it should have shown one. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -345,9 +345,20 @@ def _check_raw_type( | |
| def is_fully_escaped(s: str) -> bool: | ||
| # we know we won't compile with re.VERBOSE, so whitespace doesn't need to be escaped | ||
| metacharacters = "{}()+.*?^$[]|" | ||
| 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): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use a regex for it instead? I'm thinking we can do it in 2 steps:
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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the review! Updated the changelog and refactored is_fully_escaped to use the regex approach — it's cleaner this way. |
||
| if c in metacharacters: | ||
| # Count consecutive backslashes preceding this metacharacter. | ||
| # An odd number of backslashes means the metacharacter is escaped | ||
| # (the last backslash does the escaping); an even number means | ||
| # it is not escaped (backslashes escape each other in pairs). | ||
| n_backslashes = 0 | ||
| j = i - 1 | ||
| while j >= 0 and s[j] == "\\": | ||
| n_backslashes += 1 | ||
| j -= 1 | ||
| if n_backslashes % 2 == 0: | ||
| return False | ||
| return True | ||
|
|
||
|
|
||
| def unescape(s: str) -> str: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.