Opened 13 years ago
Closed 10 years ago
#16617 closed Bug (fixed)
URLField with min_length or max_length reports wrong length in validation messages
Reported by: | Fraser Nevett | Owned by: | ANUBHAV JOSHI |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | me@… | 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
Here's a simple example to illustrate the problem:
>>> from django.forms import URLField >>> URLField(max_length=5).clean('example.com') Traceback (most recent call last): File "<console>", line 1, in <module> File "django/forms/fields.py", line 153, in clean self.run_validators(value) File "django/forms/fields.py", line 142, in run_validators raise ValidationError(errors) ValidationError: [u'Ensure this value has at most 5 characters (it has 19).']
The entered value (example.com) is only 11 characters long, but the validation message says that it is 19. This obviously has the potential to cause confusion to the user.
The value 19 comes from the fact that length check is performed after the value has been sanitised to http://example.com/.
A possible solution would be to add the sanitised value to the validation message so that the user can better understand to what value the length restriction actually applies. For example:
Ensure this value has at most 5 characters ("http://example.com/" has 19).
Note that the same problem occurs with min_length
as with max_length
.
Attachments (1)
Change History (15)
comment:1 by , 13 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
by , 13 years ago
Attachment: | length-validators.diff added |
---|
comment:3 by , 13 years ago
Has patch: | set |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Patch attached for django/core/validators.py that allows 'value' to remain as its original type, rather than being converted to an integer length. This allows the message to contain the string, or in the case of MaxLengthValidator, the first limit_value chars of the string.
comment:4 by , 13 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Resolution: | fixed |
Status: | closed → reopened |
Reopening ticket as the patch hasn't actually been committed, so the ticket is not yet fixed.
I'm also not sure that the patch is likely to be accepted when reviewed by a core dev as it's not immediately obvious how it resolves the problem and there are no test cases to verify it is behaving correctly.
comment:5 by , 12 years ago
Status: | reopened → new |
---|
comment:6 by , 11 years ago
https://github.com/django/django/pull/2261
Makes value available to BaseValidator types for use in the MinLengthValidator/MaxLengthValidator message.
comment:7 by , 11 years ago
Including value
in the params
dict looks good, but I wonder if it might be better to leave the default error messages as is and either offer an option to easily switch the default error message to include the value or just let people override the message as they see fit. In the case of really long values, it could be undesirable to have the value repeated in the error message. Any thoughts?
comment:8 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 11 years ago
Owner: | changed from | to
---|
comment:12 by , 10 years ago
Needs tests: | unset |
---|---|
Owner: | changed from | to
Patch needs improvement: | unset |
comment:13 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:14 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Patch shows string rather than length for min|max length