Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#19577 closed Bug (fixed)

admin doc may encourage bad practices

Reported by: foo@… Owned by: nobody
Component: Documentation Version: 1.4
Severity: Normal Keywords:
Cc: apollo13 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

On https://docs.djangoproject.com/en/1.4/ref/contrib/admin/ two places example code demonstrate allow_tags=True in order to return HTML fragments containing <span> tags. Inside these tags data (self.first_name and self.last_name) is not escaped/quoted.

As such example code often is copy/pasted it probably should reflect "best practices"?

Attachments (2)

19577.diff (2.8 KB) - added by timo 2 years ago.
19577.2.diff (5.2 KB) - added by timo 2 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 2 years ago by apollo13

  • Cc apollo13 added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Yes, this should definitely use format_html (https://docs.djangoproject.com/en/dev/ref/utils/#django.utils.html.format_html). Do you think you can wipe up a patch for that?

Changed 2 years ago by timo

comment:2 Changed 2 years ago by timo

  • Has patch set

comment:3 Changed 2 years ago by apollo13

The indendation is wrong at https://code.djangoproject.com/attachment/ticket/19577/19577.diff#L45 and you need to manually call safe on the arguments of the join https://code.djangoproject.com/attachment/ticket/19577/19577.diff#L63 -- otherwise it looks good to me.

EDIT:// You should be able to use https://github.com/django/django/blob/master/django/utils/html.py#L88 (format_html_join) in the last example.

Last edited 2 years ago by apollo13 (previous) (diff)

comment:4 Changed 2 years ago by apollo13

  • Patch needs improvement set

Changed 2 years ago by timo

comment:5 Changed 2 years ago by timo

  • Patch needs improvement unset

Thanks for taking a look. I don't think the example is meant to assume that get_full_address() returns a safe string, but maybe I interpreted it differently. I added a comment to try to explain what I thought the intent of the example was in addition to the other fixes you mentioned (plus documenting format_html_join).

comment:6 Changed 2 years ago by apollo13

  • Triage Stage changed from Accepted to Ready for checkin

Thanks, looks good now!

comment:7 Changed 2 years ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In eafc0364764ba12babd76194d8e1f78b876471ec:

Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

comment:8 Changed 2 years ago by Tim Graham <timograham@…>

In 42fcfcaa529dac1fe3066797d7e4ab7aa6f6cdf3:

[1.5.x] Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

Backport of eafc036476 from master

comment:9 Changed 2 years ago by Tim Graham <timograham@…>

In 42fcfcaa529dac1fe3066797d7e4ab7aa6f6cdf3:

[1.5.x] Fixed #19577 - Added HTML escaping to admin examples.

Thanks foo@ for the report and Florian Apolloner for the review.

Backport of eafc036476 from master

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