Opened 3 years ago

Closed 3 years ago

#18706 closed Bug (fixed)

django.utils.http.int_to_base36 raises ValueError instead of TypeError for invalid input

Reported by: aaugustin Owned by: aaugustin
Component: Core (Other) Version: master
Severity: Normal 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

This behavior seems wrong to me, and unfortunately, it's tested.

Keeping it in Python 3 is needlessly complicated. Python 3 will rightly raise a TypeError for invalid comparisons. Catching it and re-raising a less appropriate ValueError seems just wrong to me.

Python 2:

>>> 0 <= '' <= 2**31 - 1
False

Python 3:

>>> 0 <= '' <= 2**31 - 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unorderable types: int() <= str()

I'm attaching a patch that adds Python 3 compatibility (we can't use sys.maxint any longer) and improves the handling of invalid inputs under Python 2.

Attachments (1)

18706.patch (4.2 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (6)

Changed 3 years ago by aaugustin

comment:1 Changed 3 years ago by claudep

  • Has patch set
  • Triage Stage changed from Unreviewed to Accepted

Loogs good to me. Should we add a note in release notes as it is slightly backwards incompatible?

comment:2 Changed 3 years ago by aaugustin

  • Owner changed from nobody to aaugustin

Unless someone objects, I'm going to add a line in the release notes and commit this.

comment:3 Changed 3 years ago by Alex

I don't understand, why not just use sys.maxsize?

comment:4 Changed 3 years ago by aaugustin

The check against sys.maxint was only there to prevent accidentally operating on a long instead of an int -- see #15067. Since this distinction no longer exists under Python 3, perpetuating that check would be cargo cult.

Besides, sys.maxsize is the "largest positive integer supported by the platform’s Py_ssize_t type, and thus the maximum size lists, strings, dicts, and many other containers can have", but this function has nothing to do with containers. Using sys.maxsize would be misleading and possibly wrong in some circumstances

comment:5 Changed 3 years ago by Aymeric Augustin <aymeric.augustin@…>

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

In [d01eaf7104e96b2fcf373ddfbc80ef4568bd0387]:

[py3] Removed uses of sys.maxint under Python 3.

Also fixed #18706: improved exceptions raised by int_to_base36.

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