Opened 4 years ago

Closed 4 years ago

#18706 closed Bug (fixed)

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

Reported by: Aymeric Augustin Owned by: Aymeric Augustin
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 Aymeric Augustin 4 years ago.

Download all attachments as: .zip

Change History (6)

Changed 4 years ago by Aymeric Augustin

Attachment: 18706.patch added

comment:1 Changed 4 years ago by Claude Paroz

Has patch: set
Triage Stage: UnreviewedAccepted

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

comment:2 Changed 4 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin

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

comment:3 Changed 4 years ago by Alex Gaynor

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

comment:4 Changed 4 years ago by Aymeric Augustin

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 4 years ago by Aymeric Augustin <aymeric.augustin@…>

Resolution: fixed
Status: newclosed

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