Skip to content

Commit

Permalink
Prohibit null characters in CharField by default (#6073)
Browse files Browse the repository at this point in the history
* Implement an allow_null_bytes argument to CharField (default True)
* Switch to using native ProhibitNullCharactersValidator instead
  • Loading branch information
jleclanche authored and carltongibson committed Oct 2, 2018
1 parent 6618338 commit 0eb2dc1
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
5 changes: 5 additions & 0 deletions rest_framework/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
RegexURLResolver as URLResolver,
)

try:
from django.core.validators import ProhibitNullCharactersValidator # noqa
except ImportError:
ProhibitNullCharactersValidator = None


def get_original_route(urlpattern):
"""
Expand Down
9 changes: 7 additions & 2 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
from rest_framework import ISO_8601
from rest_framework.compat import (
MaxLengthValidator, MaxValueValidator, MinLengthValidator,
MinValueValidator, unicode_repr, unicode_to_repr
MinValueValidator, ProhibitNullCharactersValidator, unicode_repr,
unicode_to_repr
)
from rest_framework.exceptions import ErrorDetail, ValidationError
from rest_framework.settings import api_settings
Expand Down Expand Up @@ -755,7 +756,7 @@ class CharField(Field):
'invalid': _('Not a valid string.'),
'blank': _('This field may not be blank.'),
'max_length': _('Ensure this field has no more than {max_length} characters.'),
'min_length': _('Ensure this field has at least {min_length} characters.')
'min_length': _('Ensure this field has at least {min_length} characters.'),
}
initial = ''

Expand All @@ -778,6 +779,10 @@ def __init__(self, **kwargs):
self.validators.append(
MinLengthValidator(self.min_length, message=message))

# ProhibitNullCharactersValidator is None on Django < 2.0
if ProhibitNullCharactersValidator is not None:
self.validators.append(ProhibitNullCharactersValidator())

This comment has been minimized.

Copy link
@fengsi

fengsi Oct 26, 2018

Contributor

Unfortunately, if validators=(x, y, z) is passed (tuple instead of list), this append() would panic.

I know old code does append() too, but still a bummer for code passing tuple.

This comment has been minimized.

Copy link
@fengsi

fengsi Oct 26, 2018

Contributor

And, if we take a look at Django code:
https://github.com/django/django/blob/master/django/forms/fields.py#L59

It uses validators=(), changing from list to tuple by this commit:
django/django@9027e6c

Though in the comment it still says the old "list".

I believe DRF should consider using tuple by default as well, and detect tuple / list in code.
DRF does use self.validators = validators[:] to get a new object, however it also works for tuple.


def run_validation(self, data=empty):
# Test for the empty string here so that it does not get validated,
# and so that subclasses do not need to handle it explicitly
Expand Down
12 changes: 12 additions & 0 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import rest_framework
from rest_framework import exceptions, serializers
from rest_framework.compat import ProhibitNullCharactersValidator
from rest_framework.fields import DjangoImageField, is_simple_callable

try:
Expand Down Expand Up @@ -728,6 +729,17 @@ def test_disallow_blank_with_trim_whitespace(self):
field.run_validation(' ')
assert exc_info.value.detail == ['This field may not be blank.']

@pytest.mark.skipif(ProhibitNullCharactersValidator is None, reason="Skipped on Django < 2.0")
def test_null_bytes(self):
field = serializers.CharField()

for value in ('\0', 'foo\0', '\0foo', 'foo\0foo'):
with pytest.raises(serializers.ValidationError) as exc_info:
field.run_validation(value)
assert exc_info.value.detail == [
'Null characters are not allowed.'
]


class TestEmailField(FieldValues):
"""
Expand Down

0 comments on commit 0eb2dc1

Please sign in to comment.