Prechádzať zdrojové kódy

Fixes #21906: Return a 404 for REST API writes to endpoints with no trailing slash (#21967)

Jeremy Stretch 1 mesiac pred
rodič
commit
29ae9f400a

+ 19 - 0
netbox/netbox/middleware.py

@@ -8,6 +8,7 @@ from django.core.exceptions import ImproperlyConfigured
 from django.db import ProgrammingError, connection
 from django.db import ProgrammingError, connection
 from django.db.utils import InternalError
 from django.db.utils import InternalError
 from django.http import Http404, HttpResponseRedirect
 from django.http import Http404, HttpResponseRedirect
+from django.middleware.common import CommonMiddleware as DjangoCommonMiddleware
 from django_prometheus import middleware
 from django_prometheus import middleware
 
 
 from netbox.config import clear_config, get_config
 from netbox.config import clear_config, get_config
@@ -18,6 +19,7 @@ from utilities.error_handlers import handle_rest_api_exception
 from utilities.request import apply_request_processors
 from utilities.request import apply_request_processors
 
 
 __all__ = (
 __all__ = (
+    'CommonMiddleware',
     'CoreMiddleware',
     'CoreMiddleware',
     'MaintenanceModeMiddleware',
     'MaintenanceModeMiddleware',
     'PrometheusAfterMiddleware',
     'PrometheusAfterMiddleware',
@@ -26,6 +28,23 @@ __all__ = (
 )
 )
 
 
 
 
+class CommonMiddleware(DjangoCommonMiddleware):
+    """
+    Subclass of Django's CommonMiddleware that suppresses the APPEND_SLASH
+    redirect for REST API requests using an unsafe HTTP method. Redirecting a
+    POST/PUT/PATCH/DELETE to a trailing-slash URL would either drop the request
+    body (clients downgrade to GET on a 302) or raise a RuntimeError when
+    DEBUG is enabled. Letting the original 404 propagate gives the caller a
+    clear, actionable error instead.
+    """
+    UNSAFE_METHODS = frozenset(('DELETE', 'PATCH', 'POST', 'PUT'))
+
+    def should_redirect_with_slash(self, request):
+        if request.method in self.UNSAFE_METHODS and is_api_request(request):
+            return False
+        return super().should_redirect_with_slash(request)
+
+
 class CoreMiddleware:
 class CoreMiddleware:
 
 
     def __init__(self, get_response):
     def __init__(self, get_response):

+ 1 - 1
netbox/netbox/settings.py

@@ -474,7 +474,7 @@ MIDDLEWARE = [
     'corsheaders.middleware.CorsMiddleware',
     'corsheaders.middleware.CorsMiddleware',
     'django.contrib.sessions.middleware.SessionMiddleware',
     'django.contrib.sessions.middleware.SessionMiddleware',
     'django.middleware.locale.LocaleMiddleware',
     'django.middleware.locale.LocaleMiddleware',
-    'django.middleware.common.CommonMiddleware',
+    'netbox.middleware.CommonMiddleware',  # Replaces django.middleware.common.CommonMiddleware
     'django.middleware.csrf.CsrfViewMiddleware',
     'django.middleware.csrf.CsrfViewMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',
     'django.contrib.messages.middleware.MessageMiddleware',
     'django.contrib.messages.middleware.MessageMiddleware',

+ 52 - 0
netbox/utilities/tests/test_api.py

@@ -284,3 +284,55 @@ class GetViewNameTestCase(TestCase):
 
 
         name = get_view_name(view)
         name = get_view_name(view)
         self.assertEqual(name, 'Mock List')
         self.assertEqual(name, 'Mock List')
+
+
+class APITrailingSlashTestCase(APITestCase):
+    """
+    Verify behavior for REST API requests sent to a URL without a trailing slash.
+
+    GET requests should continue to be redirected to the trailing-slash URL (Django's default
+    APPEND_SLASH behavior). Write methods (POST/PUT/PATCH/DELETE) should instead receive a 404
+    so that the request body is not silently dropped by a redirect.
+    """
+    model = Site
+    user_permissions = ('dcim.view_site', 'dcim.add_site', 'dcim.change_site', 'dcim.delete_site')
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.site = Site.objects.create(name='Site 1', slug='site-1')
+
+    def _strip_slash(self, url):
+        return url.rstrip('/')
+
+    def test_get_redirects(self):
+        url = self._strip_slash(reverse('dcim-api:site-list'))
+        response = self.client.get(url, **self.header)
+        self.assertIn(response.status_code, (301, 302))
+        self.assertTrue(response['Location'].endswith('/'))
+
+    def test_post_returns_404(self):
+        url = self._strip_slash(reverse('dcim-api:site-list'))
+        data = {'name': 'Site 2', 'slug': 'site-2'}
+        with disable_warnings('django.request'):
+            response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
+
+    def test_patch_returns_404(self):
+        url = self._strip_slash(self._get_detail_url(self.site))
+        with disable_warnings('django.request'):
+            response = self.client.patch(url, {'name': 'Renamed'}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
+
+    def test_put_returns_404(self):
+        url = self._strip_slash(self._get_detail_url(self.site))
+        data = {'name': 'Renamed', 'slug': 'renamed'}
+        with disable_warnings('django.request'):
+            response = self.client.put(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
+
+    def test_delete_returns_404(self):
+        url = self._strip_slash(self._get_detail_url(self.site))
+        with disable_warnings('django.request'):
+            response = self.client.delete(url, **self.header)
+        self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
+        self.assertTrue(Site.objects.filter(pk=self.site.pk).exists())