Fixed #28577 -- Added checks for ArrayField and JSONField to prevent mutable defaults.

This commit is contained in:
Flávio Juvenal 2017-08-17 16:21:35 -07:00 committed by Tim Graham
parent 5ceaf14686
commit f6e1789654
8 changed files with 112 additions and 6 deletions

View File

@ -263,6 +263,7 @@ answer newbie questions, and generally made Django that much better:
Filip Noetzel <http://filip.noetzel.co.uk/> Filip Noetzel <http://filip.noetzel.co.uk/>
Filip Wasilewski <filip.wasilewski@gmail.com> Filip Wasilewski <filip.wasilewski@gmail.com>
Finn Gruwier Larsen <finn@gruwier.dk> Finn Gruwier Larsen <finn@gruwier.dk>
Flávio Juvenal da Silva Junior <flavio@vinta.com.br>
flavio.curella@gmail.com flavio.curella@gmail.com
Florian Apolloner <florian@apolloner.eu> Florian Apolloner <florian@apolloner.eu>
Francisco Albarran Cristobal <pahko.xd@gmail.com> Francisco Albarran Cristobal <pahko.xd@gmail.com>

View File

@ -10,17 +10,19 @@ from django.utils.inspect import func_supports_parameter
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from ..utils import prefix_validation_error from ..utils import prefix_validation_error
from .mixins import CheckFieldDefaultMixin
from .utils import AttributeSetter from .utils import AttributeSetter
__all__ = ['ArrayField'] __all__ = ['ArrayField']
class ArrayField(Field): class ArrayField(CheckFieldDefaultMixin, Field):
empty_strings_allowed = False empty_strings_allowed = False
default_error_messages = { default_error_messages = {
'item_invalid': _('Item %(nth)s in the array did not validate: '), 'item_invalid': _('Item %(nth)s in the array did not validate: '),
'nested_array_mismatch': _('Nested arrays must have the same length.'), 'nested_array_mismatch': _('Nested arrays must have the same length.'),
} }
_default_hint = ('list', '[]')
def __init__(self, base_field, size=None, **kwargs): def __init__(self, base_field, size=None, **kwargs):
self.base_field = base_field self.base_field = base_field

View File

@ -9,6 +9,8 @@ from django.db.models import (
) )
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from .mixins import CheckFieldDefaultMixin
__all__ = ['JSONField'] __all__ = ['JSONField']
@ -25,12 +27,13 @@ class JsonAdapter(Json):
return json.dumps(obj, **options) return json.dumps(obj, **options)
class JSONField(Field): class JSONField(CheckFieldDefaultMixin, Field):
empty_strings_allowed = False empty_strings_allowed = False
description = _('A JSON object') description = _('A JSON object')
default_error_messages = { default_error_messages = {
'invalid': _("Value must be valid JSON."), 'invalid': _("Value must be valid JSON."),
} }
_default_hint = ('dict', '{}')
def __init__(self, verbose_name=None, name=None, encoder=None, **kwargs): def __init__(self, verbose_name=None, name=None, encoder=None, **kwargs):
if encoder and not callable(encoder): if encoder and not callable(encoder):

View File

@ -0,0 +1,29 @@
from django.core import checks
class CheckFieldDefaultMixin:
_default_hint = ('<valid default>', '<invalid default>')
def _check_default(self):
if self.has_default() and self.default is not None and not callable(self.default):
return [
checks.Warning(
"%s default should be a callable instead of an instance so "
"that it's not shared between all field instances." % (
self.__class__.__name__,
),
hint=(
'Use a callable instead, e.g., use `%s` instead of '
'`%s`.' % self._default_hint
),
obj=self,
id='postgres.E003',
)
]
else:
return []
def check(self, **kwargs):
errors = super().check(**kwargs)
errors.extend(self._check_default())
return errors

View File

@ -687,6 +687,8 @@ fields:
* **postgres.E001**: Base field for array has errors: ... * **postgres.E001**: Base field for array has errors: ...
* **postgres.E002**: Base field for array cannot be a related field. * **postgres.E002**: Base field for array cannot be a related field.
* **postgres.E003**: ``<field>`` default should be a callable instead of an
instance so that it's not shared between all field instances.
``sites`` ``sites``
--------- ---------

View File

@ -41,7 +41,7 @@ class PostgreSQLModel(models.Model):
class IntegerArrayModel(PostgreSQLModel): class IntegerArrayModel(PostgreSQLModel):
field = ArrayField(models.IntegerField(), default=[], blank=True) field = ArrayField(models.IntegerField(), default=list, blank=True)
class NullableIntegerArrayModel(PostgreSQLModel): class NullableIntegerArrayModel(PostgreSQLModel):

View File

@ -4,7 +4,7 @@ import unittest
import uuid import uuid
from django import forms from django import forms
from django.core import exceptions, serializers, validators from django.core import checks, exceptions, serializers, validators
from django.core.exceptions import FieldError from django.core.exceptions import FieldError
from django.core.management import call_command from django.core.management import call_command
from django.db import IntegrityError, connection, models from django.db import IntegrityError, connection, models
@ -424,6 +424,38 @@ class TestChecks(PostgreSQLTestCase):
self.assertEqual(len(errors), 1) self.assertEqual(len(errors), 1)
self.assertEqual(errors[0].id, 'postgres.E002') self.assertEqual(errors[0].id, 'postgres.E002')
def test_invalid_default(self):
class MyModel(PostgreSQLModel):
field = ArrayField(models.IntegerField(), default=[])
model = MyModel()
self.assertEqual(model.check(), [
checks.Warning(
msg=(
"ArrayField default should be a callable instead of an "
"instance so that it's not shared between all field "
"instances."
),
hint='Use a callable instead, e.g., use `list` instead of `[]`.',
obj=MyModel._meta.get_field('field'),
id='postgres.E003',
)
])
def test_valid_default(self):
class MyModel(PostgreSQLModel):
field = ArrayField(models.IntegerField(), default=list)
model = MyModel()
self.assertEqual(model.check(), [])
def test_valid_default_none(self):
class MyModel(PostgreSQLModel):
field = ArrayField(models.IntegerField(), default=None)
model = MyModel()
self.assertEqual(model.check(), [])
def test_nested_field_checks(self): def test_nested_field_checks(self):
""" """
Nested ArrayFields are permitted. Nested ArrayFields are permitted.

View File

@ -2,13 +2,14 @@ import datetime
import uuid import uuid
from decimal import Decimal from decimal import Decimal
from django.core import exceptions, serializers from django.core import checks, exceptions, serializers
from django.core.serializers.json import DjangoJSONEncoder from django.core.serializers.json import DjangoJSONEncoder
from django.forms import CharField, Form, widgets from django.forms import CharField, Form, widgets
from django.test.utils import isolate_apps
from django.utils.html import escape from django.utils.html import escape
from . import PostgreSQLTestCase from . import PostgreSQLTestCase
from .models import JSONModel from .models import JSONModel, PostgreSQLModel
try: try:
from django.contrib.postgres import forms from django.contrib.postgres import forms
@ -259,6 +260,42 @@ class TestQuerying(PostgreSQLTestCase):
self.assertTrue(JSONModel.objects.filter(field__foo__iregex=r'^bAr$').exists()) self.assertTrue(JSONModel.objects.filter(field__foo__iregex=r'^bAr$').exists())
@isolate_apps('postgres_tests')
class TestChecks(PostgreSQLTestCase):
def test_invalid_default(self):
class MyModel(PostgreSQLModel):
field = JSONField(default={})
model = MyModel()
self.assertEqual(model.check(), [
checks.Warning(
msg=(
"JSONField default should be a callable instead of an "
"instance so that it's not shared between all field "
"instances."
),
hint='Use a callable instead, e.g., use `dict` instead of `{}`.',
obj=MyModel._meta.get_field('field'),
id='postgres.E003',
)
])
def test_valid_default(self):
class MyModel(PostgreSQLModel):
field = JSONField(default=dict)
model = MyModel()
self.assertEqual(model.check(), [])
def test_valid_default_none(self):
class MyModel(PostgreSQLModel):
field = JSONField(default=None)
model = MyModel()
self.assertEqual(model.check(), [])
class TestSerialization(PostgreSQLTestCase): class TestSerialization(PostgreSQLTestCase):
test_data = ( test_data = (
'[{"fields": {"field": {"a": "b", "c": null}, "field_custom": null}, ' '[{"fields": {"field": {"a": "b", "c": null}, "field_custom": null}, '