fix(api): correctly handle POST/PUT/PATCH with empty payload

api/desecapi/serializers.py

@@ -227,6 +227,8 @@ class RRsetSerializer(ConditionalExistenceModelSerializer):
     @classmethod
     def many_init(cls, *args, **kwargs):
         domain = kwargs.pop('domain')
+        # Note: We are not yet deciding the value of the child's "partial" attribute, as its value depends on whether
+        # the RRSet is created (never partial) or not (partial if PATCH), for each given item (RRset) individually.
         kwargs['child'] = cls(domain=domain)
         return RRsetListSerializer(*args, **kwargs)
 

api/desecapi/tests/base.py

@@ -65,10 +65,12 @@ class DesecAPIClient(APIClient):
         )
 
     def post_rr_set(self, domain_name, **kwargs):
-        kwargs.setdefault('ttl', 60)
+        data = kwargs or None
+        if data:
+            data.setdefault('ttl', 60)
         return self.post(
             self.reverse('v1:rrsets', name=domain_name),
-            kwargs,
+            data=data,
         )
 
     def get_rr_sets(self, domain_name, **kwargs):
@@ -850,6 +852,11 @@ class DesecTestCase(MockPDNSTestCase):
                 rr_sets_needle, rr_sets_haystack
             ))
 
+    def assertContains(self, response, text, count=None, status_code=200, msg_prefix='', html=False):
+        # convenience method to check the status separately, which yields nicer error messages
+        self.assertStatus(response, status_code)
+        return super().assertContains(response, text, count, status_code, msg_prefix, html)
+
 
 class PublicSuffixMockMixin():
     def _mock_get_public_suffix(self, domain_name, public_suffixes=None):

api/desecapi/tests/test_rrsets.py

@@ -213,6 +213,10 @@ class AuthenticatedRRSetTestCase(AuthenticatedRRSetBaseTestCase):
             self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
             self.assertIn('Subname can only use (lowercase)', str(response.data))
 
+    def test_create_my_rr_sets_empty_payload(self):
+        response = self.client.post_rr_set(self.my_empty_domain.name)
+        self.assertContains(response, 'No data provided', status_code=status.HTTP_400_BAD_REQUEST)
+
     def test_create_my_rr_sets_unknown_type(self):
         for _type in ['AA', 'ASDF']:
             with self.assertPdnsRequests(

api/desecapi/tests/test_rrsets_bulk.py

@@ -142,6 +142,12 @@ class AuthenticatedRRSetBulkTestCase(AuthenticatedRRSetBaseTestCase):
             ]
         )
 
+    def test_bulk_post_accepts_empty_list(self):
+        self.assertResponse(
+            self.client.bulk_post_rr_sets(domain_name=self.my_empty_domain.name, payload=[]),
+            status.HTTP_201_CREATED,
+        )
+
     def test_bulk_patch_fresh_rrsets_need_records(self):
         response = self.client.bulk_patch_rr_sets(self.my_empty_domain.name, payload=self.data_no_records)
         self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
@@ -169,6 +175,14 @@ class AuthenticatedRRSetBulkTestCase(AuthenticatedRRSetBaseTestCase):
         response = self.client.bulk_patch_rr_sets(domain_name=self.my_empty_domain.name, payload=self.data[0])
         self.assertContains(response, 'Expected a list of items but got dict.', status_code=status.HTTP_400_BAD_REQUEST)
 
+    def test_bulk_patch_does_accept_empty_list(self):
+        response = self.client.bulk_patch_rr_sets(domain_name=self.my_empty_domain.name, payload=[])
+        self.assertStatus(response, status.HTTP_200_OK)
+
+    def test_bulk_patch_does_not_accept_empty_payload(self):
+        response = self.client.bulk_patch_rr_sets(domain_name=self.my_empty_domain.name, payload=None)
+        self.assertContains(response, 'No data provided', status_code=status.HTTP_400_BAD_REQUEST)
+
     def test_bulk_patch_full_on_empty_domain(self):
         # Full patch always works
         with self.assertPdnsRequests(self.requests_desec_rr_sets_update(name=self.my_empty_domain.name)):
@@ -311,6 +325,14 @@ class AuthenticatedRRSetBulkTestCase(AuthenticatedRRSetBaseTestCase):
         response = self.client.bulk_put_rr_sets(domain_name=self.my_empty_domain.name, payload=self.data[0])
         self.assertContains(response, 'Expected a list of items but got dict.', status_code=status.HTTP_400_BAD_REQUEST)
 
+    def test_bulk_put_does_accept_empty_list(self):
+        response = self.client.bulk_put_rr_sets(domain_name=self.my_empty_domain.name, payload=[])
+        self.assertStatus(response, status.HTTP_200_OK)
+
+    def test_bulk_put_does_not_accept_empty_payload(self):
+        response = self.client.bulk_put_rr_sets(domain_name=self.my_empty_domain.name, payload=None)
+        self.assertContains(response, 'No data provided', status_code=status.HTTP_400_BAD_REQUEST)
+
     def test_bulk_put_does_not_accept_list_of_crap(self):
         response = self.client.bulk_put_rr_sets(domain_name=self.my_empty_domain.name, payload=['bla'])
         self.assertContains(response, 'Expected a dictionary, but got str.', status_code=status.HTTP_400_BAD_REQUEST)
@@ -371,3 +393,12 @@ class AuthenticatedRRSetBulkTestCase(AuthenticatedRRSetBaseTestCase):
             response = method(domain_name=self.my_empty_domain.name, payload=self.data[0])
             self.assertContains(response, 'Expected a list of items but got dict.',
                                 status_code=status.HTTP_400_BAD_REQUEST)
+
+    def test_bulk_delete_rrsets(self):
+        self.assertStatus(
+            self.client.delete(
+                self.reverse('v1:rrsets', name=self.my_empty_domain.name),
+                data=None,
+            ),
+            status.HTTP_405_METHOD_NOT_ALLOWED,
+        )

api/desecapi/views.py

@@ -30,6 +30,24 @@ from desecapi.permissions import IsDomainOwner, IsOwner, IsVPNClient, WithinDoma
 from desecapi.renderers import PlainTextRenderer
 
 
+class EmptyPayloadMixin:
+    def initialize_request(self, request, *args, **kwargs):
+        request = super().initialize_request(request, *args, **kwargs)
+
+        try:
+            no_data = request.stream is None
+        except:
+            no_data = True
+
+        if no_data:
+            # In this case, data and files are both empty, so we can set request.data=None (instead of the default {}).
+            # This allows distinguishing missing payload from empty dict payload.
+            # See https://github.com/encode/django-rest-framework/pull/7195
+            request._full_data = None
+
+        return request
+
+
 class IdempotentDestroyMixin:
 
     def destroy(self, request, *args, **kwargs):
@@ -174,7 +192,7 @@ class RRsetDetail(IdempotentDestroyMixin, DomainViewMixin, generics.RetrieveUpda
             super().perform_destroy(instance)
 
 
-class RRsetList(DomainViewMixin, generics.ListCreateAPIView, generics.UpdateAPIView):
+class RRsetList(EmptyPayloadMixin, DomainViewMixin, generics.ListCreateAPIView, generics.UpdateAPIView):
     serializer_class = serializers.RRsetSerializer
     permission_classes = (IsAuthenticated, IsDomainOwner,)
 
@@ -206,12 +224,13 @@ class RRsetList(DomainViewMixin, generics.ListCreateAPIView, generics.UpdateAPIV
 
     def get_serializer(self, *args, **kwargs):
         kwargs = kwargs.copy()
-        data = kwargs.get('data')
-        if data and 'many' not in kwargs:
-            if self.request.method == 'POST':
-                kwargs['many'] = isinstance(data, list)
+
+        if 'many' not in kwargs:
+            if self.request.method in ['POST']:
+                kwargs['many'] = isinstance(kwargs.get('data'), list)
             elif self.request.method in ['PATCH', 'PUT']:
                 kwargs['many'] = True
+
         return super().get_serializer(domain=self.domain, *args, **kwargs)
 
     def perform_create(self, serializer):