Opened 8 months ago
Last modified 2 days ago
#35414 assigned Bug
Issue with AsyncClient ignoring default headers compared to synchronous Client
Reported by: | 설원준(Wonjoon Seol)/Dispatch squad | Owned by: | YashRaj1506 |
---|---|---|---|
Component: | HTTP handling | Version: | 5.0 |
Severity: | Normal | Keywords: | AsyncClient, ASGIRequest |
Cc: | Andrew Godwin, Carlton Gibson | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently, there is an inconsistency between Django's asynchronous AsyncClient and its synchronous counterpart Client regarding the handling of default headers. While the synchronous Client correctly includes default headers, the asynchronous AsyncClient ignores them. This behavior leads to discrepancies when utilizing fixtures with default headers, causing tests to fail unexpectedly.
Reproduction Steps:
- Set up a fixture with default headers for both synchronous and asynchronous clients.
- Utilize the fixtures in test cases and observe the behavior.
- Notice that the synchronous client includes default headers as expected, while the asynchronous client does not.
Code Snippets:
@pytest.fixture(scope="session") def jwt_token(token_payload: dict[str, Any]) -> str: return jwt.encode({"abc", '"abc"}, key='123', algorithm="HS256") # this passes HTTP_AUTHORIZATION default header @pytest.fixture(scope="session") def sync_client_with_token(jwt_token) -> Generator[Client, None, None]: yield Client(HTTP_AUTHORIZATION=f"Bearer {jwt_token}") # this does not @pytest.fixture(scope="session") async def async_client_with_token(jwt_token) -> AsyncIterator[AsyncClient]: async_client = AsyncClient(HTTP_AUTHORIZATION=f"Bearer {jwt_token}") # async_client.defaults["AUTHORIZATION"] = f"Bearer {jwt_token}" yield async_client
AsyncRequestFactory.generic() does not currently check if self.defaults exists and ASGIRequest only check hard-coded header names in init() method, effectively ignoring rest of the self.scope values.
Note that while RequestFactory.generic() method does not check whether self.defaults exist but WSGIRequest receives default values via ._base_environ() method when creating WSGIRequest instance.
Proposed Solutions:
Fix Method 1: Modify AsyncRequestFactory.generic() method
class AsyncRequestFactory(RequestFactory): def generic( self, method, path, data="", content_type="application/octet-stream", secure=False, *, headers=None, **extra, ): """Construct an arbitrary HTTP request.""" parsed = urlparse(str(path)) # path can be lazy. data = force_bytes(data, settings.DEFAULT_CHARSET) s = { "method": method, "path": self._get_path(parsed), "server": ("127.0.0.1", "443" if secure else "80"), "scheme": "https" if secure else "http", "headers": [(b"host", b"testserver")], } if self.defaults: # <- added extra = {**self.defaults, **extra} <- added if data: s["headers"].extend( [ (b"content-length", str(len(data)).encode("ascii")), (b"content-type", content_type.encode("ascii")), ] ) s["_body_file"] = FakePayload(data) ...
Fix Method 2: Modify ASGIRequest
Alternatively, the ASGIRequest class can be adjusted to include default headers in the META attribute during initialisation.
class ASGIRequest(HttpRequest): def __init__(self, scope, body_file): self.scope = scope ... if isinstance(query_string, bytes): query_string = query_string.decode() self.META = { **self.scope, <- # Added "REQUEST_METHOD": self.method, "QUERY_STRING": query_string, "SCRIPT_NAME": self.script_name, "PATH_INFO": self.path_info,
This would be simliar to WSGIRequest, where self.META = environ is set during the init phase.
However, it's worth noting that WSGI has a different META format.
asgi META
{'asgi': {'version': '3.0'}, 'type': 'http', 'http_version': '1.1', 'client': ['127.0.0.1', 0], 'server': ('127.0.0.1', '80'), 'scheme': 'http', 'method': 'GET', 'headers': [(b'host', b'testserver'), (b'cookie', b'')], 'HTTP_AUTHORIZATION': 'Bearer ...', 'path': '/campaigns/', 'query_string': 'date_type=created_at&date_from=2024-04-28+03%3A44%3A37.484747%2B00%3A00&date_to=2024-05-02+03%3A44%3A37.484759%2B00%3A00', 'REQUEST_METHOD': 'GET', 'QUERY_STRING': 'date_type=created_at&date_from=2024-04-28+03%3A44%3A37.484747%2B00%3A00&date_to=2024-05-02+03%3A44%3A37.484759%2B00%3A00', 'SCRIPT_NAME': '', 'PATH_INFO': '/campaigns/', 'wsgi.multithread': True, 'wsgi.multiprocess': True, 'REMOTE_ADDR': '127.0.0.1', 'REMOTE_HOST': '127.0.0.1', 'REMOTE_PORT': 0, 'SERVER_NAME': '127.0.0.1', 'SERVER_PORT': '80', 'HTTP_HOST': 'testserver', 'HTTP_COOKIE': ''}
ASGI META has separate 'headers' but the custom headers are not added there.
wsgi META
{'HTTP_COOKIE': '', 'PATH_INFO': '/campaigns/', 'REMOTE_ADDR': '127.0.0.1', 'REQUEST_METHOD': 'GET', 'SCRIPT_NAME': '', 'SERVER_NAME': 'testserver', 'SERVER_PORT': '80', 'SERVER_PROTOCOL': 'HTTP/1.1', 'wsgi.version': (1, 0), 'wsgi.url_scheme': 'http', 'wsgi.input': <django.test.client.FakePayload object at 0x1061b0d90>, 'wsgi.errors': <_io.BytesIO object at 0x104f42020>, 'wsgi.multiprocess': True, 'wsgi.multithread': False, 'wsgi.run_once': False, 'HTTP_AUTHORIZATION': 'Bearer ....', 'QUERY_STRING': 'date_type=created_at&date_from=2024-04-28+03%3A46%3A35.172175%2B00%3A00&date_to=2024-05-02+03%3A46%3A35.172190%2B00%3A00'}
Addressing this inconsistency will ensure that the behaviour of both synchronous and asynchronous clients remains consistent and predictable across Django applications.
Thanks.
Change History (21)
comment:1 by , 8 months ago
Description: | modified (diff) |
---|
comment:2 by , 8 months ago
Description: | modified (diff) |
---|
comment:3 by , 8 months ago
Description: | modified (diff) |
---|
comment:4 by , 8 months ago
Description: | modified (diff) |
---|
comment:5 by , 8 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:6 by , 8 months ago
Replying to Sarah Boyce:
Hi 설원준(Wonjoon Seol)/Dispatch squad 👋
I can see you are using pytest, which is an external dependency, there's a good chance you are using others.
I need to be able to replicate an issue using only Django and currently it's quite hard to understand what's going on.
I think Trac is not the right place to discuss this at this point, perhaps you want to ask for support on the Django forum or perhaps you want to raise this with a different dependency (perhaps pytest-django is more appropriate?).
If you can write a test case for Django that shows the issue without using pytest, then I think we can reopen the ticket 👍
Hi Sarah, the pytest fixture is only there to reduce boilerplates.
Here is your requested test case without dependencies.
Using django tutorial polls guide:
polls/views.py
from django.http import JsonResponse def index(request): data = {"message": "This is an example API response"} return JsonResponse(data)
middleware.py
class JWTMiddleware: def __init__(self, get_response): self.get_response = get_response def __call__(self, request): if 'HTTP_AUTHORIZATION' not in request.META: return JsonResponse({'error': 'Authorization header is missing'}, status=401) return self.get_response(request)
mysite/settings.py
MIDDLEWARE = [ 'django.middleware.security.SecurityMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'middleware.JWTMiddleware' # <- ]
polls/tests.py
from http import HTTPStatus from django.test import TestCase, AsyncClient from django.urls import reverse class EXAMPLE_TESTS(TestCase): async def test_should_return_ok( # Fails self, ) -> None: async_client = AsyncClient(HTTP_AUTHORIZATION=f"Bearer I_AM_JWT_TOKEN") # AUTHORIZATION, HTTP_AUTHORIZATION both fails due to the reason in the original post. response = await async_client.get( reverse("index"), ) self.assertEqual(response.status_code, HTTPStatus.OK) async def test_should_return_ok2( # Passes self, ) -> None: async_client = AsyncClient() response = await async_client.get( reverse("index"), AUTHORIZATION=f"Bearer I_AM_JWT_TOKEN" ) self.assertEqual(response.status_code, HTTPStatus.OK)
**printing META: (Customer header missing)** {'REQUEST_METHOD': 'GET', 'QUERY_STRING': '', 'SCRIPT_NAME': '', 'PATH_INFO': '/polls/', 'wsgi.multithread': True, 'wsgi.multiprocess': True, 'REMOTE_ADDR': '127.0.0.1', 'REMOTE_HOST': '127.0.0.1', 'REMOTE_PORT': 0, 'SERVER_NAME': '127.0.0.1', 'SERVER_PORT': '80', 'HTTP_HOST': 'testserver', 'HTTP_COOKIE': ''} F ====================================================================== FAIL: test_should_return_ok (polls.tests.EXAMPLE_TESTS.test_should_return_ok) ---------------------------------------------------------------------- Traceback (most recent call last): ... File "/Users/.../workspace/django-mvp/mysite/polls/tests.py", line 17, in test_should_return_ok self.assertEqual(response.status_code, HTTPStatus.OK) AssertionError: 401 != <HTTPStatus.OK: 200> Ran 2 tests in 0.012s FAILED (failures=1)
comment:7 by , 8 months ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
comment:8 by , 8 months ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
This works as expected and documented:
"In the initialization, arbitrary keyword arguments in defaults are added directly into the ASGI scope."
If you want to add default headers you should pass them in the headers
arguments.
comment:9 by , 8 months ago
Hi Mariusz,
Thanks for the documentation link. So the async client intended behaviour is inconsistent with the sync counterpart.
But I should've mentioned in my original post that the headers argument does not work neither.
Because converting the above example test using headers argument still fails.
class EXAMPLE_TESTS(TestCase): async def test_should_return_ok( # FAILS self, ) -> None: async_client = AsyncClient(headers={"AUTHORIZATION": f"Bearer I_AM_JWT_TOKEN"}) # AUTHORIZATION, HTTP_AUTHORIZATION would both fail response = await async_client.get( reverse("index"), ) self.assertEqual(response.status_code, HTTPStatus.OK) async def test_should_return_ok2( # Passes self, ) -> None: async_client = AsyncClient() response = await async_client.get( reverse("index"), AUTHORIZATION=f"Bearer I_AM_JWT_TOKEN" ) self.assertEqual(response.status_code, HTTPStatus.OK)
The reason is still due the the original post.
def _base_scope(self, **request): """The base scope for a request.""" # This is a minimal valid ASGI scope, plus: # - headers['cookie'] for cookie support, # - 'client' often useful, see #8551. scope = { "asgi": {"version": "3.0"}, "type": "http", "http_version": "1.1", "client": ["127.0.0.1", 0], "server": ("testserver", "80"), "scheme": "http", "method": "GET", "headers": [], # <- scope ignores default header **self.defaults, **request, } scope["headers"].append( ( b"cookie", b"; ".join( sorted( ("%s=%s" % (morsel.key, morsel.coded_value)).encode("ascii") for morsel in self.cookies.values() ) ), ) ) return scope
the scope only takes in default argument but ignores default header.
Using the example test, this is the constructed META header after the initialization.
The default headers are still missing:
{'REQUEST_METHOD': 'GET', 'QUERY_STRING': '', 'SCRIPT_NAME': '', 'PATH_INFO': '/polls/', 'wsgi.multithread': True, 'wsgi.multiprocess': True, 'REMOTE_ADDR': '127.0.0.1', 'REMOTE_HOST': '127.0.0.1', 'REMOTE_PORT': 0, 'SERVER_NAME': '127.0.0.1', 'SERVER_PORT': '80', 'HTTP_HOST': 'testserver', 'HTTP_COOKIE': ''}
Thanks.
comment:10 by , 8 months ago
Resolution: | invalid |
---|---|
Status: | closed → new |
comment:11 by , 7 months ago
Cc: | added |
---|
I've been thinking about this report for a while, and after making some experiments, I think this is a valid issue. Originally I thought this was a dupe of #32159 but I don't think that's the case.
What I can't put my finger on is whether there is an issue with the docs, or if the headers should really be added to the ASGI scope headers
key (as the reporter says). The docs shows:
AsyncClient has the same methods and signatures as the synchronous (normal) test client, with the following exceptions:
In the initialization, arbitrary keyword arguments in defaults are added directly into the ASGI scope.
Headers passed as extra keyword arguments should not have the HTTP_ prefix required by the synchronous client (see Client.get()). For example, here is how to set an HTTP Accept header:
To me this implies that the headers
parameter at class instantiation time could be use to set parameters to be used in every client operation (just like with Client
). But when using the test case provided by the reporter this is not the case (using a simple Django app with the provided middleware and a simple view):
from django.test import TestCase, AsyncClient class Ticket35414Tests(TestCase): async def test_should_return_ok(self): async_client = AsyncClient(headers={"AUTHORIZATION": "A Token"}) response = await async_client.get("/ticket_35414/") self.assertEqual(response.status_code, 200)
Failure:
====================================================================== FAIL: test_should_return_ok (ticket_35414.tests.Ticket35414Tests.test_should_return_ok) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/nessita/.virtualenvs/djangodev/lib/python3.11/site-packages/asgiref/sync.py", line 254, in __call__ return call_result.result() ^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/concurrent/futures/_base.py", line 449, in result return self.__get_result() ^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/concurrent/futures/_base.py", line 401, in __get_result raise self._exception File "/home/nessita/.virtualenvs/djangodev/lib/python3.11/site-packages/asgiref/sync.py", line 331, in main_wrap result = await self.awaitable(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/nessita/fellowship/projectfromrepo/ticket_35414/tests.py", line 9, in test_should_return_ok self.assertEqual(response.status_code, 200) AssertionError: 401 != 200
Carlton, Andrew, would you have an opinion?
comment:12 by , 7 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:13 by , 7 months ago
This looks correct to me.
The generic()
method takes in headers
and extra
and combines them before passing on to _base_scope()
:
def generic(...): ... if headers: extra.update(HttpHeaders.to_asgi_names(headers)) s["headers"] += [ (key.lower().encode("ascii"), value.encode("latin1")) for key, value in extra.items() ] return self.request(**s)
But then _base_scope()
isn't then correctly setting the headers
key in the scope (as reported):
def _base_scope(..., **request): ... "headers": [], # <- scope ignores default header **self.defaults, **request,
comment:14 by , 4 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:15 by , 2 weeks ago
Has patch: | set |
---|---|
Needs tests: | set |
comment:16 by , 11 days ago
Has patch: | unset |
---|---|
Needs tests: | unset |
comment:17 by , 11 days ago
Has patch: | set |
---|
comment:18 by , 6 days ago
Has patch: | unset |
---|
comment:19 by , 6 days ago
Has patch: | set |
---|
comment:21 by , 2 days ago
Needs tests: | set |
---|
Hi 설원준(Wonjoon Seol)/Dispatch squad 👋
I can see you are using pytest, which is an external dependency, there's a good chance you are using others.
I need to be able to replicate an issue using only Django and currently it's quite hard to understand what's going on.
I think Trac is not the right place to discuss this at this point, perhaps you want to ask for support on the Django forum or perhaps you want to raise this with a different dependency (perhaps pytest-django is more appropriate?).
If you can write a test case for Django that shows the issue without using pytest, then I think we can reopen the ticket 👍