CSRF middleware calculates new tokens that are never used

[jay...@linear3d.com]

It was requested of me that I post this here to get some more eyes on it:

Django ticket: https://code.djangoproject.com/ticket/24696

Pull request: https://github.com/django/django/pull/4550

I noticed that the csrf middleware will always calculate a new csrf token for any request that does not include a csrf cookie.  This is rather wasteful because in many (most?) cases this token is never sent back to the client.  I have found that avoiding this wasteful computation can speed up requests by about 40%.

For example take the following simple view:

def json(request):

  response = {

    "message": "Hello, World!"

  }

  return HttpResponse(uj_dumps(response), content_type="application/json")

Using siege or openload to request above view with csrf middleware enabled:

  Stock django 1.9:  876 transactions/sec

  With above pull request applied: 1410 transactions/sec 

The above pull request (https://github.com/django/django/pull/4550) causes auth_tests.test_views.LoginTest. test_login_csrf_rotate test to fail.  This is because the test is manually manipulating the request.META["CSRF_COOKIE_USED"] variable.

Do we need to support the case where apps or other middleware change that variable directly?  

Should I change the test, or should I change the patch to support the case of CSRF_COOKIE_USED getting manipulated outside of the csrf middleware?

Thanks,

Jay Cox

[Carl Meyer]

Hi Jay,

On 04/24/2015 12:29 PM, jay...@linear3d.com wrote:
> It was requested of me that I post this here to get some more eyes on it:
>
> Django ticket: https://code.djangoproject.com/ticket/24696
> Pull request: https://github.com/django/django/pull/4550
>
> I noticed that the csrf middleware will always calculate a new csrf
> token for any request that does not include a csrf cookie. This is
> rather wasteful because in many (most?) cases this token is never sent
> back to the client. I have found that avoiding this wasteful
> computation can speed up requests by about 40%.
>
> For example take the following simple view:
>
> def json(request):
>
> response = {
>
> "message": "Hello, World!"
>
> }
>
> return HttpResponse(uj_dumps(response), content_type="application/json")
>
>
>
> Using siege or openload to request above view with csrf middleware enabled:
> Stock django 1.9: 876 transactions/sec
> With above pull request applied: 1410 transactions/sec

Wow. I'm surprised that generating a CSRF token is that expensive, but
it doesn't really matter; if it doesn't need to be done for a given
request, there's no reason to do it.

> The above pull request
> (https://github.com/django/django/pull/4550) causes
> auth_tests.test_views.LoginTest. test_login_csrf_rotate test to fail.
> This is because the test is manually manipulating the
> request.META["CSRF_COOKIE_USED"] variable.
>
> Do we need to support the case where apps or other middleware change
> that variable directly?

META['CSRF_COOKIE_USED'] is never mentioned in the documentation, so I
don't think there is any need to maintain backwards-compatibility with it.

Carl

> Should I change the test, or should I change the patch to support the
> case of CSRF_COOKIE_USED getting manipulated outside of the csrf middleware?
>
>
> Thanks,
>
> Jay Cox
>
>

> --
> You received this message because you are subscribed to the Google
> Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com
> <mailto:django-d...@googlegroups.com>.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/5b4f572f-23c5-4bea-b31e-fe65dd62305b%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/5b4f572f-23c5-4bea-b31e-fe65dd62305b%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[jay...@linear3d.com]

On Friday, April 24, 2015 at 12:18:08 PM UTC-7, Carl Meyer wrote:

Wow. I'm surprised that generating a CSRF token is that expensive, but
it doesn't really matter; if it doesn't need to be done for a given
request, there's no reason to do it.

Most of that time is spent in posix.urandom

 

> The above pull request
> (https://github.com/django/django/pull/4550) causes
> auth_tests.test_views.LoginTest. test_login_csrf_rotate test to fail.
>  This is because the test is manually manipulating the
> request.META["CSRF_COOKIE_USED"] variable.
>
> Do we need to support the case where apps or other middleware change
> that variable directly?  

META['CSRF_COOKIE_USED'] is never mentioned in the documentation, so I
don't think there is any need to maintain backwards-compatibility with it.

Thanks.  I have updated the pull request to fix that test.

 Jay Cox