Opened 16 years ago

Closed 15 years ago

Last modified 15 years ago

#6160 closed (fixed)

Escaping of validation errors

Reported by: Petr Marhoun <petr.marhoun@…> Owned by: Karen Tracey
Component: Forms Version: dev
Severity: Keywords: easy-pickings
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Error classes from django.newforms.utils.py don't escape error messages:

class ErrorList(list, StrAndUnicode):

    def as_ul(self):
        if not self: return u''
        return mark_safe(u'<ul class="errorlist">%s</ul>'
                % ''.join([u'<li>%s</li>' % force_unicode(e) for e in self]))

I don't thing that author of validation error (or translator) should use html escaping - message can be used in another context. But utility method as_ul is for html so I think it should escape it.

I am not sure if my patch is the right solution but it works for me.

Attachments (6)

00-validation-error-escape.diff (510 bytes ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.
validation-escaping.diff (7.7 KB ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.
validation-escaping.2.diff (8.0 KB ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.
validation-escaping.3.diff (9.2 KB ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.
validation-escaping.4.diff (9.7 KB ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.
validation-escaping.5.diff (4.9 KB ) - added by Petr Marhoun <petr.marhoun@…> 16 years ago.

Download all attachments as: .zip

Change History (16)

by Petr Marhoun <petr.marhoun@…>, 16 years ago

comment:1 by Gary Wilson, 16 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Should probably be using conditional_escape() instead of escape() here so that it won't get double-escaped if the developer already escaped the text.

comment:2 by Chris Beaven, 16 years ago

Keywords: easy-pickings added

by Petr Marhoun <petr.marhoun@…>, 16 years ago

Attachment: validation-escaping.diff added

comment:3 by Petr Marhoun <petr.marhoun@…>, 16 years ago

It is not so simple as it seemed to be:

  • There are more problems in django.newforms.utils.
  • There are some interactions between ErrorList and ErrorDict.
  • There are some interactions between ErrorList and BaseForm.

The new patch should solve it and its tests should prove it.

by Petr Marhoun <petr.marhoun@…>, 16 years ago

Attachment: validation-escaping.2.diff added

comment:4 by Petr Marhoun <petr.marhoun@…>, 16 years ago

New version of patch does not remove escape in _html_output - it only replaces it with conditional_escape. I think it is safer.

by Petr Marhoun <petr.marhoun@…>, 16 years ago

Attachment: validation-escaping.3.diff added

by Petr Marhoun <petr.marhoun@…>, 16 years ago

Attachment: validation-escaping.4.diff added

by Petr Marhoun <petr.marhoun@…>, 16 years ago

Attachment: validation-escaping.5.diff added

comment:5 by Petr Marhoun <petr.marhoun@…>, 16 years ago

Needs tests: unset
Patch needs improvement: unset

I thing the last version of patch has sensible tests (the previous one was quite strange for a validation error) and also quite similar solution as [8601] (Allow safe-strings in the "attrs" parameter to form widgets). So I changed "Needs tests" and "Patch needs improvement".

comment:6 by Thomas Güttler, 15 years ago

Cc: hv@… added

I installed validation-escaping.5.diff today and it works the way it should. The unittest "./runtests.py forms" pass. I think it is ready for checkin.

comment:7 by Karen Tracey, 15 years ago

Owner: changed from nobody to Karen Tracey
Status: newassigned

comment:8 by Karen Tracey, 15 years ago

Resolution: fixed
Status: assignedclosed

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

comment:9 by Karen Tracey, 15 years ago

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

comment:10 by Thomas Güttler, 15 years ago

Cc: hv@… removed
Note: See TracTickets for help on using tickets.
Back to Top