Opened 19 months ago

Last modified 19 months ago

#27397 assigned Bug

Querying with an integer larger than SQLite supports crashes with OverflowError

Reported by: Ramin Farajpour Cami Owned by: Ramin Farajpour Cami
Component: Database layer (models, ORM) Version: 1.10
Severity: Normal Keywords: SQLite, Error Handling
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Hi,

i don't know, you accept this behavior Overflow on SQLite or there is max length here?

Step :

http://127.0.0.1:8000/admin/auth/user/1111111111111111111111/change/

Error :

OverflowError at /admin/auth/user/1111111111111111111111/change/
Python int too large to convert to SQLite INTEGER
Request Method:	GET
Request URL:	http://127.0.0.1:8000/admin/auth/user/1111111111111111111111/change/
Django Version:	1.10.2
Exception Type:	OverflowError
Exception Value:	
Python int too large to convert to SQLite INTEGER
Exception Location:	C:\Python27\lib\site-packages\django\db\backends\sqlite3\operations.py in _quote_params_for_last_executed_query, line 129
Python Executable:	C:\Python27\python.exe
Python Version:	2.7.11
Python Path:	
['E:/Programmer Language/NodeJS/untitled1',
 'C:\\Users\\RaminFP\\.IntelliJIdea14\\config\\plugins\\python\\helpers\\pydev',
 'C:\\Python27\\lib\\site-packages\\six-1.10.0-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\pisa-3.0.33-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\sqlacodegen-1.1.6-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\inflect-0.2.5-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\0x10c_asm-0.0.2-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\jpype1-0.6.1-py2.7-win32.egg',
 'C:\\Python27\\lib\\site-packages\\simpleaes-1.0-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\celery-3.1.23-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\kombu-3.0.35-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\anyjson-0.3.3-py2.7.egg',
 'C:\\Python27\\lib\\site-packages\\pythonnet-2.1.0.dev1-py2.7-win32.egg',
 'E:\\Programmer Language\\NodeJS\\untitled1',
 'C:\\Windows\\SYSTEM32\\python27.zip',
 'C:\\Python27\\DLLs',
 'C:\\Python27\\lib',
 'C:\\Python27\\lib\\plat-win',
 'C:\\Python27\\lib\\lib-tk',
 'C:\\Python27',
 'C:\\Python27\\lib\\site-packages',
 'c:\\python27\\lib\\site-packages']
Server time:	Fri, 28 Oct 2016 11:43:05 +0000

Attachments (2)

excep.diff (833 bytes) - added by Ramin Farajpour Cami 19 months ago.
Overflow_handel.diff (1.9 KB) - added by Ramin Farajpour Cami 19 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 19 months ago by nickparkin

Owner: changed from nobody to nickparkin
Status: newassigned

comment:2 Changed 19 months ago by Tim Graham

Summary: OverflowError at /admin/auth/user/<.id>/changeQuerying with an integer larger than SQLite supports crashes with OverflowError
Triage Stage: UnreviewedAccepted

I'm not sure how to fix that, but it would be nice not to crash if possible.

comment:3 Changed 19 months ago by nickparkin

Summary: Querying with an integer larger than SQLite supports crashes with OverflowErrorOverflowError at /admin/auth/user/<.id>/change
Triage Stage: AcceptedUnreviewed

I have reproduced this error. This is an error on SQLite side as the integer is too large to be handled by SQLite.

You will see this error as above when you search for User ID 1111111111111111111111

The max integer is 9223372036854775807. Using this will give you the following error:

Page not found (404)
Request Method: GET
Request URL: http://127.0.0.1:8000/admin/auth/user/9223372036854775807/change/
Raised by: django.contrib.admin.options.change_view

Out of interest, if you use the user ID 9223372036854775808, you will get the same error you saw.

Was not able to recreate the server crashing on my side. It held up fine and simply throws a Server Error when running under production (Debug = False).

comment:6 Changed 19 months ago by nickparkin

comment:7 in reply to:  4 Changed 19 months ago by Ramin Farajpour Cami

Replying to nickparkin:

Looking into if this happens on Postgres now to see if Django is able to handle of these errors.

query can not check max length of valid integer? if check raise on excpetion

Page not found (404)
Request Method: GET
Request URL: ​http://127.0.0.1:8000/admin/auth/user/9223372036854775807/change/
Raised by: django.contrib.admin.options.change_view

check range int after execute query on SQLLite ,

comment:8 Changed 19 months ago by nickparkin

Have confirmed this error by Django handled in Postgres: e.g. http://127.0.0.1:8000/admin/auth/user/922337203685477582000000000000000000/change/ This does not throw the same Overflow error and rather gives a 404 not found.

As such, keeping this open as this could be handled for SQLLite.

Last edited 19 months ago by nickparkin (previous) (diff)

comment:9 Changed 19 months ago by nickparkin

Keywords: SQLite Error Handling added

comment:10 Changed 19 months ago by Ramin Farajpour Cami

Summary: OverflowError at /admin/auth/user/<.id>/changeQuerying with an integer larger than SQLite supports crashes with OverflowError
Triage Stage: UnreviewedAccepted

i test on mysql with many id integer nothing of overflow

comment:11 Changed 19 months ago by Ramin Farajpour Cami

Owner: changed from nickparkin to Ramin Farajpour Cami

Changed 19 months ago by Ramin Farajpour Cami

Attachment: excep.diff added

comment:12 Changed 19 months ago by Ramin Farajpour Cami

Hi Tim,

please look this patch and your feedback ,

comment:13 Changed 19 months ago by Ramin Farajpour Cami

comment:14 Changed 19 months ago by Tim Graham

Assuming a 404 should be raised wherever an OverflowError is raised doesn't seem correct. A fix at the query level seems more correct. Tests are also needed.

comment:15 Changed 19 months ago by Ramin Farajpour Cami

Which query change? (please give me python file this query)

max length range define on sqllite wherever return OverflowError ,

comment:16 Changed 19 months ago by Tim Graham

For example, I was thinking that QuerySet.filter() with a filter parameter value that overflows (and hence can't match anything) might return zero results.

comment:17 Changed 19 months ago by Ramin Farajpour Cami

i think you means this , this patch is a very easy Tim, and best way,

$ git diff HEAD
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 499d27d..58049ed 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -668,7 +668,7 @@ class ModelAdmin(BaseModelAdmin):
         try:
             object_id = field.to_python(object_id)
             return queryset.get(**{field.name: object_id})
-        except (model.DoesNotExist, ValidationError, ValueError):
+        except (model.DoesNotExist, ValidationError, ValueError, OverflowError):
             return None

     def get_changelist_form(self, request, **kwargs):

view :

Page not found (404)
Request Method:	GET
Request URL:	http://127.0.0.1:8000/admin/auth/user/1111111111111111111111111111/change/
Raised by:	django.contrib.admin.options.change_view
user object with primary key u'1111111111111111111111111111' does not exist.

Overflow Code ;

return queryset.get(**{field.name: object_id})

comment:18 Changed 19 months ago by Ramin Farajpour Cami

i'm missing for use which test folders tests ?

please give me sample python file tests object_id, ready for new PR ,

Thanks,
Ramin

comment:19 Changed 19 months ago by Tim Graham

The issue affects all queries, not just those from the admin. A proper fix my go somewhere in the SQLite database backend (django/db/backends/sqlite3) but I'm not sure.

comment:20 Changed 19 months ago by Ramin Farajpour Cami

are you sure "The issue affects all queries, not just those from the admin"? where is issue? i think just happen object_id change_view _ _

i test patch is working (http://127.0.0.1:8000/admin/auth/user/1111111111111111111111/change/),
i need more information a fix,what your means "A proper fix my go somewhere in the SQLite database backend (django/db/backends/sqlite3)"

comment:21 Changed 19 months ago by Ramin Farajpour Cami

still i'm not sure because i test http://127.0.0.1:8000/admin/auth/group/11111111111111111111111111111111111111111/change/ nothing error of OverflowError,

comment:23 Changed 19 months ago by Tim Graham

The idea is to fix things so that Question.objects.filter(id=10000000000000000000) doesn't crash. Doing so should also fix the admin.

comment:24 Changed 19 months ago by Ramin Farajpour Cami

>CREATE TABLE numbers (i INTEGER, r REAL, t TEXT, b BLOB);

>INSERT INTO numbers VALUES (9223372036854775807, 9223372036854775807, 9223372036854775807, 9223372036854775807); 

> SELECT * FROM numbers;
i                     r                     t                     b
--------------------  --------------------  --------------------  --------------------
9223372036854775807   9.22337203685478e+18  9223372036854775807   9223372036854775807

> SELECT typeof(i),typeof(r),typeof(t),typeof(b) FROM numbers;
typeof(i)   typeof(r)   typeof(t)   typeof(b)
----------  ----------  ----------  ----------
integer     real        text        integer

> SELECT i+1,r+1,t+1,b+1 FROM numbers;
i+1                   r+1                   t+1                   b+1
--------------------  --------------------  --------------------  --------------------
-9223372036854775808  9.22337203685478e+18  -9223372036854775808  -9223372036854775808

you see shift +1 and -9223372036854775808

INTEGER. The value is a signed integer, stored in 1, 2, 3, 4, 6, or 8 bytes depending on the magnitude of the value.

REAL. The value is a floating point value, stored as an 8-byte IEEE floating point number.

That is, you can only store values from 2**63 to (2**63-1)

Be careful when you need to store large numbers in SQLite. If you really need to support unsigned 64-bit numbers, ?? but it makes a SQLite bad choice for developing

please look :
https://github.com/python/cpython/blob/master/Modules/_sqlite/util.c#L129&L153

Last edited 19 months ago by Ramin Farajpour Cami (previous) (diff)

comment:25 Changed 19 months ago by Ramin Farajpour Cami

Tim, i see https://docs.djangoproject.com/en/dev/ref/models/fields/#bigintegerfield ,

do you think change fieldid int to biginteger or BigAutoField is good?

Last edited 19 months ago by Ramin Farajpour Cami (previous) (diff)

comment:26 Changed 19 months ago by Ramin Farajpour Cami

final patch

diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py
index 66ad278..36c1351 100644
--- a/django/db/backends/sqlite3/base.py
+++ b/django/db/backends/sqlite3/base.py
@@ -325,8 +325,11 @@ class SQLiteCursorWrapper(Database.Cursor):
         if params is None:
             return Database.Cursor.execute(self, query)
         query = self.convert_query(query)
-        return Database.Cursor.execute(self, query, params)
-
+        try:
+            return Database.Cursor.execute(self, query, params)
+        except OverflowError:
+            return None
+            
     def executemany(self, query, param_list):
         query = self.convert_query(query)
         return Database.Cursor.executemany(self, query, param_list)
diff --git a/django/db/backends/sqlite3/operations.py b/django/db/backends/sqlite3/operations.py
index 47a26b5..6404ea6 100644
--- a/django/db/backends/sqlite3/operations.py
+++ b/django/db/backends/sqlite3/operations.py
@@ -122,6 +122,8 @@ class DatabaseOperations(BaseDatabaseOperations):
         # Native sqlite3 cursors cannot be used as context managers.
         try:
             return cursor.execute(sql, params).fetchone()
+        except OverflowError:
+            return None    
         finally:
             cursor.close()

usage :

>>> User.objects.get(id=1111111111111111111111111111111111111111111111111111111111111111111111111111111111)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "C:\Python27\lib\site-packages\django\db\models\manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "C:\Python27\lib\site-packages\django\db\models\query.py", line 385, in get
    self.model._meta.object_name
DoesNotExist: User matching query does not exist.
>>> User.objects.get(id=111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "C:\Python27\lib\site-packages\django\db\models\manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "C:\Python27\lib\site-packages\django\db\models\query.py", line 385, in get
    self.model._meta.object_name
DoesNotExist: User matching query does not exist.
>>> User.objects.filter(id=1111111111111111111111111111111111111111111111111111111111111111111111111111111111)
<QuerySet []>
>>>
Last edited 19 months ago by Ramin Farajpour Cami (previous) (diff)

comment:27 Changed 19 months ago by Ramin Farajpour Cami

Has patch: set

Changed 19 months ago by Ramin Farajpour Cami

Attachment: Overflow_handel.diff added

comment:28 Changed 19 months ago by Ramin Farajpour Cami

this change is not important i will remove code,

diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 499d27d..58049ed 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -668,7 +668,7 @@ class ModelAdmin(BaseModelAdmin):
         try:
             object_id = field.to_python(object_id)
             return queryset.get(**{field.name: object_id})
-        except (model.DoesNotExist, ValidationError, ValueError):
+        except (model.DoesNotExist, ValidationError, ValueError, OverflowError):
             return None

     def get_changelist_form(self, request, **kwargs):

do you confirm fix and patch for PR?

https://github.com/django/django/blob/master/tests/queries/modles.py

class Ticket27397(models.Model):
    id = models.AutoField(primary_key=True)
    name = models.CharField(max_length=10)

https://github.com/django/django/blob/master/tests/queries/tests.py

    def test_ticket27397(self):
        self.assertEqual(len(Ticket27397.objects.filter(id=111111111111111111111111111111111111)),0)

Last edited 19 months ago by Ramin Farajpour Cami (previous) (diff)

comment:29 Changed 19 months ago by Ramin Farajpour Cami

Needs tests: set

comment:30 Changed 19 months ago by Tim Graham

I'm not sure if the fix is the cleanest one, but I'd have to look at the issue in more detail. I guess you can create a pull request at this point. You should be able to reuse an existing model for the test.

comment:31 Changed 19 months ago by Tim Graham

Needs tests: unset
Patch needs improvement: set

As noted on the PR, the proposal doesn't work correctly for QuerySet.exclude().

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