Skip to content

Conversation

@lovelydinosaur
Copy link
Contributor

CharField now only accepts string and numeric types, and will raise a validation error on other inputs.

Closes #3394.

@lovelydinosaur lovelydinosaur added this to the 3.4.4 Release milestone Aug 10, 2016
@lovelydinosaur lovelydinosaur changed the title Stricter CharField type validation. Stricter type validation for CharField. Aug 10, 2016
@lovelydinosaur lovelydinosaur merged commit f16e880 into master Aug 10, 2016
@lovelydinosaur lovelydinosaur deleted the stricter-charfield-type-validation branch August 10, 2016 16:22
@michael-k
Copy link
Contributor

Breaking change + minor version update 💥
But now we know about the issue in our code, .i.e. “eg. unclear if booleans should represent as true or True”.

@lovelydinosaur
Copy link
Contributor Author

lovelydinosaur commented Aug 15, 2016

Noted.

I'd treated it as not a breaking change, given that it'd only be exposed in already broken use-cases. (eg clients posting booleans to a CharField)

Still, it's evidently worth us being more conservative in the future, given the large installed base, now. Thanks for the data-point.

@sergiomb2
Copy link

How I override ? :

    if isinstance(data, bool) or not isinstance(data, six.string_types + six.integer_types + (float,)):

with

    if isinstance(data, bool) or not isinstance(data, six.string_types):

I don't want integers or floats be consider as a valid string .

Thanks

@michael-k
Copy link
Contributor

michael-k commented Jun 13, 2017

@sergiomb2 Derive a new class from CharField. Override to_internal_value by either copying the implementation and changing it, or by first performing your stricter check and then calling the parent's method.

Possibility 1:

class MyCharField(CharField):
    def to_internal_value(self, data):
        if isinstance(data, bool) or not isinstance(data, six.string_types):
            self.fail('invalid')
        value = six.text_type(data)
        return value.strip() if self.trim_whitespace else value

Possibility 2:

class MyCharField(CharField):
    def to_internal_value(self, data):
        if isinstance(data, bool) or not isinstance(data, six.string_types):
            self.fail('invalid')
        return super().to_internal_value(data)  # python 3 syntax

Do not forget to use your field in your serializers.

If you're using the ModelSerializer, you can derive from it and adjust serializer_field_mapping.

@sergiomb2
Copy link

sergiomb2 commented Jun 13, 2017

many thanks, yes I already use Possibility 1 , but Possibility 2 looks to me even better .
yes I already added this code (and to complete the example) :

from rest_framework import serializers 
from rest_framework.serializers import CharField

class CustomSerializer(serializers.ModelSerializer):
    user_name = MyCharField(max_length=45)

If you're using the ModelSerializer, you can derive from it and adjust serializer_field_mapping.

Could you exemplify what you mean ? I think will much more easy to me understand, yes I'd like override all CharFields in the project with MyCharField options , do I need reference all CharField in Serializers class ?

Many thanks for the feedback

@michael-k
Copy link
Contributor

class MyModelSerializer(ModelSerializer):
    serializer_field_mapping = {
        models.AutoField: IntegerField,
        models.BigIntegerField: IntegerField,
        models.BooleanField: BooleanField,
        models.CharField: MyCharField,  # ← see here
        models.CommaSeparatedIntegerField: MyCharField,  # ← see here
        models.DateField: DateField,
        models.DateTimeField: DateTimeField,
        models.DecimalField: DecimalField,
        models.EmailField: EmailField,
        models.Field: ModelField,
        models.FileField: FileField,
        models.FloatField: FloatField,
        models.ImageField: ImageField,
        models.IntegerField: IntegerField,
        models.NullBooleanField: NullBooleanField,
        models.PositiveIntegerField: IntegerField,
        models.PositiveSmallIntegerField: IntegerField,
        models.SlugField: SlugField,
        models.SmallIntegerField: IntegerField,
        models.TextField: MyCharField,  # ← see here
        models.TimeField: TimeField,
        models.URLField: URLField,
        models.GenericIPAddressField: IPAddressField,
        models.FilePathField: FilePathField,
    }

Then use MyModelSerializer instead.

@sergiomb2
Copy link

Thanks , exemplifying a more complete solution :

class CharField_stricter(serializers.CharField):
    def to_internal_value(self, data):
        if isinstance(data, bool) or not isinstance(data, six.string_types):
            self.fail('invalid')
        value = six.text_type(data)
        return value.strip() if self.trim_whitespace else value

class ModelSerializer_stricter(serializers.ModelSerializer):
    serializer_field_mapping = serializers.ModelSerializer.serializer_field_mapping.copy()
    serializer_field_mapping.update({
         models.CharField: CharField_stricter
    })

class FooSerializer(ModelSerializer_stricter):

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 this pull request may close these issues.

4 participants