Skip to content

Conversation

@oioki
Copy link
Member

@oioki oioki commented Oct 18, 2023

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,
)

@oioki oioki requested a review from a team October 18, 2023 09:13
@oioki oioki requested a review from a team as a code owner October 18, 2023 09:13
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 18, 2023
@oioki oioki changed the title Use format_html for avatar related functions fix(security): Use format_html for avatar related functions Oct 18, 2023
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #58319 (cfc1a94) into master (92d7c4f) will decrease coverage by 0.01%.
Report is 6 commits behind head on master.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##           master   #58319      +/-   ##
==========================================
- Coverage   79.07%   79.06%   -0.01%     
==========================================
  Files        5140     5140              
  Lines      223833   223834       +1     
  Branches    37689    37689              
==========================================
- Hits       176995   176981      -14     
- Misses      41171    41180       +9     
- Partials     5667     5673       +6     
Files Coverage Δ
...entry/notifications/notifications/activity/base.py 97.24% <100.00%> (ø)
src/sentry/templatetags/sentry_avatars.py 62.50% <100.00%> (-0.77%) ⬇️
src/sentry/notifications/utils/avatar.py 50.00% <80.00%> (+1.35%) ⬆️
src/sentry/utils/avatar.py 88.40% <83.33%> (+0.17%) ⬆️
src/sentry/templatetags/sentry_platforms.py 77.77% <0.00%> (-2.23%) ⬇️

... and 8 files with indirect coverage changes

@oioki oioki merged commit df7a1be into master Oct 23, 2023
@oioki oioki deleted the fix/safestring-avatar-as-html branch October 23, 2023 08:52
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants