Merge pull request #23164 from netbox-community/23012-eszett-in-search

Fixes #23012: Respect column collation when filtering case-insensitively

docs/reference/filtering.md

@@ -101,6 +101,16 @@ Here is an example of a lookup expression on a string field that will return all
 GET /api/dcim/devices/?name__ic=switch
 ```
 
+!!! note "Case-insensitive matching depends on the field's collation"
+    Most `name` fields use a database collation which sorts them in natural order, so that
+    `device-2` precedes `device-10`. Case-insensitive matching on those fields follows the
+    same collation, which treats a character as equivalent to the sequence it expands to in
+    upper case. The German `ß` is the common example: a search for `Strasse` matches a
+    device named `Straße`, and vice versa. Ligatures such as `fi` behave the same way. One
+    consequence is that a case-insensitive exact match on such a field may return more than
+    one object. Fields which do not use this collation, such as `serial` and `description`,
+    match these characters literally.
+
 ### Foreign Keys & Other Fields
 
 Certain other fields, namely foreign key relationships support just the negation

netbox/dcim/tests/test_filtersets.py

@@ -2,6 +2,7 @@ from decimal import Decimal
 
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
+from django.db import DEFAULT_DB_ALIAS, connection
 from django.test import TestCase
 
 from circuits.models import Circuit, CircuitTermination, CircuitType, Provider
@@ -18,6 +19,7 @@ from netbox.choices import (
 )
 from tenancy.models import Tenant, TenantGroup
 from users.models import User
+from utilities.query_functions import CollateAsChar
 from utilities.testing import ChangeLoggedFilterSetTestMixin, create_test_device, create_test_virtualmachine
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface
 from wireless.choices import WirelessChannelChoices, WirelessRoleChoices
@@ -3375,6 +3377,117 @@ class DeviceTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
 
+class DeviceCollatedFilterTestCase(TestCase):
+    """
+    Case-insensitive filtering against a column which carries the natural_sort collation.
+
+    UPPER() folds according to the collation of its argument, so a collated column and an
+    uncollated parameter disagree: UPPER('ß') is 'SS' under natural_sort but 'ß' under the
+    database default. Searching for 'ß' therefore matched nothing at all (#23012).
+    """
+    queryset = Device.objects.all()
+    filterset = DeviceFilterSet
+
+    @classmethod
+    def setUpTestData(cls):
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+        device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Model 1', slug='model-1')
+        role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1')
+        site = Site.objects.create(name='Site 1', slug='site-1')
+
+        Device.objects.bulk_create((
+            # The reported case: an eszett within the name.
+            Device(name='Straße Switch 1', device_type=device_type, role=role, site=site),
+            # The same word spelled 'ss', which must match the name above and vice versa.
+            Device(name='Strasse Switch 2', device_type=device_type, role=role, site=site),
+            # An eszett alongside an umlaut, to pin that umlauts are not folded.
+            Device(name='Grüße Router 3', device_type=device_type, role=role, site=site),
+            # Plain ASCII control.
+            Device(name='Device 4', device_type=device_type, role=role, site=site),
+        ))
+
+    def assertFilterReturns(self, params, expected_names):
+        names = self.filterset(params, self.queryset).qs.values_list('name', flat=True)
+        self.assertEqual(sorted(names), sorted(expected_names))
+
+    def test_icontains_eszett(self):
+        self.assertFilterReturns(
+            {'name__ic': ['straße']}, ['Straße Switch 1', 'Strasse Switch 2']
+        )
+
+    def test_icontains_ss_matches_eszett(self):
+        self.assertFilterReturns(
+            {'name__ic': ['strasse']}, ['Straße Switch 1', 'Strasse Switch 2']
+        )
+
+    def test_icontains_bare_eszett(self):
+        self.assertFilterReturns(
+            {'name__ic': ['ß']}, ['Straße Switch 1', 'Strasse Switch 2', 'Grüße Router 3']
+        )
+
+    def test_iexact_eszett(self):
+        self.assertFilterReturns({'name__ie': ['strasse switch 1']}, ['Straße Switch 1'])
+
+    def test_istartswith_eszett(self):
+        self.assertFilterReturns(
+            {'name__isw': ['Strasse']}, ['Straße Switch 1', 'Strasse Switch 2']
+        )
+
+    def test_iendswith_eszett(self):
+        self.assertFilterReturns({'name__iew': ['ße Router 3']}, ['Grüße Router 3'])
+
+    def test_ascii_matching_is_unchanged(self):
+        self.assertFilterReturns({'name__ic': ['device']}, ['Device 4'])
+        self.assertFilterReturns({'name__ic': ['SWITCH']}, ['Straße Switch 1', 'Strasse Switch 2'])
+
+    def test_umlauts_are_not_folded(self):
+        # Only the eszett is folded; 'ü' must not match 'u'. Otherwise this would be
+        # blanket accent stripping, which is a much broader change than intended.
+        self.assertFilterReturns({'name__ic': ['grusse']}, [])
+
+    def test_q_search_finds_eszett(self):
+        # The surface reported in #23012: the object list's quick search, which is also
+        # what the REST API uses.
+        self.assertFilterReturns(
+            {'q': 'straße'}, ['Straße Switch 1', 'Strasse Switch 2']
+        )
+
+    def test_uncollated_field_is_unaffected(self):
+        # serial carries no collation, so it keeps plain case-insensitive matching.
+        Device.objects.filter(name='Device 4').update(serial='Straße')
+        self.assertFilterReturns({'serial__ic': ['straße']}, ['Device 4'])
+        self.assertFilterReturns({'serial__ic': ['strasse']}, [])
+
+    def test_explicit_lhs_collation_does_not_error(self):
+        # Applying an explicit collation to the left-hand side must not collide with the
+        # collation applied to the parameter: PostgreSQL rejects two explicit collations
+        # in one comparison, which would turn a working query into a 500.
+        qs = Device.objects.annotate(collated=CollateAsChar('name')).filter(collated__icontains='switch')
+        self.assertEqual(qs.count(), 2)
+
+    def test_collation_is_applied_to_parameter(self):
+        # The tests above assert on results, which stay correct for ASCII values even if
+        # the collation is never applied. This asserts on the lookup's own output instead,
+        # so that the mechanism failing open is caught rather than passing silently.
+        for lookup in ('icontains', 'iexact', 'istartswith', 'iendswith'):
+            with self.subTest(lookup=lookup):
+                self.assertEqual(
+                    self._compiled_rhs(Device, 'name', lookup),
+                    '%s COLLATE "natural_sort"'
+                )
+                self.assertEqual(self._compiled_rhs(Device, 'serial', lookup), '%s')
+
+    @staticmethod
+    def _compiled_rhs(model, field_name, lookup):
+        """
+        Compile a single filter's right-hand side and return its SQL.
+        """
+        query = model.objects.filter(**{f'{field_name}__{lookup}': 'x'}).query
+        compiler = query.get_compiler(using=DEFAULT_DB_ALIAS)
+        rhs, _ = query.where.children[0].process_rhs(compiler, connection)
+        return rhs
+
+
 class ModuleTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
     queryset = Module.objects.all()
     filterset = ModuleFilterSet

netbox/extras/lookups.py

@@ -1,12 +1,18 @@
 from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.fields.ranges import RangeField
 from django.db.models import CharField, JSONField, Lookup
+from django.db.models.expressions import Col
 from django.db.models.fields.json import KeyTextTransform
+from django.db.models.lookups import IContains, IEndsWith, IExact, IStartsWith
 
 from .fields import CachedValueField, ChoiceSetField
 
 __all__ = (
     'ChoiceValueLookup',
+    'CollatedIContains',
+    'CollatedIEndsWith',
+    'CollatedIExact',
+    'CollatedIStartsWith',
     'Empty',
     'JSONEmpty',
     'NetContainsOrEquals',
@@ -14,6 +20,10 @@ __all__ = (
     'RangeContains',
 )
 
+# The ICU collation created by dcim.migrations.0197_natural_sort_collation and applied to
+# the name field of most models.
+NATURAL_SORT_COLLATION = 'natural_sort'
+
 
 class RangeContains(Lookup):
     """
@@ -123,9 +133,70 @@ class NetContainsOrEquals(Lookup):
         return f'CAST({lhs} AS INET) >>= {rhs}', params
 
 
+class CollatedCaseInsensitiveMixin:
+    """
+    Apply the column's collation to the right-hand side of a case-insensitive comparison.
+
+    UPPER() folds according to the collation of its argument. Django uppercases the column
+    under the column's own collation but the parameter under the database default, so for a
+    column using natural_sort the two sides disagree: UPPER('ß') is 'SS' on the left and
+    'ß' on the right, and the comparison silently matches nothing (#23012).
+
+    The COLLATE clause must sit inside UPPER(), not after the comparison, or it applies to
+    the comparison's result rather than to its operand and has no effect.
+
+    Tested in dcim.tests.test_filtersets.DeviceCollatedFilterTestCase, which is where the
+    collated fields these lookups act upon are defined.
+    """
+    def process_rhs(self, compiler, connection):
+        rhs, params = super().process_rhs(compiler, connection)
+        collation = getattr(self.lhs.output_field, 'db_collation', None)
+
+        # Restricted to a bare column compared against a single placeholder. An expression
+        # wrapping the column (Collate() and CollateAsChar() in particular) may already
+        # carry an explicit collation, and PostgreSQL rejects two explicit collations in
+        # one comparison. Requiring a Col also avoids reading a collation from an
+        # annotation's output_field which the annotation itself does not carry, as Concat()
+        # and Coalesce() both do.
+        #
+        # The placeholder is compared literally rather than inspected structurally: a field
+        # declaring its own get_placeholder() compiles to something other than '%s', and
+        # splicing a COLLATE clause into that is not safe. Any other rhs is a deliberate
+        # opt-out which leaves the lookup at its previous behaviour.
+        if collation == NATURAL_SORT_COLLATION and rhs == '%s' and isinstance(self.lhs, Col):
+            # The collation name cannot be passed as a query parameter, but it originates
+            # from the field definition rather than from user input.
+            rhs = f'%s COLLATE "{collation}"'
+
+        return rhs, params
+
+
+class CollatedIContains(CollatedCaseInsensitiveMixin, IContains):
+    pass
+
+
+class CollatedIExact(CollatedCaseInsensitiveMixin, IExact):
+    pass
+
+
+class CollatedIStartsWith(CollatedCaseInsensitiveMixin, IStartsWith):
+    pass
+
+
+class CollatedIEndsWith(CollatedCaseInsensitiveMixin, IEndsWith):
+    pass
+
+
 ArrayField.register_lookup(RangeContains)
 ChoiceSetField.register_lookup(ChoiceValueLookup)
 CharField.register_lookup(Empty)
 JSONField.register_lookup(JSONEmpty)
 CachedValueField.register_lookup(NetHost)
 CachedValueField.register_lookup(NetContainsOrEquals)
+
+# Override the built-in case-insensitive lookups so that they respect the collation of the
+# column being searched.
+CharField.register_lookup(CollatedIContains)
+CharField.register_lookup(CollatedIExact)
+CharField.register_lookup(CollatedIStartsWith)
+CharField.register_lookup(CollatedIEndsWith)