[Django] #32643: CookieStorage for contrib.messages crashes after upgrade to django 3.2
Django
--------------------------------------------------+------------------------
Reporter: Jan Pieter Waagmeester | Owner: nobody
Type: Bug | Status: new
Component: contrib.messages | Version: 3.2
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 |
--------------------------------------------------+------------------------
After upgrading to django 3.2, a prevously stored cookie for
contrib.messages crashes in
Django Version: 3.2
Python Version: 3.8.2
{{{
Traceback (most recent call last):
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/core/handlers/exception.py", line 47, in inner
response = get_response(request)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/utils/deprecation.py", line 119, in __call__
response = self.process_response(request, response)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/middleware.py", line 23, in
process_response
unstored_messages = request._messages.update(response)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/storage/base.py", line 127, in update
messages = self._loaded_messages + self._queued_messages
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/storage/base.py", line 79, in
_loaded_messages
messages, all_retrieved = self._get()
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/storage/fallback.py", line 25, in _get
messages, all_retrieved = storage._get()
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/storage/cookie.py", line 86, in _get
messages = self._decode(data)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/contrib/messages/storage/cookie.py", line 175, in _decode
return self.signer.unsign_object(data, serializer=MessageSerializer)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/core/signing.py", line 195, in unsign_object
data = b64_decode(base64d)
File "/home/obs/virtualenv/lib/python3.8/site-
packages/django/core/signing.py", line 68, in b64_decode
return base64.urlsafe_b64decode(s + pad)
File "/usr/lib/python3.8/base64.py", line 133, in urlsafe_b64decode
return b64decode(s)
File "/usr/lib/python3.8/base64.py", line 87, in b64decode
return binascii.a2b_base64(s)
Exception Type: Error at /user/login/
Exception Value: Invalid base64-encoded string: number of data characters
(369) cannot be 1 more than a multiple of 4
}}}
(redacted) contents of the 'messages' cookie:
{{{
'[["__json_message",0,25,"Successfully signed in as '
'ad...@example.org."],["__json_message",0,25,"Successfully '
'signed in as jieter."],["__json_message",0,25,"Ingelogd als '
'ad...@example.org."],["__json_message",0,25,"Ingelogd '
'als '
'ad...@example.org."],["__json_message",0,20,"Bevestigingsmail '
'verzonden naar te...@example.nl."],["__json_message",0,25,"Ingelogd '
'als '
'te...@example.nl."]]:1lTkj1:j_3PlpYSKiqPTMAB6_p2Q00eE8j6k7n0Sg_-_IpXG7Y')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32643>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Type: Bug | Status: new
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Jan Pieter Waagmeester:
Old description:
New description:
After upgrading to django 3.2, a previously stored cookie for
contrib.messages crashes in
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:1>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
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 Mariusz Felisiak):
* cc: Florian Apolloner (added)
Comment:
Jan thanks for this report, however I cannot reproduce this issue.
Everything works when I will use your messages in
[https://github.com/django/django/blob/59552bea5790c97be0da0a6f16ccd0189857c7a7/tests/messages_tests/test_cookie.py#L182-L193
messages_tests.test_cookie.CookieTests.test_legacy_encode_decode].
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:2>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
We might need the original cookie as seen by the browser. Can you send it
to secu...@djangoproject.com (for the lack of a better "trusted" address,
we will handle the data with care).
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:3>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
What confuses me here is that according to the traceback the signature was
valid and it then failed in
https://github.com/django/django/blob/b6475d7d7940f3ce575e0b0f2d83e517f899b4cf/django/core/signing.py#L195
-- but if the signature was valid in the first place the data should have
been base64 encoded. The contents of the cookie seem to suggest otherwise.
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:4>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jan Pieter Waagmeester):
I sent the contents of the cookie to security@
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:5>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Thanks, I think I understand what is going on. Out of pure luck we
created test messages that properly decode from base64. They then fail
json decoding and fall back to the old mechanism. I think we need to add
`binascii.Error` to
https://github.com/django/django/commit/2d6179c819010f6a9d00835d5893c4593c0b85a0
#diff-6c5e944e6869d04c50af2ee02dd85c621177547eb7771d11066f452788d483f4R185
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:6>
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
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 Florian Apolloner):
* Attachment "32643.diff" added.
Django
Reporter: Jan Pieter | Owner: nobody
Waagmeester |
Component: contrib.messages | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Hi Jan, I have attached a diff that should fix your issue. Do you think
you could apply that single change to your Django codebase and retry?
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:7>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: assigned
Component: contrib.messages | Version: 3.2
Severity: Release blocker | Resolution:
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 Mariusz Felisiak):
* owner: nobody => Florian Apolloner
* status: new => assigned
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Regression in 2d6179c819010f6a9d00835d5893c4593c0b85a0.
#32191
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:8>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jan Pieter Waagmeester):
Replying to [comment:7 Florian Apolloner]:
> Hi Jan, I have attached a diff that should fix your issue. Do you think
you could apply that single change to your Django codebase and retry?
Works like expected with the patch applied, great!
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:9>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: assigned
Severity: Release blocker | 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):
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/14266 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:10>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Component: contrib.messages | Version: 3.2
Severity: Release blocker | 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 <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"4511d1459810037b91faa5b506e4f75c77aa72be" 4511d145]:
{{{
#!CommitTicketReference repository=""
revision="4511d1459810037b91faa5b506e4f75c77aa72be"
Fixed #32643 -- Fixed decoding of messages in the pre-Django 3.2 format.
Thanks Jan Pieter Waagmeester for the report.
Regression in 2d6179c819010f6a9d00835d5893c4593c0b85a0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:11>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
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:"539d005aa5fb496f0a648a2385465eca06c604e9" 539d005a]:
{{{
#!CommitTicketReference repository=""
revision="539d005aa5fb496f0a648a2385465eca06c604e9"
[3.2.x] Fixed #32643 -- Fixed decoding of messages in the pre-Django 3.2
format.
Thanks Jan Pieter Waagmeester for the report.
Regression in 2d6179c819010f6a9d00835d5893c4593c0b85a0.
Backport of 4511d1459810037b91faa5b506e4f75c77aa72be from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:12>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Hooper):
Could Django please release a patch release that fixes this bug?
Unless I'm misunderstanding, I've run into this:
1. Upgrade to Django 3.2.
2. Test. Everything passes.
3. Deploy to production.
4. Users experience 500 errors.
In my case, I _happened_ to catch this when I deployed to staging. Pure
luck.
But in general, this crash affect tons of existing sites and there is no
workaround until 3.2.1 is released, right?
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:13>
Django
Reporter: Jan Pieter | Owner: Florian
Waagmeester | Apolloner
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
> and there is no workaround until 3.2.1 is released, right?
Setting a custom storage backend with the fix applied is one option. The
other option is a custom Django package till 3.2.1 is released.
--
Ticket URL: <https://code.djangoproject.com/ticket/32643#comment:14>