[Django] #34385: BaseTemporalField child fields causing AttributeError to be raised by calling form.is_valid().

3 views
Skip to first unread message

Django

unread,
Mar 3, 2023, 10:49:52 AM3/3/23
to django-...@googlegroups.com
#34385: BaseTemporalField child fields causing AttributeError to be raised by
calling form.is_valid().
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
FabioDavidF |
Type: Bug | Status: new
Component: Forms | Version: 4.1
Severity: Normal | Keywords: DateField
| DateTimeField validation
Triage Stage: | AttributeError
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When validating a form with a temporal field that has been passed a non
`str` native value, `AttributeError` is raised.

The problem happens in `DateField`, `DateTimeField`, and `TimeField`
fields. The respective temporal field's `to_python` method treats the
input assuming it is of type `str`, `datetime.datetime`, `datetime.date`,
or `datetime.timedelta`. So when something like an integer is fed to a
field and `clean` is called in the validation process, `AttributeError` is
raised instead of a `ValidationError`, causing the form not to catch the
error since it only catches `ValidationError` instances, and, therefore,
raising an unexpected exception from a, in my opinion, validation error.

=== Reproducing the problem ===
You can test this through the form validation:
{{{
from django import forms

class MyForm(forms.Form):
datefield = forms.DateField()

form = MyForm({'datefield': 1})
form.is_valid()
------------------------------------------------------------------------------------
> def to_python(self, value):
> value = value.strip()
E AttributeError: 'int' object has no attribute 'strip'
}}}

Or directly with a field:
{{{
from django.forms import DateField

field = DateField()
field.to_python(1)
------------------------------------------------------------------------------------
> def to_python(self, value):
> value = value.strip()
E AttributeError: 'int' object has no attribute 'strip'
}}}

Note that through the form validation process the problem is more
apparent, since when calling `is_valid`, we do not expect an exception
raised from said validation, we expect the form to store its validation
errors so we can handle them directly. Passing an integer to a
`DateField`, however, causes an exception to be raised when validating a
form.

=== Test case ===
{{{
from django import forms
from unittest import TestCase


class TemporalFieldValidationTests(TestCase):
def
test_date_field_raises_validationerror_when_value_is_not_str_or_temporal_object(self):
date_field = forms.DateField()

with self.assertRaises(forms.ValidationError):
date_field.clean(1)

def
test_time_field_raises_validationerror_when_value_is_not_str_or_temporal_object(self):
time_field = forms.TimeField

with self.assertRaises(forms.ValidationError):
time_field.clean(1)

def
test_datetime_field_raises_validationerror_when_value_is_not_str_or_temporal_object(self):
datetime_field = forms.DateTimeField()

with self.assertRaises(forms.ValidationError):
datetime_field.clean(1)
}}}

=== Considerations ===
- Tested on versions `3.2.16`, and `4.1.7`
- I will open a PR to solve this

--
Ticket URL: <https://code.djangoproject.com/ticket/34385>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 3, 2023, 11:15:46 AM3/3/23
to django-...@googlegroups.com
#34385: BaseTemporalField child fields causing AttributeError to be raised by
calling form.is_valid().
-------------------------------------+-------------------------------------
Reporter: FabioDavidF | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 4.1
Severity: Normal | Resolution: wontfix
Keywords: DateField | Triage Stage:
DateTimeField validation | Unreviewed
AttributeError |

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => wontfix


Comment:

Thanks for this ticket, however I don't see anything wrong in this
behavior, `to_python()` accepts the raw value from the widget i.e. string,
and it's not designed to accept anything and never crash. Can you explain
how you ended up with passing `int` to this method?

--
Ticket URL: <https://code.djangoproject.com/ticket/34385#comment:1>

Django

unread,
Mar 3, 2023, 1:28:14 PM3/3/23
to django-...@googlegroups.com
#34385: BaseTemporalField child fields causing AttributeError to be raised by
calling form.is_valid().
-------------------------------------+-------------------------------------
Reporter: Fábio David Freitas | Owner: nobody

Type: Bug | Status: closed
Component: Forms | Version: 4.1
Severity: Normal | Resolution: wontfix
Keywords: DateField | Triage Stage:
DateTimeField validation | Unreviewed
AttributeError |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Fábio David Freitas):

Replying to [comment:1 Mariusz Felisiak]:


> Thanks for this ticket, however I don't see anything wrong in this
behavior, `to_python()` accepts the raw value from the widget i.e. string,
and it's not designed to accept anything and never crash. Can you explain
how you ended up with passing `int` to this method?

Thanks for the response, that makes sense. What I did in my code to get
this behavior is basically the first code section of `Reproducing the
problem` from the ticket, which is creating a form with a `DateField` and
populating it with data when instantiating the form. I am only using the
form for validation, but the actual input comes from a JSON request.

From your response I am getting the impression that django forms are only
supposed to be used when it's gonna be populated by the form itself on the
template. Is that the case? I was under the impression that forms could be
used as a "Struct with validation and streamlined model access".

--
Ticket URL: <https://code.djangoproject.com/ticket/34385#comment:2>

Reply all
Reply to author
Forward
0 new messages