Opened 6 weeks ago

Last modified 6 weeks ago

#36399 assigned New feature

Missing cookies when using ASGI and HTTP/2

Reported by: Ingmar Stein Owned by: JaeHyuckSa
Component: HTTP handling Version: 5.2
Severity: Normal Keywords:
Cc: Carlton Gibson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

I originally created the report here: https://forum.djangoproject.com/t/missing-cookies-when-using-asgi-and-http-2/40946

https://github.com/paperless-ngx/paperless-ngx/issues/9935 describes the issue in more detail. In a nutshell: when serving a Django app using ASGI and HTTP/2, cookies may get dropped. In case this hits the csrftoken cookie, it might explain the various "CSRF verification failed" topics in this forum category.

I had a brief look at the coke and it looks like the ASGI module joins multiple values for the same header using commas but `parse_cookie` splits by semicolon.

Same same issue has also hit other ASGI frameworks: https://github.com/encode/starlette/discussions/2916

@carltongibson created this minimal repro:

from django.conf import settings
from django.core.handlers.asgi import ASGIRequest

settings.configure(DEBUG=True)

scope = {
    "type": "http",
    "asgi": {
        "version": "3.0",
        "spec_version": "2.3",
    },
    "http_version": "2.0",
    "method": "GET",
    "scheme": "http",
    "path": "/",
    "raw_path": b"/",
    "query_string": b"",
    "root_path": "",
    "headers": [
        (b"cookie", b"a=abc;"),
        (b"cookie", b"b=def;"),
        (b"cookie", b"c=ghi;")
    ],
    "client": ("127.0.0.1", 10000),
    "server": ("127.0.0.1", 8000),
    "extensions": {}
}

request = ASGIRequest(scope, None)

print(request.COOKIES)  # Prints: {'a': 'abc', ',b': 'def', ',c': 'ghi'}
assert request.COOKIES == {'a': 'abc', 'b': 'def', 'c': 'ghi'}

Change History (12)

comment:1 by Sarah Boyce, 6 weeks ago

Cc: Carlton Gibson added
Triage Stage: UnreviewedAccepted

Thank you, replicated

comment:2 by Carlton Gibson, 6 weeks ago

Cc: Carlton Gibson removed
Easy pickings: set
Type: BugNew feature

Yes, I confirm this.

ASGIRequest will need to special case the cookie header in a similar way to how it handles content-length &co before entering the _generic headers_ loop.
Multiple cookie values need to be joined with semicolons — which are already present terminating the value — not commas.

Looks like HTTP/1 didn't allow multiple Cookie headers, and they had to be automatically grouped by the client, where HTTP/2 does allow this.

Since this has never been supported I'll mark it a new feature: it probably merits a line in the release notes.

comment:3 by Carlton Gibson, 6 weeks ago

Cc: Carlton Gibson added

comment:4 by JaeHyuckSa, 6 weeks ago

Owner: set to JaeHyuckSa
Status: newassigned

comment:5 by JaeHyuckSa, 6 weeks ago

Has patch: set

comment:6 by Carlton Gibson, 6 weeks ago

Patch needs improvement: set

comment:7 by JaeHyuckSa, 6 weeks ago

Patch needs improvement: unset

comment:8 by Carlton Gibson, 6 weeks ago

Triage Stage: AcceptedReady for checkin

comment:9 by Ingmar Stein, 6 weeks ago

This is a fantastic turnaround time! Well done and thank you :)

comment:10 by Ingmar Stein, 6 weeks ago

Would you consider cherry-picking this into the 5.x branch as well?

comment:11 by Carlton Gibson, 6 weeks ago

This doesn't qualify for a backport but you can easily do yourself by adjusting your asgi.py file.

Something like...

# asgi.py

import os

# Restore when removing ASGIRequest backport.
# from django.core.asgi import get_asgi_application

# Imports for Backport
import django
from django.core.handlers.asgi import ASGIHandler

# https://code.djangoproject.com/ticket/36399
assert django.VERSION < (6,0), "Remove ASGIRequest backport."

# Inline ASGIRequest here.
# You can copy/paste the whole class or just override __init__. Up to you. 

# Subclass ASGIHandler to use your request class. 
class BackportASGIHandler(ASGIHandler):
    request_class = # Your backported request class. 

os.environ.setdefault("DJANGO_SETTINGS_MODULE", "your_app.settings")

# Restore when removing ASGIRequest backport.
# application = get_asgi_application()

django.setup(set_prefix=False)

application = BackportASGIHandler()

That's just a sketch. Adjust to your needs.

comment:12 by Sarah Boyce, 6 weeks ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted
Note: See TracTickets for help on using tickets.
Back to Top