Repository navigation
Stop uuid() from accepting urn:/braced forms via dead-code fallback - #480
Open
mathewOracle wants to merge 1 commit into
Open
mathewOracle wants to merge 1 commit into
mathewOracle wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
`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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
uuid() builds its result like this:
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:
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:
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
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.