#15067 closed (fixed)
base36_to_int returns a long in certain situations
Reported by: | Kyle Fleming | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | 1.2 |
Severity: | Keywords: | blocker | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django.utils.http.base36_to_int
uses the int()
cast, which returns a long
if it can't fit within an int
. The input is truncated to 13 characters before calling int()
, but if you supply 13 Z's you receive a long
instead of an int
.
>>> type(int('zzzzzzzzzzzzz', 36)) <type 'long'>
This causes an OverflowError
with django.contrib.auth.views.password_reset_done
if you supply uidb36='zzzzzzzzzzzzz'
and token='123'
Note: testing of the password_reset_done
was done on version 1.2.3 and testing of base36_to_int
was done on 1.2.3 and the trunk.
Attachments (2)
Change History (11)
comment:2 Changed 13 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
I suspect the answer here will be to only allow 12 digits instead of 13. That will allow integers up to 4738381338321616895 instead of 9223372036854775807, but (ahem) that should be enough for anybody.
comment:3 Changed 13 years ago by
Keywords: | blocker added |
---|---|
milestone: | → 1.3 |
Changed 13 years ago by
Attachment: | 15067.patch added |
---|
comment:4 Changed 13 years ago by
Has patch: | set |
---|---|
Needs tests: | set |
Limiting it to 12 characters would ensure that the return value is an int, but for 32-bit systems, it is far too large. For 32-bit systems, it would need to be limited to 5 characters. We discussed changes to throw a ValueError from base36_to_int if the value is greater than sys.maxint, but that would potentially break a lot of user systems that may be happily using longs without issue. The safest approach seems to be to just catch the OverflowError in password_reset_confirm and set user to None if it occurs. Patch is attached.
Changed 13 years ago by
Attachment: | 15067_test.diff added |
---|
Test case for overflow condition. The supplied patch passes this test.
comment:5 Changed 13 years ago by
Needs tests: | unset |
---|
comment:6 Changed 13 years ago by
Fair point about 32 bit systems; in which case, a check should be performed against sys.maxint. That enables us to make the contract for base36_to_int into "always returns an int", which makes a certain amount of sense anyway.
comment:7 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |