Opened 2 years ago
Closed 2 years ago
#35041 closed Cleanup/optimization (wontfix)
DATA_UPLOAD_MAX_MEMORY_SIZE causes a confusing error when not an integer
| Reported by: | Dimitar Tasev | Owned by: | |
|---|---|---|---|
| Component: | File uploads/storage | Version: | 4.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
When trying to POST to a FileField or ImageField, an error will be shown when trying to save the object (even without specifying a file) if DATA_UPLOAD_MAX_MEMORY_SIZE is not an integer.
To replicate:
# in settings.py
DATA_UPLOAD_MAX_MEMORY_SIZE = 4e7
# in models.py
from django.db import models
class FileHolder(models.Model):
file = models.FileField(upload_to="files", blank=True, null=True)
# in admin,py
from django.contrib import admin
admin.site.register(FileHolder, admin.ModelAdmin)
Make migrations & migrate, then go to the Admin view of the model, create a new instance and save. There is no need to specify any file for upload, the error will be shown, here is a stacktrace with Django 4.2.8 and Python 3.11.6
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/contrib/admin/options.py", line 688, in wrapper
return self.admin_site.admin_view(view)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/utils/decorators.py", line 130, in _wrapper_view
result = middleware.process_view(request, view_func, args, kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/middleware/csrf.py", line 470, in process_view
self._check_token(request)
File "/usr/local/lib/python3.11/site-packages/django/middleware/csrf.py", line 373, in _check_token
request_csrf_token = request.POST.get("csrfmiddlewaretoken", "")
^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/core/handlers/wsgi.py", line 93, in _get_post
self._load_post_and_files()
File "/usr/local/lib/python3.11/site-packages/django/http/request.py", line 373, in _load_post_and_files
self._post, self._files = self.parse_file_upload(self.META, data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/http/request.py", line 321, in parse_file_upload
return parser.parse()
^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/http/multipartparser.py", line 123, in parse
return self._parse()
^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/http/multipartparser.py", line 235, in _parse
data = field_stream.read(size=read_size)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/http/multipartparser.py", line 465, in read
return b"".join(parts())
^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/http/multipartparser.py", line 460, in parts
emitting = chunk[:remaining]
~~~~~^^^^^^^^^^^^
TypeError: slice indices must be integers or None or have an __index__ method
Changing DATA_UPLOAD_MAX_MEMORY_SIZE = 4e7 to DATA_UPLOAD_MAX_MEMORY_SIZE = 40000000 and repeating the steps will remove the error.
As far as I can tell Django doesn't do type checking of the value of the settings, at least I couldn't get other settings to fail due to invalid types. If a type check during the "system check" step is not possible then an additional type assertion would be good so that a better error message can be shown as only integer works.
I have only tested this through Django Admin, but the error happens inside django.http so perhaps it can be replicated via other ways of POST-ing.
Related links:
- StackOverflow question where I found the solution first: https://stackoverflow.com/questions/48865441/django-admin-typeerror-on-any-post-request
Change History (5)
comment:1 by , 2 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 2 years ago
| Has patch: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 2 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 2 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
| Triage Stage: | Accepted → Unreviewed |
Looks like it'll probably be the user's responsibility for handling this case per this comment on my PR:
We have dozens of settings and we cannot add type checks for all of them to the Settings. Especially when an expected type is clearly documented.
comment:5 by , 2 years ago
| Has patch: | unset |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
| Type: | Bug → Cleanup/optimization |
Thanks for the report, however we cannot add type checks for all settings. It's documented as integer and Django crashes when you use it incorrectly, so it's hard to miss.
You can start a discussion on DevelopersMailingList if you don't agree.
I was able to reproduce the error using the steps you provided. It seems like the bug is that we're not properly guarding to ensure we're setting DATA_UPLOAD_MAX_MEMORY_SIZE as an integer, so I added a guard for it and added a test in this PR. The new result of the bug will be:
Traceback (most recent call last): File "C:\Users\Josh\PycharmProjects\djangoProject\manage.py", line 22, in <module> main() File "C:\Users\Josh\PycharmProjects\djangoProject\manage.py", line 18, in main execute_from_command_line(sys.argv) File "C:\Users\Josh\django\django\core\management\__init__.py", line 442, in execute_from_command_line utility.execute() File "C:\Users\Josh\django\django\core\management\__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "C:\Users\Josh\django\django\core\management\base.py", line 412, in run_from_argv self.execute(*args, **cmd_options) File "C:\Users\Josh\django\django\core\management\commands\runserver.py", line 74, in execute super().execute(*args, **options) File "C:\Users\Josh\django\django\core\management\base.py", line 458, in execute output = self.handle(*args, **options) File "C:\Users\Josh\django\django\core\management\commands\runserver.py", line 81, in handle if not settings.DEBUG and not settings.ALLOWED_HOSTS: File "C:\Users\Josh\django\django\conf\__init__.py", line 81, in __getattr__ self._setup(name) File "C:\Users\Josh\django\django\conf\__init__.py", line 68, in _setup self._wrapped = Settings(settings_module) File "C:\Users\Josh\django\django\conf\__init__.py", line 191, in __init__ raise ImproperlyConfigured( django.core.exceptions.ImproperlyConfigured: The DATA_UPLOAD_MAX_MEMORY_SIZE setting must be an int.