diff --git a/docs/reference/filtering.md b/docs/reference/filtering.md index e6187d3e2..14f14b46f 100644 --- a/docs/reference/filtering.md +++ b/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 diff --git a/netbox/dcim/tests/test_filtersets.py b/netbox/dcim/tests/test_filtersets.py index 7e4344f84..a9cdd9527 100644 --- a/netbox/dcim/tests/test_filtersets.py +++ b/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 diff --git a/netbox/extras/lookups.py b/netbox/extras/lookups.py index e1b4d2339..466aad6e8 100644 --- a/netbox/extras/lookups.py +++ b/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)