Return ValidationError instead of leaking AttributeError on non-string input#465
Open
gaoflow wants to merge 1 commit into
Open
Return ValidationError instead of leaking AttributeError on non-string input#465gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
…uteError Validators that reach for string methods on the value -- uuid via UUID(value) -> value.replace, email/hostname via value.count, cron via value.strip -- raise AttributeError when given a non-string (int, float, bool, list, dict). The validator decorator only converted (ValueError, TypeError, UnicodeError) to ValidationError, so AttributeError escaped uncaught: a caller passing untrusted or unknown-typed data to a validator got a crash instead of the documented True/ValidationError result, and inconsistently with the many validators that already return ValidationError for wrong-typed input. Add AttributeError to the decorator's caught set so this whole class of validators handles non-string input uniformly.
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
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.
Summary
Several validators leak an uncaught
AttributeErroron non-string input instead of returningValidationError:This is inconsistent with the many validators that already return
ValidationErrorfor wrong-typed input (domain,slug,md5,url,iban,mac_address, …). Since a validator's job is to vet untrusted/unknown input, a caller reasonably passes a value of unknown type and expectsTrue/ValidationError— not a crash.Cause
These validators reach for a string method on the value (
UUID(value)→value.replace(...),value.count(...),value.strip()), which raisesAttributeErrorfor anint/float/bool/list/dict. The@validatordecorator inutils.pyconverts(ValueError, TypeError, UnicodeError)intoValidationError, butAttributeErrorisn't in that set, so it escapes.Fix
Add
AttributeErrorto the decorator's caught set.AttributeErrorfrom calling a string method on a non-string is squarely the "invalid input" category the decorator already handles forTypeError/ValueError, so this makes the whole class of validators handle non-string input uniformly — at the one place that defines the contract — and guards future validators from the same gap.Added a cross-validator regression test (
test_returns_validation_error_on_non_string_input,uuid/email/hostname/cron×int/float/bool/list/dict). Full suite: 915 passed (was 895).ruff check/ruff format --checkclean.Note / scope
I scoped this to the
AttributeErrorclass. Two validators (es_doi,es_nie) leakKeyErroron some inputs via a different code path; I left those out deliberately, since catchingKeyErrorglobally in the decorator would risk masking genuine logic errors. Happy to address them separately if you'd like.This pull request was prepared with the assistance of AI, under my direction and review.