Skip to content

Commit b66ed85

Browse files
authored
Add format_html() fixer (#478)
For #477.
1 parent 2d7d4a5 commit b66ed85

File tree

4 files changed

+255
-1
lines changed

4 files changed

+255
-1
lines changed

CHANGELOG.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
Changelog
33
=========
44

5+
* Add Django 5.0+ fixer to rewrite ``format_html()`` calls without ``args`` or ``kwargs`` probably using ``str.format()`` incorrectly.
6+
7+
`Issue #477 <https://github.com/adamchainz/django-upgrade/issues/477>`__.
8+
59
1.20.0 (2024-07-19)
610
-------------------
711

README.rst

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,24 @@ Django 5.0
242242

243243
`Release Notes <https://docs.djangoproject.com/en/5.0/releases/5.0/>`__
244244

245-
No fixers yet.
245+
``format_html()`` calls
246+
~~~~~~~~~~~~~~~~~~~~~~~
247+
248+
**Name:** ``format_html``
249+
250+
Rewrites ``format_html()`` calls without ``args`` or ``kwargs`` but using ``str.format()``.
251+
Such calls are most likely incorrectly applying formatting without escaping, making them vulnerable to HTML injection.
252+
Such use cases are why calling ``format_html()`` without any arguments or keyword arguments was deprecated in `Ticket #34609 <https://code.djangoproject.com/ticket/34609>`__.
253+
254+
.. code-block:: diff
255+
256+
from django.utils.html import format_html
257+
258+
-format_html("<marquee>{}</marquee>".format(message))
259+
+format_html("<marquee>{}</marquee>", message)
260+
261+
-format_html("<marquee>{name}</marquee>".format(name=name))
262+
+format_html("<marquee>{name}</marquee>", name=name)
246263
247264
Django 4.2
248265
----------
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
"""
2+
Rewrite some format_html() calls passing formatted strings without other
3+
arguments or keyword arguments to use the format_html formatting.
4+
5+
https://docs.djangoproject.com/en/5.0/releases/5.0/#features-deprecated-in-5-0
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import ast
11+
from functools import partial
12+
from typing import Iterable
13+
14+
from tokenize_rt import Offset
15+
from tokenize_rt import Token
16+
17+
from django_upgrade.ast import ast_start_offset
18+
from django_upgrade.data import Fixer
19+
from django_upgrade.data import State
20+
from django_upgrade.data import TokenFunc
21+
from django_upgrade.tokens import OP
22+
from django_upgrade.tokens import alone_on_line
23+
from django_upgrade.tokens import find
24+
from django_upgrade.tokens import find_last_token
25+
from django_upgrade.tokens import insert
26+
27+
fixer = Fixer(
28+
__name__,
29+
min_version=(5, 0),
30+
)
31+
32+
33+
@fixer.register(ast.Call)
34+
def visit_Call(
35+
state: State,
36+
node: ast.Call,
37+
parents: tuple[ast.AST, ...],
38+
) -> Iterable[tuple[Offset, TokenFunc]]:
39+
if (
40+
"format_html" in state.from_imports["django.utils.html"]
41+
and isinstance(node.func, ast.Name)
42+
and node.func.id == "format_html"
43+
# Template only
44+
and len(node.args) == 1
45+
and len(node.keywords) == 0
46+
# str.format()
47+
and isinstance((str_format := node.args[0]), ast.Call)
48+
and isinstance(str_format.func, ast.Attribute)
49+
and isinstance(str_format.func.value, ast.Constant)
50+
and isinstance(str_format.func.value.value, str)
51+
and str_format.func.attr == "format"
52+
):
53+
yield ast_start_offset(node), partial(rewrite_str_format, node=str_format)
54+
55+
56+
def rewrite_str_format(
57+
tokens: list[Token],
58+
i: int,
59+
*,
60+
node: ast.Call,
61+
) -> None:
62+
open_start = find(tokens, i, name=OP, src=".")
63+
open_end = find(tokens, open_start, name=OP, src="(")
64+
65+
# closing paren
66+
cp_start = cp_end = find_last_token(tokens, open_end, node=node)
67+
if alone_on_line(tokens, cp_start, cp_end):
68+
cp_start -= 1
69+
cp_end += 1
70+
71+
del tokens[cp_start : cp_end + 1]
72+
del tokens[open_start : open_end + 1]
73+
insert(tokens, open_start, new_src=", ")

tests/fixers/test_format_html.py

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
from __future__ import annotations
2+
3+
from functools import partial
4+
5+
from django_upgrade.data import Settings
6+
from tests.fixers import tools
7+
8+
settings = Settings(target_version=(5, 0))
9+
check_noop = partial(tools.check_noop, settings=settings)
10+
check_transformed = partial(tools.check_transformed, settings=settings)
11+
12+
13+
def test_not_imported():
14+
check_noop(
15+
"""\
16+
format_html("<marquee>{}</marquee>".format(message))
17+
""",
18+
)
19+
20+
21+
def test_has_arg():
22+
check_noop(
23+
"""\
24+
format_html("<marquee>{} {{}}</marquee>".format(message), name)
25+
""",
26+
)
27+
28+
29+
def test_has_kwarg():
30+
check_noop(
31+
"""\
32+
format_html("<marquee>{} {{name}}</marquee>".format(message), name=name)
33+
""",
34+
)
35+
36+
37+
def test_variable_format_call():
38+
check_noop(
39+
"""\
40+
format_html(template.format(message))
41+
""",
42+
)
43+
44+
45+
def test_int_format_call():
46+
check_noop(
47+
"""\
48+
format_html((1).format(message))
49+
""",
50+
)
51+
52+
53+
def test_not_format():
54+
check_noop(
55+
"""\
56+
format_html("<marquee>{}</marquee>".fmt(message))
57+
""",
58+
)
59+
60+
61+
def test_pos_arg_single():
62+
check_transformed(
63+
"""\
64+
from django.utils.html import format_html
65+
format_html("<marquee>{}</marquee>".format(message))
66+
""",
67+
"""\
68+
from django.utils.html import format_html
69+
format_html("<marquee>{}</marquee>", message)
70+
""",
71+
)
72+
73+
74+
def test_pos_arg_double():
75+
check_transformed(
76+
"""\
77+
from django.utils.html import format_html
78+
format_html("<marquee>{} {}</marquee>".format(message, name))
79+
""",
80+
"""\
81+
from django.utils.html import format_html
82+
format_html("<marquee>{} {}</marquee>", message, name)
83+
""",
84+
)
85+
86+
87+
def test_kwarg_single():
88+
check_transformed(
89+
"""\
90+
from django.utils.html import format_html
91+
format_html("<marquee>{m}</marquee>".format(m=message))
92+
""",
93+
"""\
94+
from django.utils.html import format_html
95+
format_html("<marquee>{m}</marquee>", m=message)
96+
""",
97+
)
98+
99+
100+
def test_kwarg_double():
101+
check_transformed(
102+
"""\
103+
from django.utils.html import format_html
104+
format_html("<marquee>{m} {n}</marquee>".format(m=message, n=name))
105+
""",
106+
"""\
107+
from django.utils.html import format_html
108+
format_html("<marquee>{m} {n}</marquee>", m=message, n=name)
109+
""",
110+
)
111+
112+
113+
def test_pos_kwarg_mixed():
114+
check_transformed(
115+
"""\
116+
from django.utils.html import format_html
117+
format_html("<marquee>{} {n}</marquee>".format(message, n=name))
118+
""",
119+
"""\
120+
from django.utils.html import format_html
121+
format_html("<marquee>{} {n}</marquee>", message, n=name)
122+
""",
123+
)
124+
125+
126+
def test_indented():
127+
check_transformed(
128+
"""\
129+
from django.utils.html import format_html
130+
format_html(
131+
"<marquee>{}</marquee>".format(message)
132+
)
133+
""",
134+
"""\
135+
from django.utils.html import format_html
136+
format_html(
137+
"<marquee>{}</marquee>", message
138+
)
139+
""",
140+
)
141+
142+
143+
def test_indented_double():
144+
check_transformed(
145+
"""\
146+
from django.utils.html import format_html
147+
format_html(
148+
"<marquee>{}</marquee>".format(
149+
message
150+
)
151+
)
152+
""",
153+
"""\
154+
from django.utils.html import format_html
155+
format_html(
156+
"<marquee>{}</marquee>",\x20
157+
message
158+
)
159+
""",
160+
)

0 commit comments

Comments
 (0)