Opened 4 weeks ago

Last modified 6 days ago

#29427 new Bug

RequestDataTooBig raised in request.py prevents Middleware from returning a valid response

Reported by: S. Paquette Owned by: nobody
Component: HTTP handling Version: 1.11
Severity: Normal Keywords:
Cc: Herbert Fortes Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is effectively a request to re-open #28106, which was closed because the original author never replied to a request for more information.

We need a way to return a response from a Middleware which is handling the RequestDataTooBig exception, but Middlewares intercepting this exception never generate a valid response. This seems due to how the exception causes self._body to never be created in request.py.

In django.http.request, a check of the content length is done against settings.DATA_UPLOAD_MAX_MEMORY_SIZE at line 267.

            # Limit the maximum request data size that will be handled in-memory.
            if (settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None and
                    int(self.META.get('CONTENT_LENGTH') or 0) > settings.DATA_UPLOAD_MAX_MEMORY_SIZE):
                raise RequestDataTooBig('Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE.')

If the content length exceeds DATA_UPLOAD_MAX_MEMORY_SIZE, no request body is generated, and a RequestDataTooBig exception is raised.

In order to detect this error and return a useful response to our users' browsers, we created a Middleware to catch the exception and supply an informative JsonResponse. However, despite the status setting correctly, the response itself was never being returned. Our Middleware:

from django.http import JsonResponse
from django.core.exceptions import RequestDataTooBig


class CheckSize(object):

    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):

        try:
            body = request.body
        except RequestDataTooBig:
            return JsonResponse({"msg": "The file provided is too large. Please reduce its size and try again."}, status=400)

        response = self.get_response(request)
        return response

We tried placing the Middleware anywhere in the chain, and making it the only Middleware, but nothing worked.

Per the author of #28106, we then added in an empty body to the request when the exception is raised, and that solved the problem:

            # Limit the maximum request data size that will be handled in-memory.
            if (settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None and
                    int(self.META.get('CONTENT_LENGTH') or 0) > settings.DATA_UPLOAD_MAX_MEMORY_SIZE):
                self._body = self.read(None)
                raise RequestDataTooBig('Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE.')

After doing this, our response is returned. This can be reproduced on Django 1.11.

Change History (8)

comment:1 Changed 4 weeks ago by Claude Paroz

Triage Stage: UnreviewedAccepted

Seems legitimate. Would you be able to write a failing test for the Django test suite?

comment:2 in reply to:  1 ; Changed 4 weeks ago by S. Paquette

Replying to Claude Paroz:

Seems legitimate. Would you be able to write a failing test for the Django test suite?

Sure thing--how's this look for django/tests/requests/tests.py?

from django.core.exceptions import RequestDataTooBig

def test_req_body_exists_after_size_exceeded(self):
	"""
	If a CONTENT_LENGTH > DATA_UPLOAD_MAX_MEMORY_SIZE is encountered, an empty
	_body attribute should still be generated in the request
	"""
	with override_settings(DATA_UPLOAD_MAX_MEMORY_SIZE=12):
		payload = FakePayload('a=1&a=2;a=3\r\n')
		request = WSGIRequest({
			'REQUEST_METHOD': 'POST',
			'CONTENT_TYPE': 'application/x-www-form-urlencoded',
			'CONTENT_LENGTH': len(payload),
			'wsgi.input': payload,
		})
		
		with self.assertRaises(RequestDataTooBig):
			request.body
			
		self.assertTrue(hasattr(request,'_body'))

comment:3 Changed 3 weeks ago by Herbert Fortes

Cc: Herbert Fortes added

comment:4 Changed 2 weeks ago by Josh Schneier

Is there any reason that this can't be handled using an exception handling middleware?

from django.http import JsonResponse
from django.core.exceptions import RequestDataTooBig

class HandleDataTooBigMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_exception(self, request, exception):
        if isinstance(exception, RequestDataTooBig):
            return JsonResponse({'info': 'File too big'}, status=400)
        return None

comment:5 Changed 12 days ago by Herbert Fortes

I liked process-exception proposed by Josh Schneier. I did not test it though.

comment:6 in reply to:  2 Changed 9 days ago by Herbert Fortes

Replying to S. Paquette:

Replying to Claude Paroz:

Seems legitimate. Would you be able to write a failing test for the Django test suite?

Sure thing--how's this look for django/tests/requests/tests.py?

As I see, only the PR is missing. Test and fix (self._body) are known.
I ran the test.

comment:7 in reply to:  4 Changed 9 days ago by S. Paquette

Replying to Josh Schneier:

Is there any reason that this can't be handled using an exception handling middleware?

from django.http import JsonResponse
from django.core.exceptions import RequestDataTooBig

class HandleDataTooBigMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_exception(self, request, exception):
        if isinstance(exception, RequestDataTooBig):
            return JsonResponse({'info': 'File too big'}, status=400)
        return None

Per the author of #28106, this also doesn't work if the body isn't set; in fact that's how their Middleware is structured. (The one I pasted here is another option I tried when the exception handling Middleware wouldn't work.)

comment:8 Changed 6 days ago by Herbert Fortes

  1. Paquette,

Can you do the PR? You did the test. And if the test needs
adjustments about the style you can do it.

If it is the first time you do a PR to Django project, there is an
official how to write a patch.

Regards

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