fix(api): avoid using regex OR operator, remove '@' special case

api/desecapi/models.py

@@ -163,11 +163,8 @@ class Domain(models.Model, mixins.SetterMixin):
         if '/' in self.name or '?' in self.name:
             raise SuspiciousOperation('Invalid hostname ' + self.name)
 
-        # Transform to be valid pdns API identifiers (:id in their docs).  The
-        # '/' case here is just a safety measure (this case should never occur due
-        # to the above check).
         # See also pdns code, apiZoneNameToId() in ws-api.cc
-        name = self.name.translate(str.maketrans({'/': '=2F', '_': '=5F'}))
+        name = self.name.translate(str.maketrans({'_': '=5F'}))
 
         if not name.endswith('.'):
             name += '.'

api/desecapi/tests/testrrsets.py

@@ -226,7 +226,7 @@ class AuthenticatedRRsetTests(APITestCase):
         response = self.client.post(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '', 'A',))
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['records'][0], '1.2.3.4')
@@ -329,7 +329,7 @@ class AuthenticatedRRsetTests(APITestCase):
         response = self.client.post(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '', 'A',))
 
         data = {'records': ['2.2.3.4'], 'ttl': 30}
         response = self.client.put(url, json.dumps(data), content_type='application/json')
@@ -393,7 +393,7 @@ class AuthenticatedRRsetTests(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
         # Change records and TTL
-        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '', 'A',))
         data = {'records': ['3.2.3.4'], 'ttl': 32}
         response = self.client.patch(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -449,7 +449,7 @@ class AuthenticatedRRsetTests(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
         self.client.credentials(HTTP_AUTHORIZATION='Token ' + self.token)
-        url = reverse('v1:rrset@', args=(self.otherDomains[0].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.otherDomains[0].name, '', 'A',))
         data = {'records': ['3.2.3.4'], 'ttl': 30, 'type': 'A'}
 
         response = self.client.patch(url, json.dumps(data), content_type='application/json')
@@ -534,7 +534,7 @@ class AuthenticatedRRsetTests(APITestCase):
         response = self.client.post(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '', 'A',))
         data = {'records': []}
         response = self.client.patch(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
@@ -548,7 +548,7 @@ class AuthenticatedRRsetTests(APITestCase):
         response = self.client.post(url, json.dumps(data), content_type='application/json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.ownedDomains[1].name, '', 'A',))
         response = self.client.delete(url)
         self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
 
@@ -576,7 +576,7 @@ class AuthenticatedRRsetTests(APITestCase):
 
         # Make sure it actually is still there
         self.client.credentials(HTTP_AUTHORIZATION='Token ' + self.otherToken)
-        url = reverse('v1:rrset@', args=(self.otherDomains[0].name, '@', 'A',))
+        url = reverse('v1:rrset@', args=(self.otherDomains[0].name, '', 'A',))
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['records'][0], '1.2.3.4')

api/desecapi/urls/version_1.py

@@ -34,10 +34,15 @@ api_urls = [
 
     # Domain and RRSet endpoints
     path('domains/', views.DomainList.as_view(), name='domain-list'),
-    re_path(r'^domains/(?P<name>[a-zA-Z\.\-_0-9]+)/$', views.DomainDetail.as_view(), name='domain-detail'),
-    re_path(r'^domains/(?P<name>[a-zA-Z\.\-_0-9]+)/rrsets/$', views.RRsetList.as_view(), name='rrsets'),
-    re_path(r'^domains/(?P<name>[a-zA-Z\.\-_0-9]+)/rrsets/(?P<subname>(\*)?[a-zA-Z\.\-_0-9=]*)\.\.\./(?P<type>[A-Z][A-Z0-9]*)/$', views.RRsetDetail.as_view(), name='rrset'),
-    re_path(r'^domains/(?P<name>[a-zA-Z\.\-_0-9]+)/rrsets/(?P<subname>[*@]|[a-zA-Z\.\-_0-9=]+)/(?P<type>[A-Z][A-Z0-9]*)/$', views.RRsetDetail.as_view(), name='rrset@'),
+    path('domains/<name>/', views.DomainDetail.as_view(), name='domain-detail'),
+    path('domains/<name>/rrsets/', views.RRsetList.as_view(), name='rrsets'),
+    path('domains/<name>/rrsets/.../<type>/', views.RRsetDetail.as_view()),
+    re_path(r'domains/(?P<name>[^/]+)/rrsets/(?P<subname>(\*)?[a-zA-Z.\-_0-9]*)\.\.\./(?P<type>[A-Z][A-Z0-9]*)/',
+                views.RRsetDetail.as_view(), name='rrset'),
+    path('domains/<name>/rrsets/@/<type>/', views.RRsetDetail.as_view()),
+    path('domains/<name>/rrsets/<subname>/<type>/', views.RRsetDetail.as_view()),
+    re_path(r'domains/(?P<name>[^/]+)/rrsets/(?P<subname>(\*)?[a-zA-Z.\-_0-9]*)@/(?P<type>[A-Z][A-Z0-9]*)/',
+            views.RRsetDetail.as_view(), name='rrset@'),
 
     # DNS query endpoint
     # TODO remove?

api/desecapi/views.py

@@ -184,11 +184,6 @@ class RRsetDetail(generics.RetrieveUpdateDestroyAPIView):
     serializer_class = RRsetSerializer
     permission_classes = (permissions.IsAuthenticated, IsDomainOwner, IsUnlocked,)
 
-    def dispatch(self, request, *args, **kwargs):
-        if kwargs['subname'] == '@':
-            kwargs['subname'] = ''
-        return super().dispatch(request, *args, **kwargs)
-
     def delete(self, request, *args, **kwargs):
         try:
             super().delete(request, *args, **kwargs)
@@ -198,7 +193,7 @@ class RRsetDetail(generics.RetrieveUpdateDestroyAPIView):
 
     def get_queryset(self):
         name = self.kwargs['name']
-        subname = self.kwargs['subname'].replace('=2F', '/')
+        subname = self.kwargs.get('subname', '')
         type_ = self.kwargs['type']
 
         if type_ in RRset.RESTRICTED_TYPES:
@@ -217,10 +212,8 @@ class RRsetDetail(generics.RetrieveUpdateDestroyAPIView):
         if request.data.get('records') == []:
             return self.delete(request, *args, **kwargs)
 
-        for k in ('type', 'subname'):
-            # This works because we exclusively use JSONParser which causes request.data to be
-            # a dict (and not an immutable QueryDict, as is the case for other parsers)
-            request.data[k] = request.data.pop(k, self.kwargs[k])
+        request.data['type'] = request.data.pop('type', self.kwargs['type'])
+        request.data['subname'] = request.data.pop('subname', self.kwargs.get('subname', ''))
 
         try:
             return super().update(request, *args, **kwargs)

docs/rrsets.rst

@@ -88,12 +88,6 @@ Field details:
     (see RFC 4592 for details).  The maximum length is 178.  Further
     restrictions may apply on a per-user basis.
 
-    If a ``subname`` contains slashes ``/`` and you are using it in the URL
-    path (e.g. when `retrieving a specific RRset`_), it is required to escape
-    them by replacing them with ``=2F``, to resolve the ambiguity that
-    otherwise arises.  (This escape mechanism does not apply to query strings
-    or inside JSON documents.)
-
 ``ttl``
     :Access mode: read, write