Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializer field raises TypeError instead of ValidationError #474

Closed
videda opened this issue Nov 11, 2021 · 2 comments · Fixed by #523
Closed

Serializer field raises TypeError instead of ValidationError #474

videda opened this issue Nov 11, 2021 · 2 comments · Fixed by #523
Assignees
Labels

Comments

@videda
Copy link

videda commented Nov 11, 2021

I noticed that when validating a phone number a TypeError is raised. Comparing this to the EmailField where a ValidationError is raised. I would expect also a ValidationError. Is this behaviour intended?

from rest_framework import serializers
from phonenumber_field.serializerfields import PhoneNumberField

class EmailSerializer(serializers.Serializer):
    email = serializers.EmailField()

s = EmailSerializer(data={'email': 0})
print('Email valid:', s.is_valid())

Output:

Email valid: False

from rest_framework import serializers
from phonenumber_field.serializerfields import PhoneNumberField

class PhoneSerializer(serializers.Serializer):
    phone = PhoneNumberField()

s = PhoneSerializer(data={'phone': 0})
print('Phone valid:', s.is_valid())

Output:

Traceback (most recent call last):
File "/usr/lib/python3.9/code.py", line 90, in runcode
exec(code, self.locals)
File "", line 1, in
File "/path/to/test_phonenumberfield.py", line 16, in
print('Is phone valid:', s.is_valid())
File "/path/to/venv/lib/python3.9/site-packages/rest_framework/serializers.py", line 220, in is_valid
self._validated_data = self.run_validation(self.initial_data)
File "/path/to/venv/lib/python3.9/site-packages/rest_framework/serializers.py", line 419, in run_validation
value = self.to_internal_value(data)
File "/path/to/venv/lib/python3.9/site-packages/rest_framework/serializers.py", line 476, in to_internal_value
validated_value = field.run_validation(primitive_value)
File "/path/to/venv/lib/python3.9/site-packages/rest_framework/fields.py", line 799, in run_validation
return super().run_validation(data)
File "/path/to/venv/lib/python3.9/site-packages/rest_framework/fields.py", line 568, in run_validation
value = self.to_internal_value(data)
File "/path/to/venv/lib/python3.9/site-packages/phonenumber_field/serializerfields.py", line 12, in to_internal_value
phone_number = to_python(data)
File "/path/to/venv/lib/python3.9/site-packages/phonenumber_field/phonenumber.py", line 149, in to_python
raise TypeError("Can't convert %s to PhoneNumber." % type(value).name)
TypeError: Can't convert int to PhoneNumber.

I found some related issue but none seems to cover this: #306, #225, #389, #265
Sorry if I missed the relevant discussion

@francoisfreitag
Copy link
Collaborator

That’s indeed a bug. A TypeError is not intended, the serializer should be able to safely validate input. Otherwise, it leads to crashes.

The to_python() method expects its input to be a string. It works well with urlencoded data received with a regular form post, because the data is a string, but not so well with DRF, as DRF accepts richer representations (JSON, XML, etc).
IMO, the serializer should make sure to represent the data as string before handling it to the to_python helper.

@videda
Copy link
Author

videda commented Nov 17, 2021

For the moment I am using the following custom field

class CustomPhoneNumberField(serializers.CharField):
    default_error_messages = {"invalid": _("Enter a valid phone number.")}

    def to_internal_value(self, data):
        _data = data if isinstance(data, phonenumbers.PhoneNumber) else super().to_internal_value(data)
        phone_number = to_python(_data)
        if phone_number and not phone_number.is_valid():
            raise ValidationError(self.error_messages["invalid"])
        return phone_number

So if data is not a PhoneNumber instance the CharField.to_internal_value is used to return a string. This way the
else-case in to_python is never reached and the TypeError is not raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants