Skip to content

Commit 48735be

Browse files
authored
Replace Bleach with JustHTML (#4489)
1 parent 8925113 commit 48735be

12 files changed

Lines changed: 129 additions & 54 deletions

File tree

pontoon/base/forms.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
from pathlib import Path
44

5-
import bleach
6-
75
from django import forms
86
from django.conf import settings
97
from django.core.exceptions import ValidationError
@@ -27,13 +25,7 @@ class HtmlField(forms.CharField):
2725

2826
def clean(self, value):
2927
value = super().clean(value)
30-
value = bleach.clean(
31-
value,
32-
strip=True,
33-
tags=settings.ALLOWED_TAGS,
34-
attributes=settings.ALLOWED_ATTRIBUTES,
35-
)
36-
return value
28+
return utils.sanitize_html(value)
3729

3830

3931
class NoTabStopCharField(forms.CharField):

pontoon/base/templatetags/helpers.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010

1111
from allauth.socialaccount.adapter import get_adapter
1212
from allauth.utils import get_request_param
13-
from bleach.linkifier import Linker
1413
from django_jinja import library
14+
from justhtml import JustHTML, Linkify, SetAttrs, Unwrap
1515

1616
from django import template
1717
from django.conf import settings
@@ -297,15 +297,16 @@ def as_plain_message(translation):
297297
def linkify(source):
298298
"""Render URLs in the string as links."""
299299

300-
def set_attrs(attrs, new=False):
301-
attrs[(None, "target")] = "_blank"
302-
attrs[(None, "rel")] = "noopener noreferrer"
303-
return attrs
304-
305-
# Escape all tags
306-
linker = Linker(callbacks=[set_attrs])
307-
308-
return linker.linkify(source)
300+
return JustHTML(
301+
source,
302+
fragment=True,
303+
sanitize=False,
304+
transforms=[
305+
Linkify(),
306+
Unwrap('a[href^="mailto:"]'),
307+
SetAttrs("a", target="_blank", rel="noopener noreferrer"),
308+
],
309+
).to_html(pretty=False)
309310

310311

311312
@library.filter

pontoon/base/tests/test_helpers.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
format_datetime,
1010
full_static,
1111
full_url,
12+
linkify,
1213
metric_prefix,
1314
nospam,
1415
to_json,
@@ -100,3 +101,39 @@ def test_user_editor_theme_anonymous_resolves_to_default():
100101
"""Anonymous users get the default editor theme."""
101102
anon = AnonymousUser()
102103
assert user_editor_theme(anon) == UserProfile.DEFAULT_EDITOR_THEME
104+
105+
106+
@pytest.mark.parametrize(
107+
"source,expected",
108+
(
109+
("no links here", "no links here"),
110+
(
111+
"See https://pontoon.mozilla.org/?a=1&b=2 now",
112+
"See "
113+
'<a href="https://pontoon.mozilla.org/?a=1&amp;b=2" target="_blank" '
114+
'rel="noopener noreferrer">https://pontoon.mozilla.org/?a=1&amp;b=2</a>'
115+
" now",
116+
),
117+
(
118+
"Go to www.mozilla.org.",
119+
'Go to <a href="http://www.mozilla.org" target="_blank" '
120+
'rel="noopener noreferrer">www.mozilla.org</a>.',
121+
),
122+
# Existing links are left alone, but get the same attributes
123+
(
124+
'<a href="https://mozilla.org">Mozilla</a>',
125+
'<a href="https://mozilla.org" target="_blank" '
126+
'rel="noopener noreferrer">Mozilla</a>',
127+
),
128+
("<b>bold</b> & <i>italic</i>", "<b>bold</b> &amp; <i>italic</i>"),
129+
# Email addresses are not linkified
130+
("contact me@example.com", "contact me@example.com"),
131+
(
132+
"me@example.com or https://mozilla.org",
133+
'me@example.com or <a href="https://mozilla.org" target="_blank" '
134+
'rel="noopener noreferrer">https://mozilla.org</a>',
135+
),
136+
),
137+
)
138+
def test_helper_linkify(source, expected):
139+
assert linkify(source) == expected

pontoon/base/tests/test_utils.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from django.urls import reverse
66
from django.urls.exceptions import NoReverseMatch
77

8-
from pontoon.base.utils import get_m2m_changes, get_search_phrases
8+
from pontoon.base.utils import get_m2m_changes, get_search_phrases, sanitize_html
99
from pontoon.test.factories import (
1010
LocaleCodeHistoryFactory,
1111
LocaleFactory,
@@ -585,3 +585,40 @@ def test_get_m2m_mixed(user_a, user_b, user_c):
585585
)
586586
def test_get_search_phrases(search_query, expected_results):
587587
assert get_search_phrases(search_query) == expected_results
588+
589+
590+
@pytest.mark.parametrize(
591+
"html,expected",
592+
(
593+
("", ""),
594+
("plain text", "plain text"),
595+
# Allowed tags and attributes are kept
596+
(
597+
'<p>Hello <b>bold</b> <a href="https://mozilla.org" target="_blank" '
598+
'title="t">link</a></p>',
599+
'<p>Hello <b>bold</b> <a href="https://mozilla.org" target="_blank" '
600+
'title="t">link</a></p>',
601+
),
602+
("<ul><li>one</li></ul><br>", "<ul><li>one</li></ul><br>"),
603+
# Disallowed tags are stripped, but their content is kept
604+
('<div class="c">div <em>em</em></div>', "div <em>em</em>"),
605+
("<img src=x onerror=alert(1)>text", "text"),
606+
# Dangerous content is dropped entirely
607+
("a<script>alert(1)</script>b", "ab"),
608+
# Disallowed attributes and URL schemes are removed
609+
(
610+
'<a href="https://mozilla.org" onclick="x()">l</a>',
611+
'<a href="https://mozilla.org">l</a>',
612+
),
613+
('<a href="javascript:alert(1)">l</a>', "<a>l</a>"),
614+
(
615+
'<a href="mailto:a@example.com">m</a>',
616+
'<a href="mailto:a@example.com">m</a>',
617+
),
618+
('<p style="color:red">styled</p>', "<p>styled</p>"),
619+
# Text is escaped
620+
("Tom & Jerry <3", "Tom &amp; Jerry &lt;3"),
621+
),
622+
)
623+
def test_sanitize_html(html, expected):
624+
assert sanitize_html(html) == expected

pontoon/base/tests/views/test_comment.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def test_add_comment(member, translation_a):
3232
def test_add_comment_sanitizes_html(member, entity_a, locale_a, project_locale_a):
3333
url = reverse("pontoon.add_comment")
3434

35-
payload = "<svg><script>alert(1)</script>safe"
35+
payload = "<svg><script>alert(1)</script></svg>safe"
3636

3737
response = member.client.post(
3838
url,

pontoon/base/utils.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
from datetime import datetime
66
from xml.sax.saxutils import escape, quoteattr
77

8+
from justhtml import JustHTML, SanitizationPolicy, UrlPolicy, UrlRule
9+
10+
from django.conf import settings
811
from django.http import HttpResponseBadRequest
912
from django.utils.text import slugify
1013
from django.utils.timezone import make_aware
@@ -213,3 +216,25 @@ def get_search_phrases(search):
213216

214217
def parse_bool(value) -> bool:
215218
return str(value).lower() in ("1", "true", "yes", "on")
219+
220+
221+
@functools.cache
222+
def _html_sanitization_policy():
223+
return SanitizationPolicy(
224+
allowed_tags=frozenset(settings.ALLOWED_TAGS),
225+
allowed_attributes=settings.ALLOWED_ATTRIBUTES,
226+
url_policy=UrlPolicy(
227+
allow_rules={
228+
("a", "href"): UrlRule(allowed_schemes={"http", "https", "mailto"}),
229+
}
230+
),
231+
# Strip disallowed tags but keep their text content
232+
disallowed_tag_handling="unwrap",
233+
)
234+
235+
236+
def sanitize_html(text):
237+
"""Strip HTML tags and attributes not in ALLOWED_TAGS and ALLOWED_ATTRIBUTES."""
238+
return JustHTML(text, fragment=True, policy=_html_sanitization_policy()).to_html(
239+
pretty=False
240+
)

pontoon/settings/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ def _default_from_email():
10471047
# See docs/admin/deployment.rst for more information.
10481048
ENABLE_INSIGHTS = os.environ.get("ENABLE_INSIGHTS", "False") != "False"
10491049

1050-
# Bleach tags and attributes
1050+
# Tags and attributes allowed in user-submitted HTML
10511051
ALLOWED_TAGS = [
10521052
"a",
10531053
"abbr",

pontoon/teams/views.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
from typing import cast
77

8-
import bleach
9-
108
from guardian.decorators import permission_required_or_403
119

1210
from django.conf import settings
@@ -53,7 +51,7 @@
5351
user_locale_role,
5452
user_role,
5553
)
56-
from pontoon.base.utils import require_AJAX
54+
from pontoon.base.utils import require_AJAX, sanitize_html
5755
from pontoon.contributors.views import ContributorsMixin
5856
from pontoon.insights.utils import get_locale_health_insights, get_locale_insights
5957
from pontoon.teams.forms import LocaleRequestForm
@@ -231,12 +229,7 @@ def ajax_info(request, locale):
231229
@transaction.atomic
232230
def ajax_update_info(request, locale):
233231
team_description = request.POST.get("team_info", None)
234-
team_description = bleach.clean(
235-
team_description,
236-
strip=True,
237-
tags=settings.ALLOWED_TAGS,
238-
attributes=settings.ALLOWED_ATTRIBUTES,
239-
)
232+
team_description = sanitize_html(team_description)
240233
locale = get_object_or_404(Locale, code=locale)
241234
locale.team_description = team_description
242235
locale.save()
@@ -268,7 +261,7 @@ def ajax_permissions(request, locale):
268261
else:
269262
errors = locale_form.errors
270263
errors.update(project_locale_form.errors_dict)
271-
error_msg = bleach.clean(json.dumps(errors))
264+
error_msg = sanitize_html(json.dumps(errors))
272265
return HttpResponseBadRequest(error_msg)
273266

274267
else:

requirements/base.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
# Dependencies that do not come from pypi (eg. eggs from github) are listed at the end of the list.
1717
# -------------------------------------------------------------------------------------------------
1818
beautifulsoup4==4.12.3
19-
bleach==6.4.0
2019
celery==5.4.0
2120
compare-locales==9.0.4
2221
dj-database-url==3.1.2
@@ -36,6 +35,7 @@ django-pipeline==4.1.0
3635
drf-spectacular[sidecar]==0.29.0
3736
google-cloud-translate==3.16.0
3837
gunicorn==23.0.0
38+
justhtml==3.11.2
3939
markupsafe==2.0.1
4040
moz.l10n[xml]==0.14.1
4141
newrelic==9.6.0

requirements/dev.txt

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,6 @@ billiard==4.2.4 \
8181
# via
8282
# -c prod.txt
8383
# celery
84-
bleach==6.4.0 \
85-
--hash=sha256:4202482733d85cedd04e59fcb2f89f4e4c7c385a78d3c3c23c30446843a37452 \
86-
--hash=sha256:4b6b6a54fff2e69a3dde9d21cc6301220bee3c3cb792187d11403fd795031081
87-
# via
88-
# -c prod.txt
89-
# -r base.in
9084
celery==5.4.0 \
9185
--hash=sha256:369631eb580cf8c51a82721ec538684994f8277637edde2dfc0dacd73ed97f64 \
9286
--hash=sha256:504a19140e8d3029d5acad88330c541d4c3f64c789d85f94756762d8bca7e706
@@ -952,6 +946,12 @@ jsonschema-specifications==2025.9.1 \
952946
# via
953947
# -c prod.txt
954948
# jsonschema
949+
justhtml==3.11.2 \
950+
--hash=sha256:366986de83fab5f7ab643f79ffb3ad44dbaeb63f34b6361b955930575417cdd2 \
951+
--hash=sha256:83329a7436620a79ebd0b7310b5e4d653fa0688cd315e7a92d2188febd0d6ab0
952+
# via
953+
# -c prod.txt
954+
# -r base.in
955955
kombu==5.6.2 \
956956
--hash=sha256:8060497058066c6f5aed7c26d7cd0d3b574990b09de842a8c5aaed0b92cc5a55 \
957957
--hash=sha256:efcfc559da324d41d61ca311b0c64965ea35b4c55cc04ee36e55386145dace93
@@ -2499,12 +2499,6 @@ wcwidth==0.6.0 \
24992499
# via
25002500
# -c prod.txt
25012501
# prompt-toolkit
2502-
webencodings==0.5.1 \
2503-
--hash=sha256:a0af1213f3c2226497a97e2b3aa01a7e4bee4f403f95be16fc9acd2947514a78 \
2504-
--hash=sha256:b36a1c245f2d304965eb4e0a82848379241dc04b865afcc4aab16748587e1923
2505-
# via
2506-
# -c prod.txt
2507-
# bleach
25082502
wheel==0.46.3 \
25092503
--hash=sha256:4b399d56c9d9338230118d705d9737a2a468ccca63d5e813e2a4fc7815d8bc4d \
25102504
--hash=sha256:e3e79874b07d776c40bd6033f8ddf76a7dad46a7b8aa1b2787a83083519a1803

0 commit comments

Comments
 (0)