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

Better discord webhook url regex #208

Closed
wants to merge 8 commits into from
Closed

Better discord webhook url regex #208

wants to merge 8 commits into from

Conversation

IlluminatiFish
Copy link
Contributor

@IlluminatiFish IlluminatiFish commented Oct 11, 2021

Prerequisites

Why do we need this pull request?

Better webhook url regex, to allow other subdomains such as ptb & canary which are actively used as well as allowing for the detection of webhooks using the old domain discordapp.com and allowing for the matching of webhook urls that include the api version after the api path and more distinct expressions for webhook tokens and webhook ids, also allowing for webhook urls that have the scheme of http

What GitHub issues does this fix?

Expands upon changes made in #173

Copy / paste of output

Couldn't get the script to work so I could not test the changes to the regex i've made with the regex however I have tested it on its own in python 3.10 as well as using https://pythex.org

Better webhook url regex, to allow other subdomains such as ptb & canary which are actively used as well as allowing for the detection of webhooks using the old domain discordapp.com and allowing for the matching of webhook urls that include the api version after the api path and more distinct expressions for webhook tokens and webhook ids, also allowing for webhook urls that have the scheme of http
@@ -832,7 +832,7 @@
},
{
"Name": "Discord Webhook",
"Regex": "(?i)^(https://discord\\.com/api/webhooks/[0-9]+/[A-Za-z0-9-_]+)$",
"Regex": "(?i)^(https?:\/\/(?:ptb\.|canary\.)?discord(?:app)?\.com\/api(?:\/v\d{1,2})?\/webhooks\/(\d{17,19})\/([\w\-]{68}))$",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we can get tests for ptb, canary and discordapp ? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing! :)

IlluminatiFish and others added 7 commits October 12, 2021 23:18
Added more discord webhook test candidates, to test the api version string and the new subdomains & domains (ptb, canary and discordapp)
Hopefully this escaped everything
Forgot to escape one more character class
More escaping of regex in the JSON file
Hopefully this is the final commit
Fix indent
"https://canary.discord.com/api/v9/webhooks/894893734582452235/KhNc2-_zwY9FfCAK0iGUa_KfYyW8m5Ja_5i-V24fEY6ETwvLLn-GmdT_vq0Do9-YRsij",
"https://discordapp.com/api/v9/webhooks/894893734582452235/KhNc2-_zwY9FfCAK0iGUa_KfYyW8m5Ja_5i-V24fEY6ETwvLLn-GmdT_vq0Do9-YRsij",
"https://ptb.discordapp.com/api/v9/webhooks/894893734582452235/KhNc2-_zwY9FfCAK0iGUa_KfYyW8m5Ja_5i-V24fEY6ETwvLLn-GmdT_vq0Do9-YRsij",
"https://canary.discordapp.com/api/v9/webhooks/894893734582452235/KhNc2-_zwY9FfCAK0iGUa_KfYyW8m5Ja_5i-V24fEY6ETwvLLn-GmdT_vq0Do9-YRsij"
]
)
_assert_match_first_item("Discord Webhook", res)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only testing that one of these URLs works, not that all of them work. Please make them into separate functions? :) <3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it!

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2021

Codecov Report

Merging #208 (81edf2c) into main (e8aaaaa) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #208   +/-   ##
=======================================
  Coverage   94.46%   94.46%           
=======================================
  Files          14       14           
  Lines        1717     1717           
=======================================
  Hits         1622     1622           
  Misses         95       95           
Impacted Files Coverage Δ
tests/test_regex_identifier.py 98.54% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8aaaaa...81edf2c. Read the comment docs.

IlluminatiFish added a commit to IlluminatiFish/pyWhat that referenced this pull request Oct 17, 2021
Added loads of test candidates and updated discord webhook regex as per bee-san#208
@IlluminatiFish
Copy link
Contributor Author

Moved to #216 due to merge conflicts with the new regex test system introduced via #202

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.

3 participants