Opened 3 years ago

Closed 3 years ago

Last modified 16 months ago

#32189 closed Bug (invalid)

test.client.AsyncClient request POST failing

Reported by: perryjrandall Owned by: nobody
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When you try to access POST in a view with AsyncClient with different encoding type, there are several errors but POST is unusable

First due to the limitation in reading from FakePayload, in the real world you can ask to read more just you would not read more i think
I found http request was setting a high chunk size for an unrelated test and failing to read from fake payload

Then i notice that when trying to use a content type json not multipart (default) that the post data was empty again!
This time it seemed to be due to handling of application / json content

I think this approach works? But I'm super new to django internals please point me in the right direction for more tests!

patch to cause error

(ingredients) case@neuromancer:~/src/django/tests$ git show 
commit d78c05e4b3dc52940dd1ccdd6d988a7a3ff6efde (HEAD)
Author: case <case@errorspace.net>
Date:   Wed Nov 11 22:38:34 2020 -0800

    Alter tests to catch error

diff --git a/tests/test_client/tests.py b/tests/test_client/tests.py
index ef8312d1c0..6a75ac1e47 100644
--- a/tests/test_client/tests.py
+++ b/tests/test_client/tests.py
@@ -25,7 +25,7 @@ from unittest import mock
 
 from django.contrib.auth.models import User
 from django.core import mail
-from django.http import HttpResponse, HttpResponseNotAllowed
+from django.http import HttpResponse, HttpResponseNotAllowed, JsonResponse
 from django.test import (
     AsyncRequestFactory, Client, RequestFactory, SimpleTestCase, TestCase,
     override_settings,
@@ -998,16 +998,30 @@ class AsyncRequestFactoryTest(SimpleTestCase):
                 response = await async_generic_view(request)
                 self.assertEqual(response.status_code, 200)
 
-    async def test_request_factory_data(self):
+    async def test_request_factory_data_multipart(self):
         async def async_generic_view(request):
-            return HttpResponse(status=200, content=request.body)
+            return JsonResponse(request.POST)
+
+        request = self.request_factory.post(
+            '/somewhere/',
+            data={'example': 'data'},
+        )
+        self.assertEqual(request.headers['content-length'], '94', request.body)
+        self.assertEqual(request.headers['content-type'], 'multipart/form-data; boundary=BoUnDaRyStRiNg')
+        response = await async_generic_view(request)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, b'{"example": "data"}')
+
+    async def test_request_factory_data_json(self):
+        async def async_generic_view(request):
+            return JsonResponse(request.POST)
 
         request = self.request_factory.post(
             '/somewhere/',
             data={'example': 'data'},
             content_type='application/json',
         )
-        self.assertEqual(request.headers['content-length'], '19')
+        self.assertEqual(request.headers['content-length'], '19', request.body)
         self.assertEqual(request.headers['content-type'], 'application/json')
         response = await async_generic_view(request)
         self.assertEqual(response.status_code, 200)

Corresponding fix I think, the solution is intentionally super naive because I'm unsure of best way to address this comment welcome

commit a1c611ed47633aee79f30860b186d4c76c6b95b2 (fix_async_client_post)
Author: case <case@errorspace.net>
Date:   Wed Nov 11 22:38:56 2020 -0800

    Fix the async client

diff --git a/django/http/request.py b/django/http/request.py
index 2488bf9ccd..9551884a64 100644
--- a/django/http/request.py
+++ b/django/http/request.py
@@ -366,6 +366,12 @@ class HttpRequest:
                 raise
         elif self.content_type == 'application/x-www-form-urlencoded':
             self._post, self._files = QueryDict(self.body, encoding=self._encoding), MultiValueDict()
+        elif self.content_type == 'application/json':
+            self._post = QueryDict(encoding=self._encoding, mutable=True)
+            import json
+            for key, val in json.loads(self.body).items():
+                self._post[key] = val
+            self._files = MultiValueDict()
         else:
             self._post, self._files = QueryDict(encoding=self._encoding), MultiValueDict()
 
diff --git a/django/test/client.py b/django/test/client.py
index 2d501e0da6..f74faa2702 100644
--- a/django/test/client.py
+++ b/django/test/client.py
@@ -74,9 +74,16 @@ class FakePayload:
             self.read_started = True
         if num_bytes is None:
             num_bytes = self.__len or 0
-        assert self.__len >= num_bytes, "Cannot read more than the available bytes from the HTTP incoming data."
         content = self.__content.read(num_bytes)
-        self.__len -= num_bytes
+        return content
+
+    def readline(self, num_bytes=None):
+        if not self.read_started:
+            self.__content.seek(0)
+            self.read_started = True
+        if num_bytes is None:
+            num_bytes = self.__len or 0
+        content = self.__content.readline(num_bytes)
         return content
 
     def write(self, content):

Example error

(ingredients) case@neuromancer:~/src/django/tests$ ./runtests.py test_client.tests
Testing against Django installed in '/home/case/src/django/django' with up to 8 processes
Creating test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
Cloning test database for alias 'default'...
System check identified no issues (0 silenced).
.....F.......................................................................................
======================================================================
FAIL: test_request_factory_data_json (test_client.tests.AsyncRequestFactoryTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.8/unittest/case.py", line 60, in testPartExecutor
    yield
  File "/usr/lib/python3.8/unittest/case.py", line 676, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.8/unittest/case.py", line 633, in _callTestMethod
    method()
  File "/opt/venv/ingredients/lib/python3.8/site-packages/asgiref/sync.py", line 147, in __call__
    return call_result.result()
  File "/usr/lib/python3.8/concurrent/futures/_base.py", line 432, in result
    return self.__get_result()
  File "/usr/lib/python3.8/concurrent/futures/_base.py", line 388, in __get_result
    raise self._exception
  File "/opt/venv/ingredients/lib/python3.8/site-packages/asgiref/sync.py", line 212, in main_wrap
    result = await self.awaitable(*args, **kwargs)
  File "/home/case/src/django/tests/test_client/tests.py", line 1028, in test_request_factory_data_json
    self.assertEqual(response.content, b'{"example": "data"}')
  File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: b'{}' != b'{"example": "data"}'

----------------------------------------------------------------------
Ran 93 tests in 0.470s

Change History (3)

comment:1 by Carlton Gibson, 3 years ago

Resolution: invalid
Status: newclosed

This is expected behaviour. It's exactly the same using the synchronous Client.

request.POST is not populated for non-form requests, where you should use `request.body`

Adjusting the view in your test case:

from json import loads

async def async_generic_view(request):
       parsed_body = loads(request.body)
       return JsonResponse(parsed_body)

comment:2 by perryjrandall, 3 years ago

Wow you're totally right, I guess I misunderstood that part of the framework

Thanks for clarifying!

comment:3 by Tim Graham, 16 months ago

Component: UncategorizedTesting framework

While there aren't explicit steps to reproduce here, the first part of the description ("First due to the limitation in reading from FakePayload...") may be #34063.

Note: See TracTickets for help on using tickets.
Back to Top