Skip to content
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

Prevent attempts at nesting character sets in regex filter #389

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JoshuaS3
Copy link
Contributor

Not sure how I didn't catch this when testing the initial regex feature a couple years ago. Basically, nesting character sets is impossible in Python's regex, which is what my filter code tried to do because it wasn't checking for IN tokens, so those confusable glyphs would be discarded by the regex compiler (thankfully it doesn't outright throw an error). This PR adds that check and performs confusable insertion in place for those sets. Also cleans up some of the code in that area.

Example of the issue:
h[ea]lp would effectively be converted into this, which is invalid regex:

[hhℎ𝐡𝒉𝒽𝓱𝔥𝕙𝖍𝗁𝗵𝘩𝙝𝚑һհᏂ]**[**[e℮eℯⅇ𝐞𝑒𝒆𝓮𝔢𝕖𝖊𝖾𝗲𝘦𝙚𝚎ꬲеҽ][a⍺a𝐚𝑎𝒂𝒶𝓪𝔞𝕒𝖆𝖺𝗮𝘢𝙖𝚊ɑα𝛂𝛼𝜶𝝰𝞪а]**]**[l𝟏𝟙𝟣𝟭𝟷IIⅠℐℑ𝐈𝐼𝑰𝓘𝕀𝕴𝖨𝗜𝘐𝙄𝙸Ɩlⅼℓ𝐥𝑙𝒍𝓁𝓵𝔩𝕝𝖑𝗅𝗹𝘭𝙡𝚕ǀΙ𝚰𝛪𝜤𝝞𝞘ⲒІӀⵏᛁꓲ𖼨𐊊𐌉][p⍴p𝐩𝑝𝒑𝓅𝓹𝔭𝕡𝖕𝗉𝗽𝘱𝙥𝚙ρϱ𝛒𝛠𝜌𝜚𝝆𝝔𝞀𝞎𝞺𝟈ⲣр]

instead of this, which is valid and desired:

[hhℎ𝐡𝒉𝒽𝓱𝔥𝕙𝖍𝗁𝗵𝘩𝙝𝚑һհᏂ]**[**e℮eℯⅇ𝐞𝑒𝒆𝓮𝔢𝕖𝖊𝖾𝗲𝘦𝙚𝚎ꬲеҽa⍺a𝐚𝑎𝒂𝒶𝓪𝔞𝕒𝖆𝖺𝗮𝘢𝙖𝚊ɑα𝛂𝛼𝜶𝝰𝞪а**]**[l𝟏𝟙𝟣𝟭𝟷IIⅠℐℑ𝐈𝐼𝑰𝓘𝕀𝕴𝖨𝗜𝘐𝙄𝙸Ɩlⅼℓ𝐥𝑙𝒍𝓁𝓵𝔩𝕝𝖑𝗅𝗹𝘭𝙡𝚕ǀΙ𝚰𝛪𝜤𝝞𝞘ⲒІӀⵏᛁꓲ𖼨𐊊𐌉][p⍴p𝐩𝑝𝒑𝓅𝓹𝔭𝕡𝖕𝗉𝗽𝘱𝙥𝚙ρϱ𝛒𝛠𝜌𝜚𝝆𝝔𝞀𝞎𝞺𝟈ⲣр]

(** only added for emphasis)

Not sure how I didn't catch this when testing a couple years ago.
Basically, nesting character sets is impossible in Python's regex, which
is what my filter code tried to do because it wasn't checking for IN
tokens. This adds that check and performs confusable insertion in place
for those sets. Also cleans up some of the code in that area.
@JoshuaS3 JoshuaS3 requested a review from Ma-wa-re as a code owner March 17, 2023 19:13
Also missed this. "a-z" in [a-z] is tokenized as (RANGE, (97, 122)).
This commit adds support for those tokens.
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.

1 participant