[Django] #16617: URLField with min_length or max_length reports wrong length in validation messages
Django
messages
---------------------+----------------------------
Reporter: frasern | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: Forms
Version: SVN | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------+----------------------------
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/16617>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
messages
Type: Bug | Status: new
Milestone: | Component: Forms
Version: SVN | Severity: Normal
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+------------------------
Changes (by BernhardEssl):
* cc: me@… (added)
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:1>
Django
messages
Reporter: frasern | Owner: version2beta
Type: Bug | Status: assigned
Version: SVN | Severity: Normal
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
UI/UX: 0 |
------------------------------------+------------------------------
Changes (by version2beta):
* status: new => assigned
* owner: nobody => version2beta
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:2>
Django
messages
Reporter: frasern | Owner: version2beta
Version: SVN | Severity: Normal
Triage Stage: Accepted | Has patch: 1
UI/UX: 0 |
------------------------------------+------------------------------
Changes (by version2beta):
* status: assigned => closed
* has_patch: 0 => 1
* resolution: => fixed
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:3>
Django
messages
Reporter: frasern | Owner: version2beta
Version: SVN | Severity: Normal
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+------------------------------
Changes (by frasern):
* status: closed => reopened
* needs_better_patch: 0 => 1
* resolution: fixed =>
* needs_tests: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:4>
Django
messages
Reporter: frasern | Owner: version2beta
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by sperrygrove):
https://github.com/django/django/pull/2261
Makes value available to BaseValidator types for use in the validator
message.
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:6>
Django
messages
Reporter: frasern | Owner: version2beta
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by timo):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:7>
Django
messages
Reporter: frasern | Owner: anonymous
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Changes (by anonymous):
* status: new => assigned
* owner: version2beta => anonymous
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:8>
Django
messages
Reporter: frasern | Owner: sperrygrove
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Changes (by sperrygrove):
* owner: anonymous => sperrygrove
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:9>
Django
messages
Reporter: frasern | Owner: sperrygrove
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by anubhav9042):
@sperrygrove
You interested in finishing this?
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:10>
Django
messages
Reporter: frasern | Owner: sperrygrove
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by sperrygrove):
@anubhav9042 - please take it if you'd like to work on it.
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:11>
Django
messages
Reporter: frasern | Owner: anubhav9042
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anubhav9042):
* owner: sperrygrove => anubhav9042
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
https://github.com/django/django/pull/2958
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:12>
Django
messages
Reporter: frasern | Owner: anubhav9042
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by johmik):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:13>
Django
messages
Reporter: frasern | Owner: anubhav9042
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"5cdb8f8c1ea7161ca9e3b702ec7cdd7e3cd0999a"]:
{{{
#!CommitTicketReference repository=""
revision="5cdb8f8c1ea7161ca9e3b702ec7cdd7e3cd0999a"
Fixed #16617 -- Added 'value' to BaseValidator params.
Also allowed overriding the default messages in subclasses of
BaseValidator.
Thanks sperrygrove for initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16617#comment:14>