diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index d33203bd8..b2b75f7a3 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -490,6 +490,40 @@ class SiteTestCase(APIViewTestCases.APIViewTestCase): site.refresh_from_db() self.assertEqual(site.description, '') + # A non-list body is described by its type, so that the client can see what was sent + self.assertEqual(response.data['detail'], 'Expected a list of objects, but got dict.') + + def test_bulk_write_objects_empty_body(self): + """ + Address a list endpoint with no body at all. An absent body reaches the bulk actions as an + empty dict, so it must not be reported as having "got dict" -- there is no object to describe. + """ + obj_perm = ObjectPermission(name='Test permission', actions=['change', 'delete']) + obj_perm.save() + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + initial_count = Site.objects.count() + + for method in ('patch', 'put', 'delete'): + with self.subTest(method=method): + response = getattr(self.client, method)(self._get_list_url(), **self.header) + + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.data['detail'], 'Expected a list of objects, but no data was submitted.' + ) + self.assertNotIn('errors', response.data) + + # An explicitly submitted empty object is indistinguishable, and reads the same way + response = self.client.patch(self._get_list_url(), {}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.data['detail'], 'Expected a list of objects, but no data was submitted.' + ) + + self.assertEqual(Site.objects.count(), initial_count, 'No objects should have been deleted') + def test_bulk_update_objects_non_numeric_id(self): """ PATCH a set of objects where one entry carries a non-numeric ID. The failure must be diff --git a/netbox/netbox/api/viewsets/mixins.py b/netbox/netbox/api/viewsets/mixins.py index f1c57c025..b84fae4a5 100644 --- a/netbox/netbox/api/viewsets/mixins.py +++ b/netbox/netbox/api/viewsets/mixins.py @@ -93,14 +93,18 @@ def get_non_list_response(data): if isinstance(data, list): return None - return Response( - { - 'detail': _('Expected a list of objects, but got {datatype}.').format( - datatype=type(data).__name__ - ), - }, - status=status.HTTP_400_BAD_REQUEST, - ) + # A request with no body at all arrives here as an empty dict, so reporting its type would tell + # the client only that it "got dict" -- unhelpful for what is the likeliest way to reach this + # point: a DELETE addressed to a list endpoint with nothing in the body. An explicitly submitted + # empty object is indistinguishable at this stage, and wants the same message anyway. + if data is None or data == {} or data == '': + detail = _('Expected a list of objects, but no data was submitted.') + else: + detail = _('Expected a list of objects, but got {datatype}.').format( + datatype=type(data).__name__ + ) + + return Response({'detail': detail}, status=status.HTTP_400_BAD_REQUEST) def get_invalid_entries_response(entry_errors): diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py index ee4c50c17..51422e82b 100644 --- a/netbox/utilities/testing/api.py +++ b/netbox/utilities/testing/api.py @@ -968,6 +968,33 @@ class APIViewTestCases: # No object named in the request may have been deleted self.assertEqual(self._get_queryset().count(), initial_count) + def test_bulk_delete_objects_no_body(self): + """ + DELETE a list endpoint with no body at all. Nothing may be deleted -- the request names no + objects, so it cannot mean "all of them" -- and the response must say so intelligibly. + """ + obj_perm = ObjectPermission( + name='Test permission', + actions=['delete'] + ) + obj_perm.save() + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + + initial_count = self._get_queryset().count() + self.assertNotEqual(initial_count, 0, 'No objects exist against which to test bulk deletion') + + response = self.client.delete(self._get_list_url(), **self.header) + + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertIn('detail', response.data) + # There are no entries to report against, so no per-object errors are returned + self.assertNotIn('errors', response.data) + self.assertEqual( + self._get_queryset().count(), initial_count, + 'A bulk delete naming no objects must not delete anything' + ) + class GraphQLTestCase(APITestCase): graphql_auto_filter_tests = True graphql_auto_filter_exclude = ()