From e15b3d908049c908562d546137540ec26caf9369 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Tue, 15 Sep 2026 10:08:58 -0500 Subject: [PATCH] Fixes #23166: Apply zero-valued numeric bounds to profile attribute form fields --- netbox/dcim/tests/test_forms.py | 27 +++ netbox/utilities/jsonschema.py | 24 ++- netbox/utilities/tests/test_jsonschema.py | 207 ++++++++++++++++++++++ 3 files changed, 249 insertions(+), 9 deletions(-) diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py index 88b31aeb5..8df753a6f 100644 --- a/netbox/dcim/tests/test_forms.py +++ b/netbox/dcim/tests/test_forms.py @@ -229,6 +229,33 @@ class ModuleTypeFormTestCase(TestCase): module_type = form.save() self.assertEqual(module_type.attribute_data, {'media': ['copper', 'qsfp28']}) + def test_zero_bound_attribute_is_enforced_by_the_form(self): + profile = ModuleTypeProfile.objects.create( + name='Module Type Profile 2', + schema={ + 'properties': { + 'offset': { + 'title': 'Offset', + 'type': 'number', + 'minimum': 0, + 'maximum': 0, + }, + }, + }, + ) + form = ModuleTypeForm(data={ + 'manufacturer': self.manufacturer.pk, + 'model': 'Module Type 2', + 'profile': profile.pk, + 'attr_offset': -5, + }) + + self.assertEqual(form.fields['attr_offset'].min_value, 0) + self.assertEqual(form.fields['attr_offset'].max_value, 0) + with patch('utilities.forms.fields.dynamic.get_action_url', return_value='/'): + self.assertFalse(form.is_valid()) + self.assertIn('attr_offset', form.errors) + class ModuleTypeProfileDescriptionRenderingTestCase(TestCase): """ diff --git a/netbox/utilities/jsonschema.py b/netbox/utilities/jsonschema.py index 418eb300e..3eb751925 100644 --- a/netbox/utilities/jsonschema.py +++ b/netbox/utilities/jsonschema.py @@ -87,6 +87,7 @@ class JSONSchemaProperty: """ Instantiate and return a Django form field suitable for editing the property's value. """ + field_class = self.field_class field_kwargs = { 'label': self.title or title(name), 'help_text': render_markdown(self.description), @@ -111,10 +112,14 @@ class JSONSchemaProperty: # String validation if self.type == PropertyTypeEnum.STRING.value: - if self.minLength is not None: - field_kwargs['min_length'] = self.minLength - if self.maxLength is not None: - field_kwargs['max_length'] = self.maxLength + # Checking against CharField is safe because the other CharField-derived fields are + # ruled out by the "is a string" check above. UUIDField is the exception: it cleans to + # a uuid.UUID, which the length validators can't call len() on. + if issubclass(field_class, forms.CharField) and not issubclass(field_class, forms.UUIDField): + if self.minLength is not None: + field_kwargs['min_length'] = self.minLength + if self.maxLength is not None: + field_kwargs['max_length'] = self.maxLength if self.pattern is not None: field_kwargs['validators'] = [ RegexValidator(regex=self.pattern) @@ -122,11 +127,12 @@ class JSONSchemaProperty: # Integer/number validation elif self.type in (PropertyTypeEnum.INTEGER.value, PropertyTypeEnum.NUMBER.value): - field_kwargs['widget'] = forms.NumberInput(attrs={'step': 'any'}) - if self.minimum: - field_kwargs['min_value'] = self.minimum - if self.maximum: - field_kwargs['max_value'] = self.maximum + if issubclass(field_class, forms.IntegerField): + field_kwargs['widget'] = forms.NumberInput(attrs={'step': 'any'}) + if self.minimum is not None: + field_kwargs['min_value'] = self.minimum + if self.maximum is not None: + field_kwargs['max_value'] = self.maximum if self.multipleOf: field_kwargs['validators'] = [ MultipleOfValidator(multiple=self.multipleOf) diff --git a/netbox/utilities/tests/test_jsonschema.py b/netbox/utilities/tests/test_jsonschema.py index 50210fc48..642838690 100644 --- a/netbox/utilities/tests/test_jsonschema.py +++ b/netbox/utilities/tests/test_jsonschema.py @@ -1,8 +1,13 @@ +from uuid import UUID + from django import forms from django.contrib.postgres.forms import SimpleArrayField +from django.core.exceptions import ValidationError +from django.core.validators import RegexValidator from django.test import TestCase from utilities.jsonschema import JSONSchemaProperty +from utilities.validators import MultipleOfValidator class JSONSchemaPropertyTestCase(TestCase): @@ -45,6 +50,208 @@ class JSONSchemaPropertyTestCase(TestCase): self.assertIsInstance(field.base_field, forms.CharField) self.assertEqual(field.clean('ge-0/0/0,ge-0/0/1'), ['ge-0/0/0', 'ge-0/0/1']) + def test_zero_minimum_is_applied_to_form_field(self): + prop = JSONSchemaProperty(type='number', title='Offset', minimum=0) + + field = prop.to_form_field('offset') + + self.assertEqual(field.min_value, 0) + with self.assertRaises(ValidationError): + field.clean(-5) + self.assertEqual(field.clean(0), 0) + + def test_zero_maximum_is_applied_to_form_field(self): + prop = JSONSchemaProperty(type='number', title='Offset', maximum=0) + + field = prop.to_form_field('offset') + + self.assertEqual(field.max_value, 0) + with self.assertRaises(ValidationError): + field.clean(5) + self.assertEqual(field.clean(0), 0) + + def test_zero_bounds_are_applied_to_integer_form_field(self): + prop = JSONSchemaProperty(type='integer', title='Slots', minimum=0, maximum=0) + + field = prop.to_form_field('slots') + + self.assertEqual(field.min_value, 0) + self.assertEqual(field.max_value, 0) + with self.assertRaises(ValidationError): + field.clean(-1) + with self.assertRaises(ValidationError): + field.clean(1) + self.assertEqual(field.clean(0), 0) + + def test_nonzero_bounds_are_applied_to_form_field(self): + prop = JSONSchemaProperty(type='number', title='Offset', minimum=1, maximum=10) + + field = prop.to_form_field('offset') + + self.assertEqual(field.min_value, 1) + self.assertEqual(field.max_value, 10) + with self.assertRaises(ValidationError): + field.clean(0) + with self.assertRaises(ValidationError): + field.clean(11) + + def test_omitted_bounds_are_not_applied_to_form_field(self): + prop = JSONSchemaProperty(type='number', title='Offset') + + field = prop.to_form_field('offset') + + self.assertIsNone(field.min_value) + self.assertIsNone(field.max_value) + self.assertEqual(field.clean(-100), -100) + + def test_numeric_enum_with_zero_bound_builds_choice_field(self): + """A numeric property carrying both an enum and a zero bound resolves to a ChoiceField. + + ChoiceField accepts neither min_value nor max_value, so the numeric bounds must not be + passed through when an enum is present. + """ + prop = JSONSchemaProperty(type='integer', title='Slots', enum=[0, 1, 2], minimum=0) + + field = prop.to_form_field('slots') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(list(field.choices), [(None, ''), (0, 0), (1, 1), (2, 2)]) + + def test_numeric_enum_with_nonzero_bound_builds_choice_field(self): + prop = JSONSchemaProperty(type='integer', title='Slots', enum=[1, 2], minimum=1, maximum=2) + + field = prop.to_form_field('slots') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(list(field.choices), [(None, ''), (1, 1), (2, 2)]) + + def test_numeric_enum_with_multiple_of_builds_choice_field(self): + """An enum suppresses the numeric bounds but retains the multipleOf validator. + + Field.__init__() accepts validators, so a MultipleOfValidator remains applicable to a + ChoiceField even though min_value and max_value are not. + """ + prop = JSONSchemaProperty(type='integer', title='Slots', enum=[2, 4], multipleOf=2) + + field = prop.to_form_field('slots') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(list(field.choices), [(None, ''), (2, 2), (4, 4)]) + self.assertEqual(len(field.validators), 1) + self.assertIsInstance(field.validators[0], MultipleOfValidator) + + def test_string_enum_with_min_length_builds_choice_field(self): + """A string property carrying both an enum and a length bound resolves to a ChoiceField. + + ChoiceField accepts neither min_length nor max_length, so the length bounds must not be + passed through when an enum is present. + """ + prop = JSONSchemaProperty(type='string', title='Media', enum=['a', 'bb'], minLength=1) + + field = prop.to_form_field('media') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(list(field.choices), [(None, ''), ('a', 'a'), ('bb', 'bb')]) + + def test_string_enum_with_max_length_builds_choice_field(self): + prop = JSONSchemaProperty(type='string', title='Media', enum=['a', 'bb'], maxLength=2) + + field = prop.to_form_field('media') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(list(field.choices), [(None, ''), ('a', 'a'), ('bb', 'bb')]) + + def test_string_enum_retains_pattern_validator(self): + """Dropping the length bounds for an enum must not also drop the pattern validator. + + Field.__init__() accepts validators, so a RegexValidator remains applicable to a + ChoiceField even though min_length and max_length are not. + """ + prop = JSONSchemaProperty( + type='string', title='Media', enum=['a', 'bb'], minLength=1, pattern='^[ab]+$' + ) + + field = prop.to_form_field('media') + + self.assertIsInstance(field, forms.ChoiceField) + self.assertEqual(len(field.validators), 1) + self.assertIsInstance(field.validators[0], RegexValidator) + self.assertEqual(field.validators[0].regex.pattern, '^[ab]+$') + + def test_string_bounds_are_applied_without_an_enum(self): + prop = JSONSchemaProperty(type='string', title='Media', minLength=1, maxLength=4) + + field = prop.to_form_field('media') + + self.assertIsInstance(field, forms.CharField) + self.assertEqual(field.min_length, 1) + self.assertEqual(field.max_length, 4) + with self.assertRaises(ValidationError): + field.clean('toolong') + + def test_string_format_with_length_bound_builds_format_field(self): + """A string format resolves to a field class which accepts no length bounds. + + DateField, TimeField and DateTimeField do not subclass CharField, so passing minLength + or maxLength to one raises TypeError. + """ + for string_format, expected_class in ( + ('date', forms.DateField), + ('time', forms.TimeField), + ('datetime', forms.DateTimeField), + ): + with self.subTest(format=string_format): + prop = JSONSchemaProperty( + type='string', title='Timestamp', format=string_format, minLength=10, maxLength=30 + ) + + field = prop.to_form_field('timestamp') + + self.assertIsInstance(field, expected_class) + + def test_string_format_retains_pattern_validator(self): + prop = JSONSchemaProperty(type='string', title='Timestamp', format='date', pattern='^x$') + + field = prop.to_form_field('timestamp') + + self.assertIsInstance(field, forms.DateField) + self.assertEqual(len(field.validators), 1) + self.assertIsInstance(field.validators[0], RegexValidator) + + def test_charfield_derived_format_retains_length_bounds(self): + """EmailField and URLField clean to a string, so the length bounds apply to them.""" + for string_format, expected_class, value in ( + ('email', forms.EmailField, 'user@example.com'), + ('uri', forms.URLField, 'https://example.com/x'), + ): + with self.subTest(format=string_format): + prop = JSONSchemaProperty( + type='string', title='Contact', format=string_format, minLength=5, maxLength=40 + ) + + field = prop.to_form_field('contact') + + self.assertIsInstance(field, expected_class) + self.assertEqual(field.min_length, 5) + self.assertEqual(field.max_length, 40) + self.assertEqual(field.clean(value), value) + + def test_uuid_format_omits_length_bounds(self): + """UUIDField subclasses CharField but cleans to a uuid.UUID, which has no length. + + CharField.__init__() installs a MinLengthValidator and MaxLengthValidator for the bounds, + and those call len() on the cleaned value, so a UUID raises TypeError at clean time. + """ + value = '12345678-1234-5678-1234-567812345678' + prop = JSONSchemaProperty(type='string', title='Serial', format='uuid', minLength=5, maxLength=40) + + field = prop.to_form_field('serial') + + self.assertIsInstance(field, forms.UUIDField) + self.assertIsNone(field.min_length) + self.assertIsNone(field.max_length) + self.assertEqual(field.clean(value), UUID(value)) + class JSONSchemaPropertyDescriptionSanitizationTestCase(TestCase): """