Code

Opened 3 years ago

Last modified 3 days ago

#16617 assigned Bug

URLField with min_length or max_length reports wrong length in validation messages

Reported by: frasern Owned by: sperrygrove
Component: Forms Version: master
Severity: Normal Keywords:
Cc: me@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
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)

length-validators.diff (1.5 KB) - added by version2beta 3 years ago.
Patch shows string rather than length for min|max length

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by BernhardEssl

  • Cc me@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by version2beta

  • Owner changed from nobody to version2beta
  • Status changed from new to assigned

Changed 3 years ago by version2beta

Patch shows string rather than length for min|max length

comment:3 Changed 3 years ago by version2beta

  • Has patch set
  • Resolution set to fixed
  • Status changed from assigned to 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 Changed 3 years ago by frasern

  • Needs tests set
  • Patch needs improvement set
  • Resolution fixed deleted
  • Status changed from closed to 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 Changed 16 months ago by aaugustin

  • Status changed from reopened to new

comment:6 Changed 5 months ago by sperrygrove

Pull request: https://github.com/django/django/pull/2261

Makes value available to BaseValidator types for use in the MinLengthValidator/MaxLengthValidator message.

Last edited 5 months ago by sperrygrove (previous) (diff)

comment:7 Changed 5 months ago 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?

comment:8 Changed 4 months ago by anonymous

  • Owner changed from version2beta to anonymous
  • Status changed from new to assigned

comment:9 Changed 4 months ago by sperrygrove

  • Owner changed from anonymous to sperrygrove

comment:10 Changed 3 days ago by anubhav9042

@sperrygrove
You interested in finishing this?

comment:11 Changed 3 days ago by sperrygrove

@anubhav9042 - please take it if you'd like to work on it.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from sperrygrove to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.