[Django] #34609: Deprecate format_html calls without args or kwargs
Django
------------------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
In my experience, a common misuse of `format_html` is to format the HTML
before calling it:
{{{
format_html(f"<i>{name}")
}}}
This makes it act like `mark_safe`, allowing data through without
escaping. It provides a false sense of security since `format_html` is
meant to be the "safe way".
I propose we deprecate calls to format_html that don’t pass `args` or
`kwargs`, and eventually raise a `TypeError` for such cases.
--
Ticket URL: <https://code.djangoproject.com/ticket/34609>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Adam Johnson | Owner: nobody
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Adam Johnson:
Old description:
> In my experience, a common misuse of `format_html` is to format the HTML
> before calling it:
>
> {{{
> format_html(f"<i>{name}")
> }}}
>
> This makes it act like `mark_safe`, allowing data through without
> escaping. It provides a false sense of security since `format_html` is
> meant to be the "safe way".
>
> I propose we deprecate calls to format_html that don’t pass `args` or
> `kwargs`, and eventually raise a `TypeError` for such cases.
New description:
In my experience, a common misuse of `format_html` is to format the HTML
before calling it:
{{{
format_html(f"<i>{name}")
}}}
This makes it act like `mark_safe`, allowing data through without
escaping. It provides a false sense of security since `format_html` is
meant to be the "safe way".
I propose we deprecate calls to format_html that don’t pass `args` or
`kwargs`, and eventually raise a `TypeError` for such cases.
(Following improvement to `format_html` docs in #34595.)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:1>
Django
Reporter: Adam Johnson | Owner: nobody
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Bhuvnesh):
{{{#!diff
diff --git a/django/utils/html.py b/django/utils/html.py
index c32a36fa93..b2a0c3d3db 100644
--- a/django/utils/html.py
+++ b/django/utils/html.py
@@ -100,6 +100,8 @@ def format_html(format_string, *args, **kwargs):
and call mark_safe() on the result. This function should be used
instead
of str.format or % interpolation to build up small HTML fragments.
"""
+ if not (args or kwargs):
+ raise TypeError("Arguments are missing.")
args_safe = map(conditional_escape, args)
kwargs_safe = {k: conditional_escape(v) for (k, v) in kwargs.items()}
return mark_safe(format_string.format(*args_safe, **kwargs_safe))
diff --git a/tests/utils_tests/test_html.py
b/tests/utils_tests/test_html.py
index b7a7396075..c83fe7ddf6 100644
--- a/tests/utils_tests/test_html.py
+++ b/tests/utils_tests/test_html.py
@@ -65,6 +65,16 @@ class TestUtilsHtml(SimpleTestCase):
"< Dangerous > <b>safe</b> < dangerous again <i>safe
again</i>",
)
+ def test_format_html_no_args(self):
+ msg = "Arguments are missing."
+ with self.assertRaisesMessage(TypeError, msg):
+ self.assertEqual(
+ format_html(
+ "<i>{name}</i>",
+ ),
+ "<i>Adam</i>",
+ )
+
def test_linebreaks(self):
items = (
("para1\n\npara2\r\rpara3",
"<p>para1</p>\n\n<p>para2</p>\n\n<p>para3</p>"),
}}}
Are these changes relevant? I don't have much experience with templates,
still a lot to learn .😅
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:3>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: nobody => Bhuvnesh
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Old description:
> In my experience, a common misuse of `format_html` is to format the HTML
> before calling it:
>
> {{{
> format_html(f"<i>{name}")
> }}}
>
> This makes it act like `mark_safe`, allowing data through without
> escaping. It provides a false sense of security since `format_html` is
> meant to be the "safe way".
>
> I propose we deprecate calls to format_html that don’t pass `args` or
> `kwargs`, and eventually raise a `TypeError` for such cases.
>
> (Following improvement to `format_html` docs in #34595.)
New description:
In my experience, a common misuse of `format_html` is to format the HTML
before calling it:
{{{
format_html(f"<i>{name}</i>")
}}}
This makes it act like `mark_safe`, allowing data through without
escaping. It provides a false sense of security since `format_html` is
meant to be the "safe way".
I propose we deprecate calls to format_html that don’t pass `args` or
`kwargs`, and eventually raise a `TypeError` for such cases.
(Following improvement to `format_html` docs in #34595.)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Michael Howitz):
@Bhuvnesh The issues talks about deprecating that args resp. kwargs can be
missing.
By raising an exception your suggested change make it impossible to call
the function without these parameters. Maybe this is a bit too harsh.
See also https://docs.djangoproject.com/en/dev/internals/release-process
/#internal-release-deprecation-policy for documentation how to deprecate a
feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Bhuvnesh):
OK, so instead of {{{TypeError}}} I should raise a
**RemovedInDjango60Warning** warning?
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16948 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:9>
Django
Reporter: Adam Johnson | Owner: Bhuvnesh
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_docs: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Cleanup/optimization |
Component: Utilities | Version: dev
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"094b0bea2ce76db9d3dc06c384d4ac3b22705810" 094b0be]:
{{{
#!CommitTicketReference repository=""
revision="094b0bea2ce76db9d3dc06c384d4ac3b22705810"
Fixed #34609 -- Deprecated calling format_html() without arguments.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In [changeset:"2b71b2c8dcd40f2604310bb3914077320035b399" 2b71b2c]:
{{{#!CommitTicketReference repository=""
revision="2b71b2c8dcd40f2604310bb3914077320035b399"
Refs #34609 -- Fixed deprecation warning stack level in format_html().
Co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In [changeset:"03e0ab5c64d4bc09c6932268b29efcc789a0f7af" 03e0ab5]:
{{{#!CommitTicketReference repository=""
revision="03e0ab5c64d4bc09c6932268b29efcc789a0f7af"
[5.1.x] Refs #34609 -- Fixed deprecation warning stack level in
Backport of 2b71b2c8dcd40f2604310bb3914077320035b399 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Bhuvnesh
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In [changeset:"1e331911a88f289f52e3d81340e209d0b78abdbb" 1e331911]:
{{{#!CommitTicketReference repository=""
revision="1e331911a88f289f52e3d81340e209d0b78abdbb"
Refs #34609 -- Removed support for calling format_html() without arguments
per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34609#comment:14>