Skip to content

Improve validation around pulp-labels#7394

Open
gerrod3 wants to merge 2 commits intopulp:mainfrom
gerrod3:fix/6593-null-label-traceback
Open

Improve validation around pulp-labels#7394
gerrod3 wants to merge 2 commits intopulp:mainfrom
gerrod3:fix/6593-null-label-traceback

Conversation

@gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Mar 4, 2026

Summary

  • pulp_labels_validator crashed with TypeError: expected string or bytes-like object when a label value was JSON null, because re.search() doesn't accept None. Skip the regex check for None values since they are already accepted by the set_label endpoint and HStoreField.
  • Unified label key validation across pulp_labels_validator, SetLabelSerializer/UnsetLabelSerializer, and LabelFilter to consistently allow alphanumerics, underscores, spaces, hyphens, and dots. Previously, keys with spaces could be created but not modified via set_label, and keys with hyphens were accepted by set_label but rejected on create.

Test plan

  • Added unit tests for pulp_labels_validator covering None values, empty strings, and keys with hyphens/dots/spaces.
  • Manually verified create and PATCH with {"build_id": null} no longer returns a 500.
  • CI passes.

closes #6593
closes #6456

The pulp_labels_validator crashed with "expected string or bytes-like
object" when a label value was null because re.search() doesn't accept
None. Skip the regex check for None values since they are already
accepted by the set_label endpoint and HStoreField.

closes pulp#6593

Assisted-by: Claude (Cursor)
Made-with: Cursor
@gerrod3 gerrod3 changed the title Fix traceback on null pulp_labels values Improve validation around pulp-labels Mar 4, 2026
@gerrod3 gerrod3 force-pushed the fix/6593-null-label-traceback branch from 2d8491e to 4a5c471 Compare March 4, 2026 21:39
],
)
def test_pulp_labels_validator_valid(labels):
"""Valid label values including None should pass validation."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is out of place. You could use pytest.param with id to add the intention to individual parameterizations.

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(r"^[\w .\-]+$", k):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's worth having a compiled version of this. But since you already needed to consolidate this RegEx in more than one place, shouldn't we have a single definition for it?

The three places validating label keys (pulp_labels_validator,
SetLabelSerializer/UnsetLabelSerializer, and LabelFilter) each allowed
different characters, making it possible to create labels that couldn't
be modified or filtered. Unify them all to accept alphanumerics,
underscores, spaces, hyphens, and dots.

closes pulp#6456

Assisted-by: Claude-Opus-4.6 (Cursor)
@gerrod3 gerrod3 force-pushed the fix/6593-null-label-traceback branch from 4a5c471 to 35cdc41 Compare March 5, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't traceback on null label Error trying to set label to existing repository

2 participants