diff --git a/CHANGES/6456.bugfix b/CHANGES/6456.bugfix new file mode 100644 index 0000000000..1c2ee91280 --- /dev/null +++ b/CHANGES/6456.bugfix @@ -0,0 +1 @@ +Unified `pulp_labels` key validation across create/update, `set_label`/`unset_label`, and label filters to consistently allow alphanumerics, underscores, spaces, hyphens, and dots. diff --git a/CHANGES/6593.bugfix b/CHANGES/6593.bugfix new file mode 100644 index 0000000000..afc313b9f8 --- /dev/null +++ b/CHANGES/6593.bugfix @@ -0,0 +1 @@ +Fixed a traceback when setting `pulp_labels` with null values on create or update. diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index e693cfc1e2..0b51442baa 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -564,16 +564,22 @@ class SetLabelSerializer(serializers.Serializer): Serializer for synchronously setting a label. """ - key = serializers.SlugField(required=True) + key = serializers.CharField(required=True) value = serializers.CharField(required=True, allow_null=True, allow_blank=True) + def validate(self, data): + from pulpcore.app.serializers.fields import pulp_labels_validator + + pulp_labels_validator({data["key"]: data["value"]}) + return super().validate(data) + class UnsetLabelSerializer(serializers.Serializer): """ Serializer for synchronously UNsetting a label. """ - key = serializers.SlugField(required=True) + key = serializers.CharField(required=True) value = serializers.CharField(read_only=True) def validate_key(self, value): diff --git a/pulpcore/app/serializers/fields.py b/pulpcore/app/serializers/fields.py index dbb2261239..505fc82d2c 100644 --- a/pulpcore/app/serializers/fields.py +++ b/pulpcore/app/serializers/fields.py @@ -11,6 +11,7 @@ from rest_framework.fields import empty from pulpcore.app import models +from pulpcore.constants import LABEL_KEY_REGEX from pulpcore.app.serializers import DetailIdentityField, IdentityField, RelatedField from pulpcore.app.util import reverse @@ -427,9 +428,14 @@ def pulp_labels_validator(value): value = json.loads(value) for k, v in value.items(): - if not re.match(r"^[\w ]+$", k): - raise serializers.ValidationError(_("Key '{}' contains non-alphanumerics.").format(k)) - if re.search(r"[,()]", v): + if not re.match(LABEL_KEY_REGEX, k): + raise serializers.ValidationError( + _( + "Key '{}' contains invalid characters. Only alphanumerics, underscores," + " spaces, hyphens, and dots are allowed." + ).format(k) + ) + if v is not None and re.search(r"[,()]", v): raise serializers.ValidationError( _("Key '{}' contains value with comma or parenthesis.").format(k) ) diff --git a/pulpcore/app/viewsets/custom_filters.py b/pulpcore/app/viewsets/custom_filters.py index dfe140e092..16b7e9f504 100644 --- a/pulpcore/app/viewsets/custom_filters.py +++ b/pulpcore/app/viewsets/custom_filters.py @@ -9,6 +9,7 @@ from gettext import gettext as _ from django.conf import settings +from pulpcore.constants import LABEL_KEY_CHARS from django.db.models import ObjectDoesNotExist from django_filters import BaseInFilter, CharFilter, Filter from drf_spectacular.types import OpenApiTypes @@ -313,7 +314,7 @@ def filter(self, qs, value): return qs for term in value.split(","): - match = re.match(r"(!?[\w\s]+)(=|!=|~)?(.*)?", term) + match = re.match(rf"(!?{LABEL_KEY_CHARS}+)(=|!=|~)?(.*)?", term) if not match: raise DRFValidationError(_("Invalid search term: '{}'.").format(term)) key, op, val = match.groups() diff --git a/pulpcore/constants.py b/pulpcore/constants.py index 614e131ea9..af980f98b9 100644 --- a/pulpcore/constants.py +++ b/pulpcore/constants.py @@ -129,6 +129,10 @@ ORPHAN_PROTECTION_TIME_LOWER_BOUND = 0 ORPHAN_PROTECTION_TIME_UPPER_BOUND = 4294967295 # (2^32)-1 +# Valid characters for pulp_labels keys: alphanumerics, underscores, spaces, hyphens, and dots. +LABEL_KEY_CHARS = r"[\w .\-]" +LABEL_KEY_REGEX = rf"^{LABEL_KEY_CHARS}+$" + # VULNERABILITY REPORT CONSTANTS # OSV API URL OSV_QUERY_URL = "https://api.osv.dev/v1/query" diff --git a/pulpcore/tests/unit/serializers/test_fields.py b/pulpcore/tests/unit/serializers/test_fields.py index 506d4c1361..d003278a9f 100644 --- a/pulpcore/tests/unit/serializers/test_fields.py +++ b/pulpcore/tests/unit/serializers/test_fields.py @@ -2,6 +2,44 @@ from rest_framework import serializers from pulpcore.app.serializers import fields +from pulpcore.app.serializers.fields import pulp_labels_validator + + +@pytest.mark.parametrize( + "labels", + [ + pytest.param({"key": "value"}, id="normal"), + pytest.param({"key": ""}, id="empty-value"), + pytest.param({"key": None}, id="none-value"), + pytest.param({"key1": "value", "key2": None, "key3": ""}, id="multiple-keys"), + pytest.param({"my-key": "value"}, id="dash-key"), + pytest.param({"my.key": "value"}, id="dotted-key"), + pytest.param({"my key": "value"}, id="spaced-key"), + pytest.param({"my-dotted.key": "value"}, id="dotted-dash-key"), + pytest.param({"spaced key-with.mixed_chars": "value"}, id="all-key"), + ], +) +def test_pulp_labels_validator_valid(labels): + """Valid label keys and values should pass validation.""" + result = pulp_labels_validator(labels) + assert result == labels + + +@pytest.mark.parametrize( + "labels", + [ + pytest.param({"key": "val,ue"}, id="comma-value"), + pytest.param({"key": "val(ue"}, id="open-parenthesis-value"), + pytest.param({"key": "val)ue"}, id="close-parenthesis-value"), + pytest.param({"bad!key": "value"}, id="exclamation-key"), + pytest.param({"bad:key": "value"}, id="colon-key"), + pytest.param({"bad@key": "value"}, id="at-sign-key"), + ], +) +def test_pulp_labels_validator_invalid(labels): + """Invalid label keys or values should raise ValidationError.""" + with pytest.raises(serializers.ValidationError): + pulp_labels_validator(labels) @pytest.mark.parametrize(