Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#11944 closed (fixed)

filesizeformat should catch ValueError

Reported by: rfk 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 rfk 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by rfk

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to 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.

comment:2 Changed 5 years ago by rfk

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 5 years ago by russellm

  • milestone set to 1.2
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to 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 Changed 5 years ago by russellm

  • Component changed from Uncategorized to Template system

comment:5 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from reopened to closed

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

comment:6 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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