Opened 9 years ago

Closed 9 years ago

#24696 closed Cleanup/optimization (fixed)

CSRF middleware wastefully calculates new tokens that are never used .

Reported by: Jay Cox Owned by: nobody
Component: CSRF Version: 1.8
Severity: Normal Keywords: CSRF performance
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When a client requests a page from a site running the csrf middleware and does not send a csrf cookie - a new CSRF token is generated even if the page does not use the token. This unnecessary token generation can add about 40% to the response time for simple requests.

I have created a pull request with a solution that only generates a csrf token when it is going to be used.

Git hub pull request:
https://github.com/django/django/pull/4550

Change History (5)

comment:1 by Moritz Sichert, 9 years ago

Do you have some evidence that the response time gets sped up this much?

As I see it, django.template.context_processors.csrf sets the token as a SimpleLazyObject so it should already be lazy.

comment:2 by Jay Cox, 9 years ago

It is not the context processor causing the computation. It is the csrf middleware (CsrfViewMiddleware .process_view) which computes a new token automatically if there was no csrf cookie sent with the request.

I benchmarked a simple view using siege with the csrf middleware active and again with my patch applied to come up with the 40% number.

The view I benchmarked:

def json(request):
  response = {
    "message": "Hello, World!"
  }
  return HttpResponse(uj_dumps(response), content_type="application/json")

I also ran it under the python cprofiler before patching while exercising the above view and got these results:

Trimmed for relavant section
 ncalls  tottime  percall  cumtime  percall filename:lineno(function)
     1418    0.010    0.000    4.970    0.004 SocketServer.py:282(_handle_request_noblock)
     1418    0.007    0.000    4.882    0.003 SocketServer.py:315(process_request)
     1418    0.018    0.000    4.775    0.003 SocketServer.py:332(finish_request)
     1418    0.011    0.000    4.752    0.003 basehttp.py:97(__init__)
     1418    0.013    0.000    4.452    0.003 SocketServer.py:643(__init__)
     1418    0.028    0.000    4.374    0.003 basehttp.py:161(handle)
     1417    0.011    0.000    3.775    0.003 handlers.py:76(run)
     1417    0.047    0.000    2.800    0.002 wsgi.py:150(__call__)
     1417    0.076    0.000    2.311    0.002 base.py:93(get_response)
     1417    0.023    0.000    1.123    0.001 csrf.py:105(process_view)
18450/18444    0.094    0.000    1.096    0.000 {method 'join' of 'unicode' objects}
     1417    0.005    0.000    1.083    0.001 csrf.py:36(_get_new_csrf_key)
     1417    0.007    0.000    1.078    0.001 crypto.py:54(get_random_string)
    46761    0.128    0.000    1.002    0.000 crypto.py:77(<genexpr>)

In this example no csrf token is ever sent back to the requesting client but it is spending a fair amount of time calculating it on every request.

comment:3 by Tim Graham, 9 years ago

Has patch: set
Triage Stage: UnreviewedAccepted

comment:4 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In eef95ea:

Fixed #24696 -- Made CSRF_COOKIE computation lazy.

Only compute the CSRF_COOKIE when it is actually used. This is a
significant speedup for clients not using cookies.

Changed result of the “test_token_node_no_csrf_cookie” test: It gets
a valid CSRF token now which seems like the correct behavior.

Changed auth_tests.test_views.LoginTest.test_login_csrf_rotate to
use get_token() to trigger CSRF cookie inclusion instead of changing
request.METACSRF_COOKIE_USED directly.

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