Fixes #19615: Properly set version request parameter for static files in S3 (#20455)

Author: arthanson

netbox/templates/base/base.html

@@ -26,7 +26,7 @@
     {# Initialize color mode #}
     <script
       type="text/javascript"
-      src="{% static 'setmode.js' %}?v={{ settings.RELEASE.version }}"
+      src="{% static_with_params 'setmode.js' v=settings.RELEASE.version %}"
       onerror="window.location='{% url 'media_failure' %}?filename=setmode.js'">
     </script>
     <script type="text/javascript">
@@ -39,12 +39,12 @@
     {# Static resources #}
     <link
       rel="stylesheet"
-      href="{% static 'netbox-external.css'%}?v={{ settings.RELEASE.version }}"
+      href="{% static_with_params 'netbox-external.css' v=settings.RELEASE.version %}"
       onerror="window.location='{% url 'media_failure' %}?filename=netbox-external.css'"
     />
     <link
       rel="stylesheet"
-      href="{% static 'netbox.css'%}?v={{ settings.RELEASE.version }}"
+      href="{% static_with_params 'netbox.css' v=settings.RELEASE.version %}"
       onerror="window.location='{% url 'media_failure' %}?filename=netbox.css'"
     />
     <link rel="icon" type="image/png" href="{% static 'netbox.ico' %}" />
@@ -53,7 +53,7 @@
     {# Javascript #}
     <script
       type="text/javascript"
-      src="{% static 'netbox.js' %}?v={{ settings.RELEASE.version }}"
+      src="{% static_with_params 'netbox.js' v=settings.RELEASE.version %}"
       onerror="window.location='{% url 'media_failure' %}?filename=netbox.js'">
     </script>
     {% django_htmx_script %}

netbox/utilities/templatetags/builtins/tags.py

@@ -1,5 +1,9 @@
+import logging
+
 from django import template
+from django.templatetags.static import static
 from django.utils.safestring import mark_safe
+from urllib.parse import urlparse, urlunparse, parse_qs, urlencode
 
 from extras.choices import CustomFieldTypeChoices
 from utilities.querydict import dict_to_querydict
@@ -11,6 +15,7 @@
     'customfield_value',
     'htmx_table',
     'formaction',
+    'static_with_params',
     'tag',
 )
 
@@ -127,3 +132,53 @@ def formaction(context):
     if context.get('htmx_navigation', False):
         return mark_safe('hx-push-url="true" hx-post')
     return 'formaction'
+
+
+@register.simple_tag
+def static_with_params(path, **params):
+    """
+    Generate a static URL with properly appended query parameters.
+
+    The original Django static tag doesn't properly handle appending new parameters to URLs
+    that already contain query parameters, which can result in malformed URLs with double
+    question marks. This template tag handles the case where static files are served from
+    AWS S3 or other CDNs that automatically append query parameters to URLs.
+
+    This implementation correctly appends new parameters to existing URLs and checks for
+    parameter conflicts. A warning will be logged if any of the provided parameters
+    conflict with existing parameters in the URL.
+
+    Args:
+        path: The static file path (e.g., 'setmode.js')
+        **params: Query parameters to append (e.g., v='4.3.1')
+
+    Returns:
+        A properly formatted URL with query parameters.
+
+    Note:
+        If any provided parameters conflict with existing URL parameters, a warning
+        will be logged and the new parameter value will override the existing one.
+    """
+    # Get the base static URL
+    static_url = static(path)
+
+    # Parse the URL to extract existing query parameters
+    parsed = urlparse(static_url)
+    existing_params = parse_qs(parsed.query)
+
+    # Check for duplicate parameters and log warnings
+    logger = logging.getLogger('netbox.utilities.templatetags.tags')
+    for key, value in params.items():
+        if key in existing_params:
+            logger.warning(
+                f"Parameter '{key}' already exists in static URL '{static_url}' "
+                f"with value(s) {existing_params[key]}, overwriting with '{value}'"
+            )
+        existing_params[key] = [str(value)]
+
+    # Rebuild the query string
+    new_query = urlencode(existing_params, doseq=True)
+
+    # Reconstruct the URL with the new query string
+    new_parsed = parsed._replace(query=new_query)
+    return urlunparse(new_parsed)

netbox/utilities/tests/test_templatetags.py

@@ -0,0 +1,48 @@
+from unittest.mock import patch
+
+from django.test import TestCase, override_settings
+
+from utilities.templatetags.builtins.tags import static_with_params
+
+
+class StaticWithParamsTest(TestCase):
+    """
+    Test the static_with_params template tag functionality.
+    """
+
+    def test_static_with_params_basic(self):
+        """Test basic parameter appending to static URL."""
+        result = static_with_params('test.js', v='1.0.0')
+        self.assertIn('test.js', result)
+        self.assertIn('v=1.0.0', result)
+
+    @override_settings(STATIC_URL='https://cdn.example.com/static/')
+    def test_static_with_params_existing_query_params(self):
+        """Test appending parameters to URL that already has query parameters."""
+        # Mock the static() function to return a URL with existing query parameters
+        with patch('utilities.templatetags.builtins.tags.static') as mock_static:
+            mock_static.return_value = 'https://cdn.example.com/static/test.js?existing=param'
+
+            result = static_with_params('test.js', v='1.0.0')
+
+            # Should contain both existing and new parameters
+            self.assertIn('existing=param', result)
+            self.assertIn('v=1.0.0', result)
+            # Should not have double question marks
+            self.assertEqual(result.count('?'), 1)
+
+    @override_settings(STATIC_URL='https://cdn.example.com/static/')
+    def test_static_with_params_duplicate_parameter_warning(self):
+        """Test that a warning is logged when parameters conflict."""
+        with patch('utilities.templatetags.builtins.tags.static') as mock_static:
+            mock_static.return_value = 'https://cdn.example.com/static/test.js?v=old_version'
+
+            with self.assertLogs('netbox.utilities.templatetags.tags', level='WARNING') as cm:
+                result = static_with_params('test.js', v='new_version')
+
+                # Check that warning was logged
+                self.assertIn("Parameter 'v' already exists", cm.output[0])
+
+                # Check that new parameter value is used
+                self.assertIn('v=new_version', result)
+                self.assertNotIn('v=old_version', result)