Просмотр исходного кода

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

Django's PostgreSQL backend compiles icontains, iexact, istartswith and
iendswith as UPPER(col::text) LIKE UPPER(%s). UPPER() folds according to the
collation of its argument, and the two sides do not share one: the column folds
under its own collation while the parameter folds under the database default.

For a column using natural_sort, UPPER('ß') is 'SS' on the left and 'ß' on the
right, so searching for 'ß' matched nothing. This affects the name field of most
models and all four case-insensitive lookups, which are also exposed through the
REST API as __ic, __ie, __isw and __iew.

Apply the column's collation to the parameter as well, inside the UPPER() call,
so that both sides fold the same way. Matching on those fields becomes
bidirectional, so 'Strasse' finds 'Straße' and vice versa. Fields without the
collation are unchanged.

The lookups only collate a bare column compared against a simple value. An
expression which already carries an explicit collation, such as Collate() or
CollateAsChar(), would otherwise raise a collation mismatch error.
Jason Novinger 15 часов назад
Родитель
Сommit
2769e3d9d9
3 измененных файлов с 160 добавлено и 0 удалено
  1. 7 0
      docs/reference/filtering.md
  2. 90 0
      netbox/dcim/tests/test_filtersets.py
  3. 63 0
      netbox/extras/lookups.py

+ 7 - 0
docs/reference/filtering.md

@@ -101,6 +101,13 @@ Here is an example of a lookup expression on a string field that will return all
 GET /api/dcim/devices/?name__ic=switch
 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 the German `ß` and `ss` as equivalent: a search for
+    `Strasse` matches a device named `Straße`, and vice versa. Fields which do not use this
+    collation, such as `serial` and `description`, match these characters literally.
+
 ### Foreign Keys & Other Fields
 ### Foreign Keys & Other Fields
 
 
 Certain other fields, namely foreign key relationships support just the negation
 Certain other fields, namely foreign key relationships support just the negation

+ 90 - 0
netbox/dcim/tests/test_filtersets.py

@@ -18,6 +18,7 @@ from netbox.choices import (
 )
 )
 from tenancy.models import Tenant, TenantGroup
 from tenancy.models import Tenant, TenantGroup
 from users.models import User
 from users.models import User
+from utilities.query_functions import CollateAsChar
 from utilities.testing import ChangeLoggedFilterSetTestMixin, create_test_device, create_test_virtualmachine
 from utilities.testing import ChangeLoggedFilterSetTestMixin, create_test_device, create_test_virtualmachine
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine, VMInterface
 from wireless.choices import WirelessChannelChoices, WirelessRoleChoices
 from wireless.choices import WirelessChannelChoices, WirelessRoleChoices
@@ -3375,6 +3376,95 @@ class DeviceTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
         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)
+
+
 class ModuleTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
 class ModuleTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
     queryset = Module.objects.all()
     queryset = Module.objects.all()
     filterset = ModuleFilterSet
     filterset = ModuleFilterSet

+ 63 - 0
netbox/extras/lookups.py

@@ -1,12 +1,18 @@
 from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.fields.ranges import RangeField
 from django.contrib.postgres.fields.ranges import RangeField
 from django.db.models import CharField, JSONField, Lookup
 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.fields.json import KeyTextTransform
+from django.db.models.lookups import IContains, IEndsWith, IExact, IStartsWith
 
 
 from .fields import CachedValueField, ChoiceSetField
 from .fields import CachedValueField, ChoiceSetField
 
 
 __all__ = (
 __all__ = (
     'ChoiceValueLookup',
     'ChoiceValueLookup',
+    'CollatedIContains',
+    'CollatedIEndsWith',
+    'CollatedIExact',
+    'CollatedIStartsWith',
     'Empty',
     'Empty',
     'JSONEmpty',
     'JSONEmpty',
     'NetContainsOrEquals',
     'NetContainsOrEquals',
@@ -14,6 +20,10 @@ __all__ = (
     'RangeContains',
     '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):
 class RangeContains(Lookup):
     """
     """
@@ -123,9 +133,62 @@ class NetContainsOrEquals(Lookup):
         return f'CAST({lhs} AS INET) >>= {rhs}', params
         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.
+    """
+    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.
+        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)
 ArrayField.register_lookup(RangeContains)
 ChoiceSetField.register_lookup(ChoiceValueLookup)
 ChoiceSetField.register_lookup(ChoiceValueLookup)
 CharField.register_lookup(Empty)
 CharField.register_lookup(Empty)
 JSONField.register_lookup(JSONEmpty)
 JSONField.register_lookup(JSONEmpty)
 CachedValueField.register_lookup(NetHost)
 CachedValueField.register_lookup(NetHost)
 CachedValueField.register_lookup(NetContainsOrEquals)
 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)