Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#11944 closed (fixed)

filesizeformat should catch ValueError

Reported by: Ryan Kelly Owned by: nobody
Component: Template system Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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)

filesizeformat.diff (921 bytes) - added by Ryan Kelly 7 years ago.

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by Ryan Kelly

Attachment: filesizeformat.diff added

comment:1 Changed 7 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: wontfix
Status: newclosed

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.

comment:2 Changed 7 years ago by Ryan Kelly

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 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Resolution: wontfix
Status: closedreopened
Triage Stage: UnreviewedAccepted

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 Changed 7 years ago by Russell Keith-Magee

Component: UncategorizedTemplate system

comment:5 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [12426]) Fixed #11944 -- Improved exception handling for the filesizeformat filter. Thanks to rfk for the report and patch.

comment:6 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top