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

util: try to fix link regex #1803

Merged
merged 1 commit into from
Dec 24, 2024
Merged

Conversation

jo3-l
Copy link
Contributor

@jo3-l jo3-l commented Dec 24, 2024

#1796 changed the link regex to catch URL-encoded links such as https://%20@yagpdb.xyz. Unfortunately, it caused a regression where text after the URL is matched. The new URL regex matches on the entirety of this text, including the second line:

https://yagpdb.xyz
and this is matched too!

We try to tune the regex so that it catches URL-encoded characters while avoiding the regression. The regex before #1796, after #1796, and after this PR is:

before #1796:    (?i)([a-z\d]+://)([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])
after #1796:     (?i)([a-z\d]+://)([\w\W_-]+(?:(?:\.[\w\W_-]+)+))([\w\W.,@?^=%&:/~+#-]*[\w\W@?^=%&/~+#-])
                                      ^^ added         ^^ added      ^^ added             ^^ added
this PR:         (?i)([a-z\d]+://)([\w-._~:/?#\[\]@!$&'()*+,;%=]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^ changed   ^^ reverted ^^ reverted        ^^ reverted

Explicitly, the problem from #1796 is that \w and \W together match on every character. We revert back to the old regex, but expand the character set in the second capturing group per https://stackoverflow.com/a/1547940 so that it still matches https://%20@yagpdb.xyz.

@ashishjh-bst ashishjh-bst merged commit bfc400f into botlabs-gg:dev Dec 24, 2024
1 check passed
ashishjh-bst pushed a commit to ashishjh-bst/yagpdb that referenced this pull request Dec 31, 2024
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.

2 participants