Hi @jleclanche - thanks for the contribution. My gut reaction is that this is straightforward enough to be implemented as a validator function and doesn't need to exist in core. That said, I'd be interested in what others have to say.

...this is straightforward enough to be implemented as a validator function...

Absolutely. (Especially vs introducing a new kwarg.)

@carltongibson @rpkilby Part of the reason I went this route rather than adding a validator was to potentially discuss making this default behaviour for CharField.

As most databases refuse null bytes in varchar and text, and most serializers.CharField instances end up in a database one way or another, I suspect most serializers in the wild out there do not correctly validate against null bytes.

I’d be okay with us defaulting to False for 3.9. Anyone else?

Also we’re using six.text_type multiple times over - it’d be nice if we could finesse that a little.

Fixed the tests and rebased on master.

Django ships with a validator for this:

https://github.com/django/django/blob/38e904e26576abffed3c257a1a741e1641c6f2de/django/core/validators.py#L518

If we’re going to do this we should use that.

forms.Charfield applies this validator by default, without a kwarg option:

https://github.com/django/django/blob/38e904e26576abffed3c257a1a741e1641c6f2de/django/forms/fields.py#L218

My initial response would be to advise doing just that in a subclass.

If not, i.e. if there’s a case for us doing this too, then do we really have to have the kwarg? It seems we’d be saying “you really should do this but we we're not going to turn it on for you”.

(In this case people needing the old behaviour would need the subclass. But isn’t the point that we should do this ourselves just that you wouldn’t need the old behaviour...?)

I guess I suspect most people are never hitting this, or we’d have a million reports over the years of unexpected 500s.

I'd be +1 for following Django's lead here and just adding the validator by default and leaving off the kwarg option. The breaking change in 3.9 would also be acceptable to me, given how minor it is.

What pushes me towards adding this as a default behavior is:

1. Django supports this.
2. As @jleclanche stated, this is the correct thing to do from a database perspective.

If users need to override, they can pop the validator from the list (note: we should probably provide a terse example of that in the changelog).

Fixed the tests and rebased on master.

GitHub doesn't allow you to reopen a closed PR after the branch has been rebased. @jleclanche - could you submit a new PR?

I'll work on the changes and create a new PR in the next few hours.

Great! Thanks.

@rpkilby Just for clarification, should I be reusing django's ProhibitNullCharactersValidator or will I be reimplementing it in drf? I'm not too familiar with validators as is.

Django Validators are compatible with DRF, so I would use ProhibitNullCharactersValidator. We already use a few of them.

One thing to note is that importing the validator will fail for Django 1.11, as it only exists in Django 2.0 and later. Two ways to handle this:

• You can add the validator to CharField for only Django 2.0 and above.
• You can attempt to import the validator in compat, then define it if the import fails. eg,

  try:
      from django.core.validators import ProhibitNullCharactersValidator
  except ImportError:  # Django 1.11
      class ProhibitNullCharactersValidator(object):
          # Copied from Django 
          ...

Oh hum.

I suppose I'm okay validating this only on Django 2.0+, seeing as it would actually follow Django's behaviour then. Thoughts?

I would be fine with that, as it matches Django's behavior.

It also means less code to cleanup later.

I'd be +1 for following Django's lead here and just adding the validator by default and leaving off the kwarg option. The breaking change in 3.9 would also be acceptable to me, given how minor it is.

Sounds good.

I suppose I'm okay validating this only on Django 2.0+, seeing as it would actually follow Django's behaviour then. Thoughts?

Either way would be acceptable.

Followup in #6073