Opened 18 months ago

Closed 18 months ago

Last modified 3 months ago

#34609 closed Cleanup/optimization (fixed)

Deprecate format_html calls without args or kwargs

Reported by: Adam Johnson Owned by: Bhuvnesh
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Michael Howitz)

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

Change History (13)

comment:1 by Adam Johnson, 18 months ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 18 months ago

Triage Stage: Unreviewed β†’ Accepted

comment:3 by Bhuvnesh, 18 months ago

  • django/utils/html.py

    diff --git a/django/utils/html.py b/django/utils/html.py
    index c32a36fa93..b2a0c3d3db 100644
    a b def format_html(format_string, *args, **kwargs):  
    100100    and call mark_safe() on the result. This function should be used instead
    101101    of str.format or % interpolation to build up small HTML fragments.
    102102    """
     103    if not (args or kwargs):
     104        raise TypeError("Arguments are missing.")
    103105    args_safe = map(conditional_escape, args)
    104106    kwargs_safe = {k: conditional_escape(v) for (k, v) in kwargs.items()}
    105107    return mark_safe(format_string.format(*args_safe, **kwargs_safe))
  • tests/utils_tests/test_html.py

    diff --git a/tests/utils_tests/test_html.py b/tests/utils_tests/test_html.py
    index b7a7396075..c83fe7ddf6 100644
    a b class TestUtilsHtml(SimpleTestCase):  
    6565            "&lt; Dangerous &gt; <b>safe</b> &lt; dangerous again <i>safe again</i>",
    6666        )
    6767
     68    def test_format_html_no_args(self):
     69        msg = "Arguments are missing."
     70        with self.assertRaisesMessage(TypeError, msg):
     71            self.assertEqual(
     72                format_html(
     73                    "<i>{name}</i>",
     74                ),
     75                "<i>Adam</i>",
     76            )
     77
    6878    def test_linebreaks(self):
    6979        items = (
    7080            ("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 .πŸ˜…

comment:4 by Bhuvnesh, 18 months ago

Owner: changed from nobody to Bhuvnesh
Status: new β†’ assigned

comment:5 by Michael Howitz, 18 months ago

Description: modified (diff)

comment:6 by Michael Howitz, 18 months ago

@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.

comment:7 by Bhuvnesh, 18 months ago

OK, so instead of TypeError I should raise a RemovedInDjango60Warning warning?

Version 0, edited 18 months ago by Bhuvnesh (next)

comment:8 by Bhuvnesh, 18 months ago

Has patch: set

comment:9 by Mariusz Felisiak, 18 months ago

Needs documentation: set

comment:10 by Mariusz Felisiak, 18 months ago

Needs documentation: unset
Triage Stage: Accepted β†’ Ready for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 18 months ago

Resolution: β†’ fixed
Status: assigned β†’ closed

In 094b0be:

Fixed #34609 -- Deprecated calling format_html() without arguments.

comment:12 by GitHub <noreply@…>, 3 months ago

In 2b71b2c:

Refs #34609 -- Fixed deprecation warning stack level in format_html().

Co-authored-by: Simon Charette <charette.s@…>

comment:13 by Natalia <124304+nessita@…>, 3 months ago

In 03e0ab5:

[5.1.x] Refs #34609 -- Fixed deprecation warning stack level in format_html().

Co-authored-by: Simon Charette <charette.s@…>

Backport of 2b71b2c8dcd40f2604310bb3914077320035b399 from main.

Note: See TracTickets for help on using tickets.
Back to Top