Skip to content

Stop uuid() from accepting urn:/braced forms via dead-code fallback - #480

Open
mathewOracle wants to merge 1 commit into
python-validators:masterfrom
mathewOracle:fix/uuid-rejects-urn-and-braced-forms
Open

Stop uuid() from accepting urn:/braced forms via dead-code fallback#480
mathewOracle wants to merge 1 commit into
python-validators:masterfrom
mathewOracle:fix/uuid-rejects-urn-and-braced-forms

Conversation

@mathewOracle

Copy link
Copy Markdown

The bug

uuid() builds its result like this:

try:
    return UUID(value) or re.match(
        r"^[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}$", value
    )
except ValueError:
    return False

A successfully-parsed UUID object is always truthy (no __bool__/__len__), so or re.match(...) is unreachable dead code. The real acceptance criteria has always been "whatever UUID() accepts", not the regex the code appears to enforce — and UUID() accepts a lot more than this validator documents or tests:

>>> uuid('urn:uuid:2bc1c94f-0deb-43e9-92a1-4775189ec9f8')
True   # should be rejected
>>> uuid('{2bc1c94f-0deb-43e9-92a1-4775189ec9f8}')
True   # should be rejected

Neither form is in the docstring, tests, or (as far as I could find via GitHub search) any prior issue/PR. The closest related history is #112/#175, about accepting hyphen-less hex — that's an intentional, tested feature this fix keeps working.

The fix

Drop the UUID()-based parsing and validate directly with a regex, extended to also accept the already-tested hyphen-less form:

return bool(re.match(r"^[0-9a-fA-F]{8}-?([0-9a-fA-F]{4}-?){3}[0-9a-fA-F]{12}$", value))

This is a strict subset of what the old code's own regex/docstring described as valid — I deliberately didn't try to guess at any broader intended behavior (e.g. version-nibble checks despite the "UUID-v4" docstring wording), since that would be speculative about intent rather than fixing a demonstrated bug.

Verification

  • All 8 existing test_uuid.py cases pass unchanged.
  • Added 2 regression cases (urn:uuid:..., {...}) to the existing invalid-input parametrize list. Confirmed they fail against the unpatched code (reverted locally to double-check) and pass with the fix.
  • Non-string inputs (123, 1.5, True, ['x'], {'a': 1}, None) still resolve to ValidationError rather than crashing — TypeError from re.match on a non-string is already caught by @validator in utils.py, so no new exception handling needed.
  • Full suite: pytest tests/ — 897 passed (895 baseline + 2 new), 0 regressions.
  • pytest --doctest-modules src/validators/ — 57 passed.
  • ruff format --check, ruff check, pyright all clean on the changed files — matches this repo's pycqa.yaml CI job exactly.

Found via targeted review of validator internals (differential testing across the library's public functions caught the over-permissive acceptance surface), not from a filed issue. AI-assisted (Code Puppy); reproduced, root-caused, fixed, and verified bidirectionally before opening this.

`uuid()` parsed input with the stdlib `UUID()` constructor and fell back
to a strict regex only "if UUID(value) is falsy":

    return UUID(value) or re.match(r"^[0-9a-fA-F]{8}-...$", value)

A successfully-constructed `UUID` object is always truthy (it defines no
`__bool__`/`__len__`), so the `or re.match(...)` branch can never run.
In practice this means the actual acceptance criteria was "whatever
Python's UUID() constructor accepts", not the regex the code appears to
enforce -- and UUID() accepts considerably more than this validator
documents or tests, e.g.:

    >>> uuid('urn:uuid:2bc1c94f-0deb-43e9-92a1-4775189ec9f8')
    True   # should be rejected
    >>> uuid('{2bc1c94f-0deb-43e9-92a1-4775189ec9f8}')
    True   # should be rejected

Neither form appears in the docstring, the tests, or any prior issue/PR
I could find (checked via GitHub search for "uuid" -- the closest,
python-validators#112/python-validators#175, are about supporting hyphen-less hex, which this fix keeps
working).

Fix: drop the UUID()-based parsing entirely and validate with the regex
directly, extended to also accept the already-tested hyphen-less form.
This is a strict subset of what the old code intended to accept (per
its own regex and docstring), a superset check would have been
speculative; verified nothing currently-valid becomes invalid.

Verification:
- All 8 existing `test_uuid.py` cases still pass.
- Added 2 regression cases (`urn:uuid:...`, `{...}`) to the existing
  invalid-input parametrize list; confirmed they fail against the
  unpatched code (reverted locally to check) and pass with the fix.
- Non-string inputs (int, float, bool, list, dict, None) still resolve
  to `ValidationError` rather than crashing -- `TypeError` from
  `re.match` on a non-string is already caught by the `@validator`
  decorator in `utils.py`, so no new exception handling was needed.
- Full suite: `pytest tests/` -- 897 passed (895 baseline + 2 new).
- `pytest --doctest-modules src/validators/` -- 57 passed, doctest for
  `uuid` unaffected.
- `ruff format --check`, `ruff check`, and `pyright` all clean on the
  changed files (matches this repo's `pycqa.yaml` CI job exactly).

Found via targeted review of validator internals after differential
fuzzing across the library's public functions, not from a filed issue.
AI-assisted (Code Puppy); reproduced, root-caused, fixed, and verified
against both the old and new code before opening this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant