[Django] #28948: CookieStorage performance issues
Django
--------------------------------------------+------------------------
Reporter: Michal Čihař | Owner: nobody
Type: Bug | Status: new
Component: contrib.messages | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------------+------------------------
The CookieStorage tries to generate as big cookie as possible to fit all
messages. However doing this on lot of small messages is very expensive
and can take several seconds on the server, potentially leading to denial
of service.
Here is simple code to reproduce the slowness:
{{{
#!/usr/bin/env python
# Configure needed settings
from django.conf import settings
settings.configure(MESSAGE_TAGS={})
from django.contrib.sessions.middleware import SessionMiddleware
from django.contrib.messages.middleware import MessageMiddleware
from django.contrib.messages.storage.cookie import CookieStorage
from django.contrib.messages.api import info
from django.http.request import HttpRequest
from django.http.response import HttpResponse
from django.contrib.messages.storage import default_storage
# Request and response objects
response = HttpResponse()
request = HttpRequest()
# Process request by middleware
SessionMiddleware().process_request(request)
mm = MessageMiddleware()
mm.process_request(request)
# Insert messages
for x in range(500):
info(request, 'm:{0}'.format(x))
# Measure response processing time
import timeit
print(timeit.timeit(
'mm.process_response(request, response)',
globals=globals(), number=10
))
}}}
In my case the DOS was triggered by broken client who repeatedly posted
form generating message, but never did follow redirect to display the
messages, so nothing really sophisticated.
Quickly looking at the code following performance improvements come to my
mind:
* Avoid repeated encoding of the messages, encode them all at once and
then operate on encoded strings
* Avoid calculating HMAC while calculating length as length of it is fixed
* Do bisect instead of removing messages one by one
--
Ticket URL: <https://code.djangoproject.com/ticket/28948>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
----------------------------------+--------------------------------------
Reporter: Michal Čihař | Owner: nobody
Type: Bug | Status: new
Component: contrib.messages | Version: 2.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam (Chainz) Johnson):
* cc: Adam (Chainz) Johnson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:1>
Django
----------------------------------+--------------------------------------
Type: Bug | Status: new
Component: contrib.messages | Version: 2.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* cc: Sergey Fedoseev (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:2>
Django
----------------------------------+------------------------------------
Reporter: Michal Čihař | Owner: nobody
Type: Bug | Status: new
Component: contrib.messages | Version: 2.0
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: Srinivas
| Reddy Thatiparthy
Type: Bug | Status: assigned
Component: contrib.messages | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Srinivas Reddy Thatiparthy):
* owner: nobody => Srinivas Reddy Thatiparthy
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:4>
Django
----------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.messages | Version: 2.0
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Srinivas Reddy Thatiparthy => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:5>
Django
----------------------------------+------------------------------------
Reporter: Michal Čihař | Owner: (none)
Component: contrib.messages | Version: 2.0
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* cc: Sergey Fedoseev (removed)
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:6>
Django
-------------------------------------+-------------------------------------
| Wobrock
Type: Bug | Status: assigned
Component: contrib.messages | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Wobrock):
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
[https://github.com/django/django/pull/16602 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: David
| Wobrock
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: David
| Wobrock
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"9d0c878abf9249da6e16f1acfec311498dc9f368" 9d0c878a]:
{{{
#!CommitTicketReference repository=""
revision="9d0c878abf9249da6e16f1acfec311498dc9f368"
Refs #28948 -- Precomputed once serialized cookie messages.
When the cookie size is too long, the same messages were serialized
over and over again.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: David
| Wobrock
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"21757bbdcd6ef31f2a4092fa1bd55dff29214c7a" 21757bbd]:
{{{
#!CommitTicketReference repository=""
revision="21757bbdcd6ef31f2a4092fa1bd55dff29214c7a"
Refs #28948 -- Removed superfluous messages from cookie through bisect.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: David
| Wobrock
Component: contrib.messages | Version: 2.0
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/28948#comment:11>