#11944 closed (fixed)
filesizeformat should catch ValueError
Reported by: | Ryan Kelly | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The "filesizeformat" template tag fails gracefully if input conversion raises a TypeError, but not in other common cases such as ValueError. The attached patch copies the safeguards found in the "floatformat" tag to handle a wider range of error conditions.
Attachments (1)
Change History (7)
by , 15 years ago
Attachment: | filesizeformat.diff added |
---|
comment:1 by , 15 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 15 years ago
Fair enough, but why catch TypeError and not ValueError? The current implementation blows up for reasonable things such as an empty string:
>>> float("") Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: empty string for float() >>>
But will return the string "0 bytes" for completely nonsensical things such as file objects:
>>> float(open("hello.txt")) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: float() argument must be a string or a number >>>
From memory, the empty-string case is the one that lead me to file this bug in the first place.
I would argue for consistency with the floatformat tag, which jumps through a lot of hoops to return *something* even if given nonsensical input. Alternately, perhaps the TypeError guard should be removed so this fails loudly in all cases?
comment:3 by , 15 years ago
milestone: | → 1.2 |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
Triage Stage: | Unreviewed → Accepted |
Reopening based on your arguments, plus some comments from Alex Gaynor in IRC pointing out that Django's templates tend towards silence, and filters like stringformat, floatformat and length all catch ValueError.
comment:4 by , 15 years ago
Component: | Uncategorized → Template system |
---|
comment:5 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I don't see why this error should be caught silently. If you're asking the filesize of a unicode string, I would argue that *should* blow up, and loudly. Please reopen if you can provide a compelling use case.