Skip to content

Commit df7a1be

Browse files
authored
fix(security): Use format_html for avatar related functions (#58319)
Continuation of #58227 According to Django docs: https://docs.djangoproject.com/en/4.2/ref/utils/#django.utils.html.format_html > For the case of building up small HTML fragments, this function is to be preferred over string interpolation using `%` or `str.format()` directly, because it applies escaping to all arguments - just like the template system applies escaping by default. > So, instead of writing: > > ``` > mark_safe( > "%s <b>%s</b> %s" > % ( > some_html, > escape(some_text), > escape(some_other_text), > ) > ) > ``` > You should instead use: > > ``` > format_html( > "{} <b>{}</b> {}", > mark_safe(some_html), > some_text, > some_other_text, > ) > ```
1 parent df3ae77 commit df7a1be

File tree

5 files changed

+37
-33
lines changed

5 files changed

+37
-33
lines changed

src/sentry/notifications/notifications/activity/base.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from urllib.parse import urlparse, urlunparse
77

88
from django.utils.html import format_html
9-
from django.utils.safestring import SafeString, mark_safe
9+
from django.utils.safestring import SafeString
1010

1111
from sentry.db.models import Model
1212
from sentry.notifications.helpers import get_reason_context
@@ -186,7 +186,7 @@ def description_as_html(self, description: str, params: Mapping[str, Any]) -> Sa
186186

187187
fmt = '<span class="avatar-container">{}</span> <strong>{}</strong>'
188188

189-
author = format_html(fmt, mark_safe(avatar_as_html(self.user)), name)
189+
author = format_html(fmt, avatar_as_html(self.user), name)
190190

191191
issue_name = self.group.qualified_short_id or "an issue"
192192
an_issue = format_html('<a href="{}">{}</a>', self.get_group_link(), issue_name)

src/sentry/notifications/utils/avatar.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
from __future__ import annotations
22

33
from django.urls import reverse
4-
from django.utils.html import escape
4+
from django.utils.html import format_html
5+
from django.utils.safestring import SafeString
56

67
from sentry.models.avatars.user_avatar import UserAvatar
78
from sentry.models.user import User
@@ -37,14 +38,14 @@ def get_sentry_avatar_url() -> str:
3738
return str(absolute_uri(get_asset_url("sentry", url)))
3839

3940

40-
def avatar_as_html(user: User | RpcUser) -> str:
41+
def avatar_as_html(user: User | RpcUser) -> SafeString:
4142
if not user:
42-
return '<img class="avatar" src="{}" width="20px" height="20px" />'.format(
43-
escape(get_sentry_avatar_url())
43+
return format_html(
44+
'<img class="avatar" src="{}" width="20px" height="20px" />', get_sentry_avatar_url()
4445
)
4546
avatar_type = user.get_avatar_type()
4647
if avatar_type == "upload":
47-
return f'<img class="avatar" src="{escape(get_user_avatar_url(user))}" />'
48+
return format_html('<img class="avatar" src="{}" />', get_user_avatar_url(user))
4849
elif avatar_type == "letter_avatar":
4950
return get_email_avatar(user.get_display_name(), user.get_label(), 20, False)
5051
else:

src/sentry/templatetags/sentry_avatars.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from django import template
44
from django.urls import reverse
5-
from django.utils.safestring import mark_safe
65

76
from sentry.models.avatars.user_avatar import UserAvatar
87
from sentry.models.user import User
@@ -23,7 +22,7 @@ def gravatar_url(context, email, size, default="mm"):
2322

2423
@register.simple_tag(takes_context=True)
2524
def letter_avatar_svg(context, display_name, identifier, size=None):
26-
return mark_safe(get_letter_avatar(display_name, identifier, size=size))
25+
return get_letter_avatar(display_name, identifier, size=size)
2726

2827

2928
@register.simple_tag(takes_context=True)
@@ -42,7 +41,7 @@ def profile_photo_url(context, user_id, size=None):
4241
# than 1-2 avatars. It will make a request for every user!
4342
@register.simple_tag(takes_context=True)
4443
def email_avatar(context, display_name, identifier, size=None, try_gravatar=True):
45-
return mark_safe(get_email_avatar(display_name, identifier, size, try_gravatar))
44+
return get_email_avatar(display_name, identifier, size, try_gravatar)
4645

4746

4847
@register.inclusion_tag("sentry/partial/avatar.html")
Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
from django import template
2-
from django.utils.safestring import mark_safe
32

43
from sentry.utils.avatar import get_letter_avatar, get_platform_avatar
54

@@ -8,18 +7,16 @@
87

98
@register.simple_tag(takes_context=True)
109
def letter_platform_svg(context, display_name, identifier, size=None, rounded=False):
11-
return mark_safe(
12-
get_letter_avatar(
13-
display_name,
14-
identifier,
15-
size,
16-
use_svg=False,
17-
initials=display_name[0:2] if display_name else None,
18-
rounded=rounded,
19-
)
10+
return get_letter_avatar(
11+
display_name,
12+
identifier,
13+
size,
14+
use_svg=False,
15+
initials=display_name[0:2] if display_name else None,
16+
rounded=rounded,
2017
)
2118

2219

2320
@register.simple_tag(takes_context=True)
2421
def platform_avatar(context, display_name, size=36):
25-
return mark_safe(get_platform_avatar(display_name, size))
22+
return get_platform_avatar(display_name, size)

src/sentry/utils/avatar.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
from django.core.exceptions import ValidationError
1212
from django.core.validators import validate_email
1313
from django.utils.encoding import force_str
14-
from django.utils.html import escape
14+
from django.utils.html import escape, format_html
15+
from django.utils.safestring import SafeString
1516
from PIL import Image
1617

1718
from sentry.http import safe_urlopen
@@ -74,32 +75,34 @@ def get_letter_avatar(
7475
use_svg: Optional[bool] = True,
7576
initials: Optional[str] = None,
7677
rounded: Optional[bool] = False,
77-
) -> str:
78+
) -> SafeString:
7879
display_name = (display_name or "").strip() or "?"
7980
names = display_name.split(" ")
8081
initials = initials or "{}{}".format(names[0][0], names[-1][0] if len(names) > 1 else "")
8182
initials = escape(initials.upper())
8283
color = get_letter_avatar_color(identifier)
8384
if use_svg:
8485
size_attrs = f'height="{size}" width="{size}"' if size else ""
85-
return (
86+
return format_html(
8687
'<svg viewBox="0 0 120 120" xmlns="http://www.w3.org/2000/svg" {size_attrs}>'
8788
'<rect x="0" y="0" width="120" height="120" rx="15" ry="15" fill={color}></rect>'
8889
'<text x="50%" y="50%" font-size="65" dominant-baseline="central" text-anchor="middle" fill="#FFFFFF">'
8990
"{initials}"
9091
"</text>"
91-
"</svg>"
92-
).format(color=color, initials=initials, size_attrs=size_attrs)
92+
"</svg>",
93+
color=color,
94+
initials=initials,
95+
size_attrs=size_attrs,
96+
)
9397
else:
9498
size_attrs = f"height:{size}px;width:{size}px;" if size else ""
9599
font_size = "font-size:%spx;" % (size / 2) if size else ""
96100
line_height = "line-height:%spx;" % size if size else ""
97101
span_class = " rounded" if rounded else ""
98-
return (
102+
return format_html(
99103
'<span class="html-avatar{span_class}" '
100104
'style="background-color:{color};{size_attrs}{font_size}{line_height}">'
101-
"{initials}</span>"
102-
).format(
105+
"{initials}</span>",
103106
color=color,
104107
initials=initials,
105108
size_attrs=size_attrs,
@@ -114,7 +117,7 @@ def get_email_avatar(
114117
identifier: str,
115118
size: Optional[int] = None,
116119
try_gravatar: Optional[bool] = True,
117-
) -> str:
120+
) -> SafeString:
118121
if try_gravatar:
119122
try:
120123
validate_email(identifier)
@@ -129,16 +132,20 @@ def get_email_avatar(
129132
if resp.status_code == 200:
130133
# default to mm if including in emails
131134
gravatar_url = get_gravatar_url(identifier, size=size)
132-
return f'<img class="avatar" src="{gravatar_url}">'
135+
return format_html('<img class="avatar" src="{}">', gravatar_url)
133136
return get_letter_avatar(display_name, identifier, size, use_svg=False)
134137

135138

136139
def get_platform_avatar(
137140
display_name: Optional[str],
138141
size: Optional[int] = None,
139-
) -> str:
142+
) -> SafeString:
140143
# TODO: @taylangocmen add platformicons from package when available
141-
return f'<img class="avatar" src="https://raw.githubusercontent.com/getsentry/platformicons/master/svg/{display_name}.svg" height={size}>'
144+
return format_html(
145+
'<img class="avatar" src="https://raw.githubusercontent.com/getsentry/platformicons/master/svg/{display_name}.svg" height={size}>',
146+
display_name=display_name,
147+
size=size,
148+
)
142149

143150

144151
def is_black_alpha_only(data: IO[bytes]) -> bool:

0 commit comments

Comments
 (0)