Django

Code

Ticket #9111 (closed: fixed)

Opened 4 months ago

Last modified 2 months ago

form error get escaped on _html_output even if it is a SafeString instance

Reported by: michelts Assigned to: kmtracey
Milestone: Component: Forms
Version: 1.0 Keywords: form error escape safestring mark_safe
Cc: michelts@gmail.com, adunar Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Hi guys,

When I define a form and call it on a template like: 

{{ form }}

The function as_table is called. The function calls _html_output to render the widget, the errors and help messages. They are in django.forms.forms module.

The error is escaped with django.utils.html.escape function but I think django.utils.html.conditional_escape should be used instead. This way, if I pass a SafeString instance to a forms.ValidationError exception, I can include html code on it.

This error isn't raised if I write a template like: 

<div>{{ form.field.errors }}{{ form.field }}</div>

What about to change escape to conditional_escape in all the module? I can send a patch if you agree...

Best regards!

Attachments

safestring-r9066.diff (1.1 kB) - added by kratorius on 09/17/08 11:23:50.
safestring-tests-r9066.diff (2.3 kB) - added by kratorius on 09/17/08 11:24:04.
form_escape.diff (4.8 kB) - added by adunar on 10/21/08 18:00:30.
conditional_escape for both field labels and errors

Change History

09/16/08 16:01:34 changed by michelts

  • cc set to michelts@gmail.com.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

09/17/08 02:54:08 changed by mtredinnick

  • stage changed from Unreviewed to Accepted.

This looks like a good change to make. Please also remember to include a test that fails beforehand and passes after the change. Probably the best place to put the test is in regressiontests/forms/forms.py.

09/17/08 11:23:50 changed by kratorius

  • attachment safestring-r9066.diff added.

09/17/08 11:24:04 changed by kratorius

  • attachment safestring-tests-r9066.diff added.

09/17/08 11:24:54 changed by kratorius

  • has_patch set to 1.

09/17/08 11:25:58 changed by kratorius

  • owner changed from nobody to kratorius.

10/21/08 17:59:48 changed by adunar

  • cc changed from michelts@gmail.com to michelts@gmail.com, adunar.

As I noted on http://groups.google.com/group/django-developers/browse_thread/thread/86fd952b0efc641e , mark_safe should also work on labels of form fields. I've updated the patch to also call conditional_escape on form labels, and added a regression test for that.

10/21/08 18:00:30 changed by adunar

  • attachment form_escape.diff added.

conditional_escape for both field labels and errors

11/06/08 13:22:23 changed by kmtracey

  • owner changed from kratorius to kmtracey.
  • status changed from new to assigned.

In the future please don't add additional unrelated fixes to existing tickets. This ticket was about escaping of errors, not labels -- fixing the label case should have gone into a different ticket to make things simpler when reviewing.

The errors part of this ticket overlaps a bit with #6160, which points out a different part of the code where error messages are not escaped. I'm going to deal with both together so that errors are consistently conditionally escaped.

11/06/08 13:49:24 changed by kmtracey

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [9365]) Fixed #6160, #9111 -- Consistently apply conditional_escape to form errors and labels when outputing them as HTML.

11/06/08 13:53:37 changed by kmtracey

(In [9366]) [1.0.X] Fixed #6160, #9111 -- Consistently apply conditional_escape to form errors and labels when outputing them as HTML.

[9365] from trunk.

Add/Change #9111 (form error get escaped on _html_output even if it is a SafeString instance)




Change Properties
Action