Skip to content

Better discord webhook url regex#208

Closed
IlluminatiFish wants to merge 8 commits intobee-san:mainfrom
IlluminatiFish:main
Closed

Better discord webhook url regex#208
IlluminatiFish wants to merge 8 commits intobee-san:mainfrom
IlluminatiFish:main

Conversation

@IlluminatiFish
Copy link
Copy Markdown
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
{
"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
Copy Markdown
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
Copy Markdown
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.discordapp.com/api/v9/webhooks/894893734582452235/KhNc2-_zwY9FfCAK0iGUa_KfYyW8m5Ja_5i-V24fEY6ETwvLLn-GmdT_vq0Do9-YRsij"
]
)
_assert_match_first_item("Discord Webhook", res)
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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