From afb15a737572fd7256c7668e5b1e495037da5cf2 Mon Sep 17 00:00:00 2001 From: Christopher Pruijsen Date: Sun, 13 Sep 2026 09:33:42 +0000 Subject: [PATCH 1/2] fix: preserve all Set-Cookie headers in webob_to_django_response Setting django_response[name] for each webob header overwrote earlier Set-Cookie headers, so only the last cookie reached the client. Route Set-Cookie values through the response's cookie jar instead, which emits one header per cookie. Fixes openedx/XBlock#523. --- CHANGELOG.rst | 3 +++ xblock/django/request.py | 7 ++++++- xblock/test/django/test_request.py | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 599ca96a2..4ca442c54 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ Change history for XBlock Unreleased ---------- +* Fixed ``webob_to_django_response`` dropping all but the last ``Set-Cookie`` + header; they are now loaded into the Django response's cookie jar so every + cookie is sent to the client. * Removed ``threading.RLock`` from ``Mixologist``'s class cache to make it safe for ASGI/async deployments. ``dict.setdefault()`` atomicity (via CPython's GIL) provides equivalent thread safety without blocking the event loop. diff --git a/xblock/django/request.py b/xblock/django/request.py index 067bdb80c..bbe8f71ac 100644 --- a/xblock/django/request.py +++ b/xblock/django/request.py @@ -22,7 +22,12 @@ def webob_to_django_response(webob_response, streaming=False): status=webob_response.status_code, ) for name, value in webob_response.headerlist: - django_response[name] = value + if name.lower() == 'set-cookie': + # Django headers can't hold multiple values with the same name, + # so Set-Cookie headers go through the response's cookie jar. + django_response.cookies.load(value) + else: + django_response[name] = value return django_response diff --git a/xblock/test/django/test_request.py b/xblock/test/django/test_request.py index 0ca88dd22..72a8e77f3 100644 --- a/xblock/test/django/test_request.py +++ b/xblock/test/django/test_request.py @@ -70,6 +70,17 @@ def test_headers(self): self.assertIn('X-Foo', self._as_django(headerlist=[('X-Foo', 'bar')])) self.assertEqual(self._as_django(headerlist=[('X-Foo', 'bar')])['X-Foo'], 'bar') + def test_multiple_set_cookie_headers(self): + response = self._as_django(headerlist=[ + ('Set-Cookie', 'yummy_cookie=choco; Path=/; HttpOnly'), + ('Set-Cookie', 'tasty_cookie=strawberry; SameSite=Lax'), + ]) + self.assertEqual(response.cookies['yummy_cookie'].value, 'choco') + self.assertEqual(response.cookies['yummy_cookie']['path'], '/') + self.assertTrue(response.cookies['yummy_cookie']['httponly']) + self.assertEqual(response.cookies['tasty_cookie'].value, 'strawberry') + self.assertEqual(response.cookies['tasty_cookie']['samesite'], 'Lax') + def test_content_types(self): # JSON content type (no charset should be returned) self.assertEqual( @@ -105,6 +116,14 @@ def test_headers(self): self.assertIn('X-Foo', self._as_django(headerlist=[('X-Foo', 'bar')])) self.assertEqual(self._as_django(headerlist=[('X-Foo', 'bar')])['X-Foo'], 'bar') + def test_multiple_set_cookie_headers(self): + response = self._as_django(headerlist=[ + ('Set-Cookie', 'yummy_cookie=choco'), + ('Set-Cookie', 'tasty_cookie=strawberry'), + ]) + self.assertEqual(response.cookies['yummy_cookie'].value, 'choco') + self.assertEqual(response.cookies['tasty_cookie'].value, 'strawberry') + def test_content_types(self): # JSON content type (no charset should be returned) self.assertEqual( From 297e1455418d2702195587703d619a3d9b21ca3e Mon Sep 17 00:00:00 2001 From: Christopher Pruijsen Date: Mon, 26 Oct 2026 16:49:44 +0000 Subject: [PATCH 2/2] fix: reject Set-Cookie headers that do not parse to one cookie SimpleCookie parses with the request Cookie grammar, so loading a Set-Cookie header straight into the response's cookie jar could drop attributes, drop the cookie, or add a second cookie. Parse each header on its own and raise BadHeaderError unless it yields exactly one cookie. --- xblock/django/request.py | 12 ++++++++++-- xblock/test/django/test_request.py | 7 +++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/xblock/django/request.py b/xblock/django/request.py index bbe8f71ac..20375c741 100644 --- a/xblock/django/request.py +++ b/xblock/django/request.py @@ -1,9 +1,10 @@ """Helpers for WebOb requests and responses.""" +from http.cookies import SimpleCookie from itertools import chain, repeat from lazy import lazy import webob -from django.http import StreamingHttpResponse, HttpResponse +from django.http import BadHeaderError, StreamingHttpResponse, HttpResponse from webob.multidict import MultiDict, NestedMultiDict, NoVars @@ -25,7 +26,14 @@ def webob_to_django_response(webob_response, streaming=False): if name.lower() == 'set-cookie': # Django headers can't hold multiple values with the same name, # so Set-Cookie headers go through the response's cookie jar. - django_response.cookies.load(value) + # SimpleCookie parses with the request Cookie grammar, so a header + # that does not parse to exactly one cookie would be changed or + # dropped; fail loudly instead. + cookie = SimpleCookie() + cookie.load(value) + if len(cookie) != 1: + raise BadHeaderError("Set-Cookie header did not parse to exactly one cookie") + django_response.cookies.update(cookie) else: django_response[name] = value return django_response diff --git a/xblock/test/django/test_request.py b/xblock/test/django/test_request.py index 72a8e77f3..ca3dbb34e 100644 --- a/xblock/test/django/test_request.py +++ b/xblock/test/django/test_request.py @@ -7,6 +7,7 @@ from unittest import TestCase try: + from django.http import BadHeaderError from django.test.client import RequestFactory HAS_DJANGO = True except ImportError: @@ -81,6 +82,12 @@ def test_multiple_set_cookie_headers(self): self.assertEqual(response.cookies['tasty_cookie'].value, 'strawberry') self.assertEqual(response.cookies['tasty_cookie']['samesite'], 'Lax') + def test_set_cookie_header_that_is_not_one_cookie(self): + # Each of these would change or drop a cookie if loaded into the response's cookie jar. + for value in ('a=1; Path=/; Priority=High; HttpOnly', 'pref=x; sessionid=evil; Path=/', 'a b=1'): + with self.subTest(value=value), self.assertRaises(BadHeaderError): + self._as_django(headerlist=[('Set-Cookie', value)]) + def test_content_types(self): # JSON content type (no charset should be returned) self.assertEqual(