Patch number formatting functions during tests to flag up potential USE_THOUSAND_SEPARATOR issues

.semgrep.yml

@@ -39,6 +39,9 @@ rules:
       - metavariable-regex:
           metavariable: $STRING_ID
           regex: ".*%\\w.*"
+    paths:
+      exclude:
+        - 'wagtail/test/numberformat.py'
     message: >
       Do not use anonymous placeholders for translations.
       Use printf style formatting with named placeholders instead.

wagtail/test/numberformat.py

@@ -0,0 +1,104 @@
+# Patch Django's number formatting functions during tests so that outputting a number onto a
+# template without explicitly passing it through one of |intcomma, |localize, |unlocalize or
+# |filesizeformat will raise an exception. This helps to catch bugs where
+# USE_THOUSAND_SEPARATOR = True incorrectly reformats numbers that are not intended to be
+# human-readable (such as image dimensions, or IDs within data attributes).
+
+from decimal import Decimal
+
+import django.contrib.humanize.templatetags.humanize
+import django.template.defaultfilters
+import django.templatetags.l10n
+import django.utils.numberformat
+from django.core.exceptions import ImproperlyConfigured
+from django.utils import formats
+from django.utils.html import avoid_wrapping
+from django.utils.translation import gettext, ngettext
+
+original_numberformat = django.utils.numberformat.format
+original_intcomma = django.contrib.humanize.templatetags.humanize.intcomma
+
+
+def patched_numberformat(*args, use_l10n=None, **kwargs):
+    if use_l10n is False or use_l10n == "explicit":
+        return original_numberformat(*args, use_l10n=use_l10n, **kwargs)
+
+    raise ImproperlyConfigured(
+        "A number was used directly on a template. "
+        "Numbers output on templates should be passed through one of |intcomma, |localize, "
+        "|unlocalize or |filesizeformat to avoid issues with USE_THOUSAND_SEPARATOR."
+    )
+
+
+def patched_intcomma(value, use_l10n=True):
+    if use_l10n:
+        try:
+            if not isinstance(value, (float, Decimal)):
+                value = int(value)
+        except (TypeError, ValueError):
+            return original_intcomma(value, False)
+        else:
+            return formats.number_format(
+                value, use_l10n="explicit", force_grouping=True
+            )
+
+    return original_intcomma(value, use_l10n=use_l10n)
+
+
+def patched_filesizeformat(bytes_):
+    """
+    Format the value like a 'human-readable' file size (i.e. 13 KB, 4.1 MB,
+    102 bytes, etc.).
+    """
+    try:
+        bytes_ = int(bytes_)
+    except (TypeError, ValueError, UnicodeDecodeError):
+        value = ngettext("%(size)d byte", "%(size)d bytes", 0) % {"size": 0}
+        return avoid_wrapping(value)
+
+    def filesize_number_format(value):
+        return formats.number_format(round(value, 1), 1, use_l10n="explicit")
+
+    KB = 1 << 10
+    MB = 1 << 20
+    GB = 1 << 30
+    TB = 1 << 40
+    PB = 1 << 50
+
+    negative = bytes_ < 0
+    if negative:
+        bytes_ = -bytes_  # Allow formatting of negative numbers.
+
+    if bytes_ < KB:
+        value = ngettext("%(size)d byte", "%(size)d bytes", bytes_) % {"size": bytes_}
+    elif bytes_ < MB:
+        value = gettext("%s KB") % filesize_number_format(bytes_ / KB)
+    elif bytes_ < GB:
+        value = gettext("%s MB") % filesize_number_format(bytes_ / MB)
+    elif bytes_ < TB:
+        value = gettext("%s GB") % filesize_number_format(bytes_ / GB)
+    elif bytes_ < PB:
+        value = gettext("%s TB") % filesize_number_format(bytes_ / TB)
+    else:
+        value = gettext("%s PB") % filesize_number_format(bytes_ / PB)
+
+    if negative:
+        value = "-%s" % value
+    return avoid_wrapping(value)
+
+
+def patched_localize(value):
+    return str(formats.localize(value, use_l10n="explicit"))
+
+
+def patch_number_formats():
+    django.utils.numberformat.format = patched_numberformat
+    django.contrib.humanize.templatetags.humanize.intcomma = patched_intcomma
+    django.template.defaultfilters.filesizeformat = patched_filesizeformat
+    django.template.defaultfilters.register.filter(
+        "filesizeformat", patched_filesizeformat, is_safe=True
+    )
+    django.templatetags.l10n.localize = patched_localize
+    django.templatetags.l10n.register.filter(
+        "localize", patched_localize, is_safe=False
+    )

wagtail/test/settings.py

@@ -3,6 +3,10 @@ import os
 from django.contrib.messages import constants as message_constants
 from django.utils.translation import gettext_lazy as _
 
+from wagtail.test.numberformat import patch_number_formats
+
+patch_number_formats()
+
 DEBUG = os.environ.get("DJANGO_DEBUG", "false").lower() == "true"
 WAGTAIL_ROOT = os.path.dirname(os.path.dirname(__file__))
 WAGTAILADMIN_BASE_URL = "http://testserver"

wagtail/tests/test_tests.py

@@ -1,6 +1,8 @@
 import json
 
+from django.core.exceptions import ImproperlyConfigured
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.template import Context, Template
 from django.test import TestCase
 
 from wagtail.admin.tests.test_contentstate import content_state_equal
@@ -460,3 +462,21 @@ class TestDummyExternalStorage(WagtailTestUtils, TestCase):
             "Content file pointer should be at 0 - got 70 instead",
         ):
             DummyExternalStorage().save("test.png", simple_png)
+
+
+class TestPatchedNumberFormat(TestCase):
+    def test_outputting_number_directly_is_disallowed(self):
+        context = Context({"num": 42})
+        template = Template("the answer is {{ num }}")
+        with self.assertRaises(ImproperlyConfigured):
+            template.render(context)
+
+    def test_outputting_number_via_intcomma(self):
+        context = Context({"num": 9000})
+        template = Template("{% load wagtailadmin_tags %}It's over {{ num|intcomma }}!")
+        self.assertEqual(template.render(context), "It's over 9,000!")
+
+    def test_outputting_number_via_unlocalize(self):
+        context = Context({"num": 9000})
+        template = Template("{% load l10n %}It's over {{ num|unlocalize }}!")
+        self.assertEqual(template.render(context), "It's over 9000!")