Fixes #23328: Return empty results or a 400 for malformed filter values

Several filters passed unvalidated values through to the database,
raising an unhandled exception (HTTP 500) on malformed input:

- vlan_id/vlan (device & VM interfaces): parse the value as an integer
  and return no results if it is invalid.
- wwn: catch ValidationError, as MultiValueMACAddressFilter does (#11544).
- time_zone: introduce MultiValueTimeZoneFilter, which does the same.
- unit (rack reservations): bound values to the range of
  PositiveSmallIntegerField, so out-of-range values return a 400.
- start_address/end_address (IP ranges): discard invalid addresses
  before querying, as the IP address filter does (#11470). The
  parse_inet_addresses() helper moves to ipam.utils to be shared.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

netbox/dcim/filtersets.py

@@ -32,6 +32,7 @@ from utilities.filters import (
     MultiValueContentTypeFilter,
     MultiValueMACAddressFilter,
     MultiValueNumberFilter,
+    MultiValueTimeZoneFilter,
     MultiValueWWNFilter,
     NumericArrayFilter,
     TreeNodeMultipleChoiceFilter,
@@ -217,7 +218,7 @@ class SiteFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFilterS
         queryset=ASN.objects.all(),
         label=_('AS (ID)'),
     )
-    time_zone = MultiValueCharFilter()
+    time_zone = MultiValueTimeZoneFilter()
 
     class Meta:
         model = Site
@@ -623,7 +624,9 @@ class RackReservationFilterSet(PrimaryModelFilterSet, TenancyFilterSet):
     )
     unit = NumericArrayFilter(
         field_name='units',
-        lookup_expr='contains'
+        lookup_expr='contains',
+        min_value=0,
+        max_value=32767
     )
     unit_count_min = django_filters.NumberFilter(
         field_name='unit_count',
@@ -2349,12 +2352,14 @@ class CommonInterfaceFilterSet(django_filters.FilterSet):
         distinct=False,
         label=_('802.1Q Mode')
     )
-    vlan_id = django_filters.CharFilter(
+    vlan_id = django_filters.NumberFilter(
         method='filter_vlan_id',
+        decimal_places=0,
         label=_('Assigned VLAN')
     )
-    vlan = django_filters.CharFilter(
+    vlan = django_filters.NumberFilter(
         method='filter_vlan',
+        decimal_places=0,
         label=_('Assigned VID')
     )
     vrf_id = django_filters.ModelMultipleChoiceFilter(
@@ -2396,23 +2401,22 @@ class CommonInterfaceFilterSet(django_filters.FilterSet):
     )
 
     def filter_vlan_id(self, queryset, name, value):
-        value = value.strip()
-        if not value:
-            return queryset
-        return queryset.filter(
-            Q(untagged_vlan_id=value) |
-            Q(tagged_vlans=value) |
-            Q(qinq_svlan=value)
-        )
+        return self._filter_assigned_vlan(queryset, 'id', value)
 
     def filter_vlan(self, queryset, name, value):
-        value = value.strip()
-        if not value:
-            return queryset
+        return self._filter_assigned_vlan(queryset, 'vid', value)
+
+    @staticmethod
+    def _filter_assigned_vlan(queryset, field, value):
+        """
+        Filter by any assigned VLAN (untagged, tagged, or Q-in-Q SVLAN) matching the given integer value on the
+        specified VLAN field.
+        """
+        value = int(value)
         return queryset.filter(
-            Q(untagged_vlan_id__vid=value) |
-            Q(tagged_vlans__vid=value) |
-            Q(qinq_svlan__vid=value)
+            Q(**{f'untagged_vlan__{field}': value}) |
+            Q(**{f'tagged_vlans__{field}': value}) |
+            Q(**{f'qinq_svlan__{field}': value})
         )
 
 

netbox/dcim/tests/test_filtersets.py

@@ -343,6 +343,31 @@ class SiteTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'facility': ['Facility 1', 'Facility 2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_time_zone(self):
+        params = {'time_zone': ['America/New_York', 'Europe/London']}
+        self.assertTrue(self.filterset(params, self.queryset).is_valid())
+
+        # An invalid time zone is rejected, including when negated or alongside a valid time zone
+        for params in (
+            {'time_zone': ['no-such-value']},
+            {'time_zone': ['America/New_York', 'no-such-value']},
+            {'time_zone__n': ['no-such-value']},
+            {'time_zone': ['America/']},
+            {'time_zone': ['../x']},
+            {'time_zone': ['x' * 300]},
+        ):
+            self.assertFalse(self.filterset(params, self.queryset).is_valid(), msg=params)
+
+        # The null choice value matches sites without a time zone (all of them, in this test data)
+        for params, count in (({'time_zone': ['null']}, self.queryset.count()), ({'time_zone__n': ['null']}, 0)):
+            filterset = self.filterset(params, self.queryset)
+            self.assertTrue(filterset.is_valid(), msg=params)
+            self.assertEqual(filterset.qs.count(), count, msg=params)
+
+        # Partial matches are not validated
+        params = {'time_zone__ic': ['no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)
+
     def test_asn(self):
         params = {'asn': ['64512', '64513']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
@@ -1302,6 +1327,29 @@ class RackReservationTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'description': ['foobar1', 'foobar2']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_unit(self):
+        params = {'unit': 3}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
+        params = {'unit': '3.0'}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
+        params = {'unit': 0}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)
+        params = {'unit__n': 0}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+        # The "empty" lookup takes a boolean value; units are never null
+        for value in (True, 'true', 1):
+            params = {'unit__empty': value}
+            self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0, msg=value)
+        for value in (False, 'false', 0):
+            params = {'unit__empty': value}
+            self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3, msg=value)
+
+        # A value outside the range of a reservation unit, or not a whole number, is rejected
+        for lookup in ('unit', 'unit__n', 'unit__gt', 'unit__lt'):
+            for value in (-1, 32768, 2147483000, '2.5', '3.7'):
+                self.assertFalse(self.filterset({lookup: value}, self.queryset).is_valid(), msg=(lookup, value))
+
     def test_unit_count(self):
         params = {'unit_count_min': 3}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
@@ -6043,6 +6091,28 @@ class InterfaceTestCase(TestCase, DeviceComponentFilterSetTestMixin, ChangeLogge
         params = {'mac_address': ['00-00-00-00-00-01', '00-00-00-00-00-02']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+    def test_wwn(self):
+        params = {'wwn': ['00:00:00:00:00:00:00:01']}
+        self.assertTrue(self.filterset(params, self.queryset).is_valid())
+
+        # An invalid WWN is rejected, including when negated or alongside a valid WWN
+        for params in (
+            {'wwn': ['no-such-value']},
+            {'wwn': ['00:00:00:00:00:00:00:01', 'no-such-value']},
+            {'wwn__n': ['no-such-value']},
+        ):
+            self.assertFalse(self.filterset(params, self.queryset).is_valid(), msg=params)
+
+        # The null choice value matches interfaces without a WWN (all of them, in this test data)
+        for params, count in (({'wwn': ['null']}, self.queryset.count()), ({'wwn__n': ['null']}, 0)):
+            filterset = self.filterset(params, self.queryset)
+            self.assertTrue(filterset.is_valid(), msg=params)
+            self.assertEqual(filterset.qs.count(), count, msg=params)
+
+        # Partial matches are not validated
+        params = {'wwn__ic': ['no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)
+
     def test_type(self):
         params = {'type': [InterfaceTypeChoices.TYPE_1GE_FIXED, InterfaceTypeChoices.TYPE_1GE_GBIC]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
@@ -6096,6 +6166,11 @@ class InterfaceTestCase(TestCase, DeviceComponentFilterSetTestMixin, ChangeLogge
         params = {'vlan': vlan.vid}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
 
+        # A non-integer value is rejected
+        for value in ('no-such-value', '1.5'):
+            self.assertFalse(self.filterset({'vlan_id': value}, self.queryset).is_valid(), msg=value)
+            self.assertFalse(self.filterset({'vlan': value}, self.queryset).is_valid(), msg=value)
+
     def test_vlan_translation_policy(self):
         vlan_translation_policies = VLANTranslationPolicy.objects.all()[:2]
         params = {'vlan_translation_policy_id': [vlan_translation_policies[0].pk, vlan_translation_policies[1].pk]}
@@ -8838,6 +8913,24 @@ class MACAddressTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'mac_address': ['00-00-00-01-01-01', '00-00-00-02-01-01']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+        # An invalid MAC address is rejected, including when negated or alongside a valid MAC address
+        for params in (
+            {'mac_address': ['no-such-value']},
+            {'mac_address': ['00-00-00-01-01-01', 'no-such-value']},
+            {'mac_address__n': ['no-such-value']},
+        ):
+            self.assertFalse(self.filterset(params, self.queryset).is_valid(), msg=params)
+
+        # The null choice value is accepted (a MAC address object always has an address)
+        for params, count in (({'mac_address': ['null']}, 0), ({'mac_address__n': ['null']}, self.queryset.count())):
+            filterset = self.filterset(params, self.queryset)
+            self.assertTrue(filterset.is_valid(), msg=params)
+            self.assertEqual(filterset.qs.count(), count, msg=params)
+
+        # Partial matches are not validated
+        params = {'mac_address__ic': ['00:00:00:01']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
+
     def test_device(self):
         devices = Device.objects.all()[:2]
         params = {'device_id': [devices[0].pk, devices[1].pk]}

netbox/extras/filters.py

@@ -41,8 +41,7 @@ class MissingKeyAwareFilterMixin:
 
     * filter() is reimplemented rather than delegated to, so any custom filter() on the base class
       is bypassed. Do not mix this into a class which overrides filter() (e.g.
-      MultiValueMACAddressFilter, MultiValueContentTypeFilter).
-      missing_key_aware_filter_factory() rejects such classes.
+      MultiValueContentTypeFilter). missing_key_aware_filter_factory() rejects such classes.
     * `conjoined` is not honored: multiple values are always OR'ed. Passing it raises TypeError.
     """
     def __init__(self, *args, **kwargs):

netbox/extras/tests/test_customfields.py

@@ -40,7 +40,7 @@ from netbox.context import query_cache
 from netbox.context_managers import event_tracking
 from netbox.tables.columns import CustomFieldColumn
 from utilities.exceptions import AbortRequest
-from utilities.filters import MultiValueCharFilter, MultiValueMACAddressFilter
+from utilities.filters import MultiValueCharFilter, MultiValueContentTypeFilter
 from utilities.testing import APITestCase, TestCase
 from virtualization.models import VirtualMachine
 
@@ -2542,9 +2542,9 @@ class MissingKeyAwareFilterTestCase(TestCase):
     set rather than an error.
     """
     def test_factory_rejects_a_class_which_defines_filter(self):
-        # MultiValueMACAddressFilter overrides filter() to swallow ValidationError
+        # MultiValueContentTypeFilter overrides filter() to match on app label and model
         with self.assertRaises(TypeError):
-            missing_key_aware_filter_factory(MultiValueMACAddressFilter)
+            missing_key_aware_filter_factory(MultiValueContentTypeFilter)
 
         # BooleanFilter does not inherit MultipleChoiceFilter.filter() at all
         with self.assertRaises(TypeError):

netbox/ipam/filtersets.py

@@ -1,7 +1,6 @@
 import django_filters
 import netaddr
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import ValidationError
 from django.db.models import Q
 from django.utils.translation import gettext as _
 from drf_spectacular.types import OpenApiTypes
@@ -30,7 +29,7 @@ from vpn.models import L2VPN
 
 from .choices import *
 from .models import *
-from .utils import normalize_port_mapping, port_mapping_q
+from .utils import normalize_port_mapping, parse_inet_addresses, port_mapping_q
 
 __all__ = (
     'ASNFilterSet',
@@ -579,10 +578,11 @@ class IPRangeFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFilt
             return queryset.none()
 
     def filter_address(self, queryset, name, value):
-        try:
-            return queryset.filter(**{f'{name}__net_in': value})
-        except ValidationError:
+        # Discard any invalid addresses. If none remain, return an empty queryset.
+        addresses = parse_inet_addresses(value)
+        if not addresses:
             return queryset.none()
+        return queryset.filter(**{f'{name}__net_in': addresses})
 
     def search_by_parent(self, queryset, name, value):
         if not value:
@@ -751,36 +751,15 @@ class IPAddressFilterSet(PrimaryModelFilterSet, TenancyFilterSet, ContactModelFi
         return queryset.filter(q)
 
     def parse_inet_addresses(self, value):
-        """
-        Parse networks or IP addresses and cast to a format
-        acceptable by the Postgres inet type.
-
-        Skips invalid values.
-        """
-        parsed = []
-        for addr in value:
-            if netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr):
-                parsed.append(addr)
-                continue
-            try:
-                network = netaddr.IPNetwork(addr)
-                parsed.append(str(network))
-            except (AddrFormatError, ValueError):
-                continue
-        return parsed
+        # Retained for backward compatibility; use ipam.utils.parse_inet_addresses() instead.
+        return parse_inet_addresses(value)
 
     def filter_address(self, queryset, name, value):
-        # Let's first parse the addresses passed
-        # as argument. If they are all invalid,
-        # we return an empty queryset
+        # Discard any invalid addresses. If none remain, return an empty queryset.
         value = self.parse_inet_addresses(value)
-        if len(value) == 0:
-            return queryset.none()
-
-        try:
-            return queryset.filter(address__net_in=value)
-        except ValidationError:
+        if not value:
             return queryset.none()
+        return queryset.filter(address__net_in=value)
 
     @extend_schema_field(OpenApiTypes.STR)
     def filter_present_in_vrf(self, queryset, name, vrf):

netbox/ipam/tests/test_filtersets.py

@@ -1058,10 +1058,26 @@ class IPRangeTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'start_address': ['10.0.1.100', '10.0.2.100']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+        # Invalid addresses are discarded
+        params = {'start_address': ['no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)
+        params = {'start_address': ['10.0.1.100', 'no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
+
+        # Surrounding whitespace is ignored
+        params = {'start_address': [' 10.0.1.100 ']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
+
     def test_end_address(self):
         params = {'end_address': ['10.0.1.199', '10.0.2.199']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+        # Invalid addresses are discarded
+        params = {'end_address': ['no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 0)
+        params = {'end_address': ['10.0.1.199', 'no-such-value']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
+
     def test_contains(self):
         params = {'contains': '10.0.1.150/24'}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
@@ -1390,6 +1406,24 @@ class IPAddressTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'address': ['10.0.0.1', '/24', '10.0.0.10/24']}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2)
 
+        # Surrounding whitespace is ignored
+        params = {'address': [' 10.0.0.1/24 ']}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
+
+        # The parse_inet_addresses() method is retained for backward compatibility
+        self.assertEqual(
+            self.filterset().parse_inet_addresses(['10.0.0.1', '/24', '10.0.0.1/255.255.255.0']),
+            ['10.0.0.1', '10.0.0.1/24']
+        )
+
+        # An override of parse_inet_addresses() is honored when filtering
+        class CustomIPAddressFilterSet(IPAddressFilterSet):
+            def parse_inet_addresses(self, value):
+                return ['10.0.0.1/24']
+
+        params = {'address': ['no-such-value']}
+        self.assertEqual(CustomIPAddressFilterSet(params, self.queryset).qs.count(), 1)
+
     def test_mask_length(self):
         params = {'mask_length': [24]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 5)

netbox/ipam/utils.py

@@ -21,6 +21,7 @@ __all__ = (
     'group_port_mappings',
     'legacy_protocol_and_ports',
     'normalize_port_mapping',
+    'parse_inet_addresses',
     'port_mapping_q',
     'rebuild_prefixes',
     'sorted_int_ports',
@@ -285,6 +286,26 @@ def get_next_available_prefix(ipset, prefix_size):
     return None
 
 
+def parse_inet_addresses(value):
+    """
+    Parse networks or IP addresses and cast to a format
+    acceptable by the Postgres inet type.
+
+    Skips invalid values.
+    """
+    parsed = []
+    for addr in value:
+        if netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr):
+            parsed.append(addr)
+            continue
+        try:
+            network = netaddr.IPNetwork(addr)
+            parsed.append(str(network))
+        except (netaddr.AddrFormatError, ValueError):
+            continue
+    return parsed
+
+
 #
 # Service port mappings
 #

netbox/netbox/filtersets.py

@@ -182,7 +182,6 @@ class BaseFilterSet(django_filters.FilterSet):
             django_filters.ChoiceFilter,
             django_filters.MultipleChoiceFilter,
             filters.MultiValueCharFilter,
-            filters.MultiValueMACAddressFilter
         )):
             return FILTER_CHAR_BASED_LOOKUP_MAP
 

netbox/utilities/filters.py

@@ -3,7 +3,9 @@ from django import forms
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
+from django.utils.translation import gettext as _
 from django_filters.constants import EMPTY_VALUES
+from django_filters.utils import get_model_field
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import extend_schema_field
 
@@ -21,6 +23,7 @@ __all__ = (
     'MultiValueMACAddressFilter',
     'MultiValueNumberFilter',
     'MultiValueTimeFilter',
+    'MultiValueTimeZoneFilter',
     'MultiValueWWNFilter',
     'NullableCharFieldFilter',
     'NumericArrayFilter',
@@ -110,20 +113,55 @@ class MultiValueArrayFilter(django_filters.MultipleChoiceFilter):
         return super().get_filter_predicate(v)
 
 
+class InvalidValueFilterMixin:
+    """
+    Reject a value which the model field cannot accept (e.g. a malformed MAC address) during validation, rather than
+    raising an exception when the query is built. (The REST API returns a 400 response for such values.) Only lookups
+    which pass the value to the model field (e.g. "exact") are validated; partial-match lookups (e.g. "icontains")
+    accept any string.
+    """
+    @property
+    def field(self):
+        field = super().field
+        # The model is assigned only once the filter has been bound to a FilterSet instance
+        if getattr(self, 'model', None) is not None and not getattr(self, '_model_field_validated', False):
+            self._model_field_validated = True
+            model_field = get_model_field(self.model, self.field_name)
+            lookup = model_field.get_lookup(self.lookup_expr) if model_field is not None else None
+            if lookup is not None and lookup.prepare_rhs:
+                field.validators.append(self._get_validator(model_field))
+        return field
+
+    def _get_validator(self, model_field):
+        null_value = self.null_value
+
+        def validator(value):
+            # The null choice value is translated to None when filtering
+            if value == null_value:
+                return
+            try:
+                model_field.get_prep_value(value)
+            except (OSError, TypeError, ValueError):
+                # Some values are rejected with an exception other than ValidationError (e.g. zoneinfo raises a
+                # ValueError for a time zone key which is not a normalized path)
+                raise ValidationError(_('Invalid value: {value}').format(value=value))
+
+        return validator
+
+
 @extend_schema_field(OpenApiTypes.STR)
-class MultiValueMACAddressFilter(django_filters.MultipleChoiceFilter):
-    field_class = multivalue_field_factory(forms.CharField)
+class MultiValueMACAddressFilter(InvalidValueFilterMixin, MultiValueCharFilter):
+    pass
 
-    def filter(self, qs, value):
-        try:
-            return super().filter(qs, value)
-        except ValidationError:
-            return qs.none()
+
+@extend_schema_field(OpenApiTypes.STR)
+class MultiValueTimeZoneFilter(InvalidValueFilterMixin, MultiValueCharFilter):
+    pass
 
 
 @extend_schema_field(OpenApiTypes.STR)
-class MultiValueWWNFilter(django_filters.MultipleChoiceFilter):
-    field_class = multivalue_field_factory(forms.CharField)
+class MultiValueWWNFilter(InvalidValueFilterMixin, MultiValueCharFilter):
+    pass
 
 
 @extend_schema_field(OpenApiTypes.STR)
@@ -157,10 +195,22 @@ class NumericArrayFilter(django_filters.NumberFilter):
     """
     Filter based on the presence of an integer within an ArrayField.
     """
+    field_class = forms.IntegerField
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if self.lookup_expr == 'isnull':
+            # The "empty" lookup takes a boolean value, to which numeric constraints do not apply
+            self.field_class = forms.NullBooleanField
+            for param in ('min_value', 'max_value', 'step_size'):
+                self.extra.pop(param, None)
+
     def filter(self, qs, value):
-        if value:
-            value = [value]
-        return super().filter(qs, value)
+        if value is None:
+            return qs
+        if self.lookup_expr == 'isnull':
+            return super().filter(qs, value)
+        return super().filter(qs, [value])
 
 
 class ContentTypeFilter(django_filters.CharFilter):

netbox/virtualization/tests/test_filtersets.py

@@ -872,6 +872,11 @@ class VMInterfaceTestCase(TestCase, ChangeLoggedFilterSetTestMixin):
         params = {'vlan': vlan.vid}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
 
+        # A non-integer value is rejected
+        for value in ('no-such-value', '1.5'):
+            self.assertFalse(self.filterset({'vlan_id': value}, self.queryset).is_valid(), msg=value)
+            self.assertFalse(self.filterset({'vlan': value}, self.queryset).is_valid(), msg=value)
+
     def test_vlan_translation_policy(self):
         vlan_translation_policies = VLANTranslationPolicy.objects.all()[:2]
         params = {'vlan_translation_policy_id': [vlan_translation_policies[0].pk, vlan_translation_policies[1].pk]}