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

Conversation

SWAGATSWAROOP
Copy link

@SWAGATSWAROOP SWAGATSWAROOP commented Jun 17, 2023

Pull Request Checklist

Uncommented the line of allowed protocols from safe_markup function and Allowed protocols as support is added in bleach library.

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

@SWAGATSWAROOP SWAGATSWAROOP requested a review from a team as a code owner June 17, 2023 19:54
@SWAGATSWAROOP SWAGATSWAROOP reopened this Jun 17, 2023
@SWAGATSWAROOP
Copy link
Author

Sorry Mistakenly Closed

@SWAGATSWAROOP
Copy link
Author

sign off : SWAGAT SWAROOP PARIDA swagatswaroop@gmail.com

This is my first open source contribution. If there are mistakes point out It will help me learn and grow with community.

@@ -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,
Copy link
Contributor

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

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

@SWAGATSWAROOP SWAGATSWAROOP deleted the Swagat branch July 3, 2023 15:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants