#6160 closed (fixed)
Escaping of validation errors
Reported by: | 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)
Change History (16)
by , 17 years ago
Attachment: | 00-validation-error-escape.diff added |
---|
comment:1 by , 17 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 17 years ago
Keywords: | easy-pickings added |
---|
by , 17 years ago
Attachment: | validation-escaping.diff added |
---|
comment:3 by , 17 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 , 16 years ago
Attachment: | validation-escaping.2.diff added |
---|
comment:4 by , 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 , 16 years ago
Attachment: | validation-escaping.3.diff added |
---|
by , 16 years ago
Attachment: | validation-escaping.4.diff added |
---|
by , 16 years ago
Attachment: | validation-escaping.5.diff added |
---|
comment:5 by , 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 , 16 years ago
Cc: | 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 , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:9 by , 16 years ago
comment:10 by , 16 years ago
Cc: | removed |
---|
Should probably be using
conditional_escape()
instead ofescape()
here so that it won't get double-escaped if the developer already escaped the text.