Opened 4 years ago

Closed 4 years ago

Last modified 4 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: Florian Apolloner 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 Tim Graham 4 years ago.
19577.2.diff (5.2 KB) - added by Tim Graham 4 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by Florian Apolloner

Cc: Florian Apolloner added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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 4 years ago by Tim Graham

Attachment: 19577.diff added

comment:2 Changed 4 years ago by Tim Graham

Has patch: set

comment:3 Changed 4 years ago by Florian Apolloner

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 4 years ago by Florian Apolloner (previous) (diff)

comment:4 Changed 4 years ago by Florian Apolloner

Patch needs improvement: set

Changed 4 years ago by Tim Graham

Attachment: 19577.2.diff added

comment:5 Changed 4 years ago by Tim Graham

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 4 years ago by Florian Apolloner

Triage Stage: AcceptedReady for checkin

Thanks, looks good now!

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

Resolution: fixed
Status: newclosed

In eafc0364764ba12babd76194d8e1f78b876471ec:

Fixed #19577 - Added HTML escaping to admin examples.

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

comment:8 Changed 4 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 4 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