-
Notifications
You must be signed in to change notification settings - Fork 538
limit link schemes in in messages in e-mail push notifications #19741
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
base: develop
Are you sure you want to change the base?
Changes from all commits
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 @@ | ||
| Limit the allowed link schemes to `["http", "https", "mailto"]` from messages contained in e-mail push notifications. Contributed by Noah Markert. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -112,7 +112,7 @@ | |||||
| "img": ["src"], | ||||||
| } | ||||||
| # When bleach release a version with this option, we can specify schemes | ||||||
| # ALLOWED_SCHEMES = ["http", "https", "ftp", "mailto"] | ||||||
| ALLOWED_SCHEMES = ["http", "https", "mailto"] | ||||||
|
Comment on lines
114
to
+115
Contributor
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. Depending on the answer to #19741 (comment) we can at-least use this PR remove this entire comment (and the commented out usage). |
||||||
|
|
||||||
|
|
||||||
| class Mailer: | ||||||
|
|
@@ -972,7 +972,7 @@ def safe_markup(raw_html: str) -> Markup: | |||||
| tags=ALLOWED_TAGS, | ||||||
| attributes=ALLOWED_ATTRS, | ||||||
| # bleach master has this, but it isn't released yet | ||||||
| # protocols=ALLOWED_SCHEMES, | ||||||
| protocols=ALLOWED_SCHEMES, | ||||||
|
Contributor
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.
A test would be good. Have you at least tested this manually? Please provide some instructions
Contributor
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. Do we even need to define this?
And I can see it in their code: https://github.com/mozilla/bleach/blob/913ab75992b845e2c9c060c41f24d46921db4693/bleach/sanitizer.py#L37-L38
Contributor
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. To make it more clear, the minimum version of We're now using Lines 75 to 76 in 8eb220a
|
||||||
| strip=True, | ||||||
| ) | ||||||
| ) | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment is no longer relevant since we're using a version of
bleachthat supports this