From a7d6cab77138f23ef37ec14acb27cb263538e6a7 Mon Sep 17 00:00:00 2001 From: Sanyam Khurana <8039608+CuriousLearner@users.noreply.github.com> Date: Wed, 21 Nov 2018 03:49:13 +0530 Subject: [PATCH] Fixed #29282 -- Prevented some admin checks from crashing with TypeError. Co-authored-by: David Sanders --- AUTHORS | 1 + django/contrib/admin/checks.py | 34 ++++++++-- tests/modeladmin/test_checks.py | 113 ++++++++++++++++++++++++++++++-- 3 files changed, 136 insertions(+), 12 deletions(-) diff --git a/AUTHORS b/AUTHORS index a52ba34954..51bc2f592a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -748,6 +748,7 @@ answer newbie questions, and generally made Django that much better: Sam Newman Sander Dijkhuis Sanket Saurav + Sanyam Khurana Sarthak Mehrish schwank@gmail.com Scot Hacker diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 4007a781fb..3f8abdd476 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -20,6 +20,17 @@ from django.utils.deprecation import RemovedInDjango30Warning from django.utils.inspect import get_func_args +def _issubclass(cls, classinfo): + """ + issubclass() variant that doesn't raise an exception if cls isn't a + class. + """ + try: + return issubclass(cls, classinfo) + except TypeError: + return False + + def check_admin_app(app_configs, **kwargs): from django.contrib.admin.sites import all_sites errors = [] @@ -341,7 +352,7 @@ class BaseModelAdminChecks: def _check_form(self, obj): """ Check that form subclasses BaseModelForm. """ - if not issubclass(obj.form, BaseModelForm): + if not _issubclass(obj.form, BaseModelForm): return must_inherit_from(parent='BaseModelForm', option='form', obj=obj, id='admin.E016') else: @@ -640,11 +651,20 @@ class ModelAdminChecks(BaseModelAdminChecks): def _check_inlines_item(self, obj, inline, label): """ Check one inline model admin. """ - inline_label = inline.__module__ + '.' + inline.__name__ + try: + inline_label = inline.__module__ + '.' + inline.__name__ + except AttributeError: + return [ + checks.Error( + "'%s' must inherit from 'InlineModelAdmin'." % obj, + obj=obj.__class__, + id='admin.E104', + ) + ] from django.contrib.admin.options import InlineModelAdmin - if not issubclass(inline, InlineModelAdmin): + if not _issubclass(inline, InlineModelAdmin): return [ checks.Error( "'%s' must inherit from 'InlineModelAdmin'." % inline_label, @@ -660,7 +680,7 @@ class ModelAdminChecks(BaseModelAdminChecks): id='admin.E105', ) ] - elif not issubclass(inline.model, models.Model): + elif not _issubclass(inline.model, models.Model): return must_be('a Model', option='%s.model' % inline_label, obj=obj, id='admin.E106') else: return inline(obj.model, obj.admin_site).check() @@ -763,7 +783,7 @@ class ModelAdminChecks(BaseModelAdminChecks): if callable(item) and not isinstance(item, models.Field): # If item is option 3, it should be a ListFilter... - if not issubclass(item, ListFilter): + if not _issubclass(item, ListFilter): return must_inherit_from(parent='ListFilter', option=label, obj=obj, id='admin.E113') # ... but not a FieldListFilter. @@ -780,7 +800,7 @@ class ModelAdminChecks(BaseModelAdminChecks): elif isinstance(item, (tuple, list)): # item is option #2 field, list_filter_class = item - if not issubclass(list_filter_class, FieldListFilter): + if not _issubclass(list_filter_class, FieldListFilter): return must_inherit_from(parent='FieldListFilter', option='%s[1]' % label, obj=obj, id='admin.E115') else: return [] @@ -1041,7 +1061,7 @@ class InlineModelAdminChecks(BaseModelAdminChecks): def _check_formset(self, obj): """ Check formset is a subclass of BaseModelFormSet. """ - if not issubclass(obj.formset, BaseModelFormSet): + if not _issubclass(obj.formset, BaseModelFormSet): return must_inherit_from(parent='BaseModelFormSet', option='formset', obj=obj, id='admin.E206') else: return [] diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index 89fde35d3c..a1b7001f68 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -225,11 +225,16 @@ class FormCheckTests(CheckTestCase): class TestModelAdmin(ModelAdmin): form = FakeForm - self.assertIsInvalid( - TestModelAdmin, ValidationTestModel, - "The value of 'form' must inherit from 'BaseModelForm'.", - 'admin.E016' - ) + class TestModelAdminWithNoForm(ModelAdmin): + form = 'not a form' + + for model_admin in (TestModelAdmin, TestModelAdminWithNoForm): + with self.subTest(model_admin): + self.assertIsInvalid( + model_admin, ValidationTestModel, + "The value of 'form' must inherit from 'BaseModelForm'.", + 'admin.E016' + ) def test_fieldsets_with_custom_form_validation(self): @@ -598,6 +603,40 @@ class ListFilterTests(CheckTestCase): 'admin.E112' ) + def test_not_list_filter_class(self): + class TestModelAdmin(ModelAdmin): + list_filter = ['RandomClass'] + + self.assertIsInvalid( + TestModelAdmin, ValidationTestModel, + "The value of 'list_filter[0]' refers to 'RandomClass', which " + "does not refer to a Field.", + 'admin.E116' + ) + + def test_callable(self): + def random_callable(): + pass + + class TestModelAdmin(ModelAdmin): + list_filter = [random_callable] + + self.assertIsInvalid( + TestModelAdmin, ValidationTestModel, + "The value of 'list_filter[0]' must inherit from 'ListFilter'.", + 'admin.E113' + ) + + def test_not_callable(self): + class TestModelAdmin(ModelAdmin): + list_filter = [[42, 42]] + + self.assertIsInvalid( + TestModelAdmin, ValidationTestModel, + "The value of 'list_filter[0][1]' must inherit from 'FieldListFilter'.", + 'admin.E115' + ) + def test_missing_field(self): class TestModelAdmin(ModelAdmin): list_filter = ('non_existent_field',) @@ -655,6 +694,19 @@ class ListFilterTests(CheckTestCase): 'admin.E115' ) + def test_list_filter_is_func(self): + def get_filter(): + pass + + class TestModelAdmin(ModelAdmin): + list_filter = [get_filter] + + self.assertIsInvalid( + TestModelAdmin, ValidationTestModel, + "The value of 'list_filter[0]' must inherit from 'ListFilter'.", + 'admin.E113' + ) + def test_not_associated_with_field_name(self): class TestModelAdmin(ModelAdmin): list_filter = (BooleanFieldListFilter,) @@ -918,6 +970,16 @@ class InlinesCheckTests(CheckTestCase): 'admin.E103' ) + def test_not_correct_inline_field(self): + class TestModelAdmin(ModelAdmin): + inlines = [42] + + self.assertIsInvalidRegexp( + TestModelAdmin, ValidationTestModel, + r"'.*\.TestModelAdmin' must inherit from 'InlineModelAdmin'\.", + 'admin.E104' + ) + def test_not_model_admin(self): class ValidationTestInline: pass @@ -960,6 +1022,32 @@ class InlinesCheckTests(CheckTestCase): 'admin.E106' ) + def test_invalid_model(self): + class ValidationTestInline(TabularInline): + model = 'Not a class' + + class TestModelAdmin(ModelAdmin): + inlines = [ValidationTestInline] + + self.assertIsInvalidRegexp( + TestModelAdmin, ValidationTestModel, + r"The value of '.*\.ValidationTestInline.model' must be a Model\.", + 'admin.E106' + ) + + def test_invalid_callable(self): + def random_obj(): + pass + + class TestModelAdmin(ModelAdmin): + inlines = [random_obj] + + self.assertIsInvalidRegexp( + TestModelAdmin, ValidationTestModel, + r"'.*\.random_obj' must inherit from 'InlineModelAdmin'\.", + 'admin.E104' + ) + def test_valid_case(self): class ValidationTestInline(TabularInline): model = ValidationTestInlineModel @@ -1102,6 +1190,21 @@ class FormsetCheckTests(CheckTestCase): invalid_obj=ValidationTestInline ) + def test_inline_without_formset_class(self): + class ValidationTestInlineWithoutFormsetClass(TabularInline): + model = ValidationTestInlineModel + formset = 'Not a FormSet Class' + + class TestModelAdminWithoutFormsetClass(ModelAdmin): + inlines = [ValidationTestInlineWithoutFormsetClass] + + self.assertIsInvalid( + TestModelAdminWithoutFormsetClass, ValidationTestModel, + "The value of 'formset' must inherit from 'BaseModelFormSet'.", + 'admin.E206', + invalid_obj=ValidationTestInlineWithoutFormsetClass + ) + def test_valid_case(self): class RealModelFormSet(BaseModelFormSet): pass