Opened 3 months ago

Last modified 3 months ago

#35673 assigned Bug

ExceptionReporter.get_traceback_data() does not handle when request.GET data exceeds DATA_UPLOAD_MAX_NUMBER_FIELDS

Reported by: Pēteris Caune Owned by: Mohammad Salehi
Component: Error reporting Version: 5.1
Severity: Normal Keywords:
Cc: Pēteris Caune Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the number of query parameters in URL exceeds settings.DATA_UPLOAD_MAX_NUMBER_FIELDS, Django takes more than a second to generate the error page and eventually returns HTTP 500 with a blank page. The "manage.py runserver" output shows a long chain of exceptions, delimited with "The above exception was the direct cause of the following exception:" line.

To reproduce: start a new Django project, and place the following in urls.py:

from django.http import HttpResponse
from django.urls import path


def index(request):
    request.GET.getlist("a")
    url = "/?" + "&".join([f"a={i}" for i in range(0, 1001)])
    return HttpResponse(f"""<a href="{url}">Click me</a>""", content_type="text/html")


urlpatterns = [
    path("", index),
]

The problem is only triggered if

  • DEBUG=True (otherwise, Django generates a HTTP 400 response with no delay)
  • If the view accesses request.GET

Change History (5)

comment:1 by Sarah Boyce, 3 months ago

Summary: When URL has 1000+ query parameters, and DEBUG=True, Django does not generate the error page correctlyExceptionReporter.get_traceback_data() does not handle when request.GET data exceeds DATA_UPLOAD_MAX_NUMBER_FIELDS
Triage Stage: UnreviewedAccepted

Thank you!

Here's a rough test

  • tests/view_tests/tests/test_debug.py

    a b class DebugViewTests(SimpleTestCase):  
    461461            response = self.client.get("/raises500/", headers={"accept": "text/plain"})
    462462        self.assertContains(response, "Oh dear, an error occurred!", status_code=500)
    463463
     464    @override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=1)
     465    def test_max_number_of_fields_exceeded(self):
     466        with self.assertLogs("django.security", "WARNING"):
     467            response = self.client.get("", query_params={"a": [1, 2]})
     468        self.assertContains(response, '<div class="context" id="', status_code=400)
     469
    464470
    465471class DebugViewQueriesAllowedTests(SimpleTestCase):
    466472    # May need a query to initialize MySQL connection
  • tests/view_tests/views.py

    diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py
    index 9eb7a352d6..f9fc2241a3 100644
    a b TEMPLATES_PATH = Path(__file__).resolve().parent / "templates"  
    2222
    2323def index_page(request):
    2424    """Dummy index page"""
     25    request.GET.getlist("a")
    2526    return HttpResponse("<html><body>Dummy page</body></html>")
    2627

comment:2 by Mohammad Salehi, 3 months ago

Hello, I would like to work on this issue if you’re okay with it?

comment:3 by Mohammad Salehi, 3 months ago

Owner: set to Mohammad Salehi
Status: newassigned

comment:4 by Mohammad Salehi, 3 months ago

Hello,

I have worked on this issue and tested some possible solutions. These are the results:

1_ Adding a flag in QueryDict class:
This does not work because QueryDict is recreated with each request so the flag state is not kept requests.

2_ Changing return value of GET and POST methods in WSGIRequest class:
Changing how GET and POST methods return values will only stop the TooManyFieldsSent error from showing. This will make GET and POST methods not work at all. Request processing will not work correctly and important data will be lost. This approach only hides the error message and does not fix the real problem with handling data.

3_ Removing all GET data:
If we set request.GET to an empt dict in foo method, it shows the error and the problem got fixed but it doesn't make sense to alter GET data.

Version 0, edited 3 months ago by Mohammad Salehi (next)

comment:5 by Mohammad Salehi, 3 months ago

We are encountering a very specific and complex error in the lower layers of the framework. Currently, we use request.GET to retrieve GET parameters. However, if request.GET encounters an error for any reason, how can the higher layers of the framework access these values and display the necessary information on the error page?

which this issue's scenario has exactly the very same problem.

What is your opinion on this?

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