Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Upgrade to Bleach 1.5, to limit link schemes to an allowlist #2860 #15793

Closed
wants to merge 2 commits into from
Closed

Upgrade to Bleach 1.5, to limit link schemes to an allowlist #2860 #15793

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f1f26f
Upgrade to Bleach 1.5, to limit link schemes to an allowlist #2860
SWAGATSWAROOP Jun 17, 2023
82439ff
Merge branch 'develop' into Swagat
SWAGATSWAROOP Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions synapse/push/mailer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,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", "ftp", "mailto"]


class Mailer:
Expand Down Expand Up @@ -890,7 +890,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,
SWAGATSWAROOP marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@MadLittleMods MadLittleMods Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We're using bleach == 6.0.0 in poetry.lock but it looks like there is a breaking change where it needs to be a Set now. As far as I can tell, we can't really support both versions before and after so would need to converge either way on our dependency versions

Copy link
Contributor

@DMRobertson DMRobertson Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's slightly alarming. I couldn't see a way in which this was obviously broken if you pass in a list, so not sure what's going on there.

$ python
Python 3.11.3 (main, May 24 2023, 00:00:00) [GCC 12.3.1 20230508 (Red Hat 12.3.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import bleach
>>> bleach.__version__
'6.0.0'
>>> bleach.clean()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: clean() missing 1 required positional argument: 'text'
>>> bleach.clean('aaaaa')
'aaaaa'
>>> bleach.clean('<naughty>')
'&lt;naughty&gt;'
>>> bleach.clean('<naughty><a>')
'&lt;naughty&gt;<a></a>'
>>> bleach.clean('<naughty><a href="foo">')
'&lt;naughty&gt;<a href="foo"></a>'
>>> bleach.clean('<naughty><a href="foo">', tags=['a'])
'&lt;naughty&gt;<a href="foo"></a>'
>>> bleach.clean('<naughty><a href="foo">', tags={'a'})
'&lt;naughty&gt;<a href="foo"></a>'
>>> bleach.clean('<naughty><a href="foo">', tags={'a', 'naughty'})
'<naughty><a href="foo"></a></naughty>'
>>> bleach.clean('<naughty><a href="foo">', tags={'naughty'})
'<naughty>&lt;a href="foo"&gt;</naughty>'
>>> bleach.clean('<naughty><a href="foo">', tags=['naughty'])
'<naughty>&lt;a href="foo"&gt;</naughty>'

Copy link
Contributor

Choose a reason for hiding this comment

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

Though note that bleach is deprecated: mozilla/bleach#698

strip=True,
)
)
Expand Down