Opened 4 months ago

Closed 4 months 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 Dimitar Tasev)

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:

Change History (5)

comment:1 by Dimitar Tasev, 4 months ago

Description: modified (diff)

comment:2 by Joshua Sheng, 4 months ago

Has patch: set
Triage Stage: UnreviewedAccepted

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.

comment:3 by Joshua Sheng, 4 months ago

Owner: changed from nobody to Joshua Sheng
Status: newassigned

comment:4 by Joshua Sheng, 4 months ago

Owner: Joshua Sheng removed
Status: assignednew
Triage Stage: AcceptedUnreviewed

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 Mariusz Felisiak, 4 months ago

Has patch: unset
Resolution: wontfix
Status: newclosed
Type: BugCleanup/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.

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