Code

#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 21 months ago.

Download all attachments as: .zip

Change History (6)

Changed 21 months ago by aaugustin

comment:1 Changed 21 months 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 21 months 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 21 months ago by Alex

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

comment:4 Changed 21 months 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 21 months 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.