fix issue where request.args.pop removed parameters inconsistently (#2111)

Fix uvloop 1912

sanic/__version__.py

@@ -1 +1 @@
-__version__ = "19.12.3"
+__version__ = "19.12.5"

sanic/app.py

@@ -1474,3 +1474,5 @@ class Sanic:
         self.asgi = True
         asgi_app = await ASGIApp.create(self, scope, receive, send)
         await asgi_app()
+
+    _asgi_single_callable = True  # We conform to ASGI 3.0 single-callable

sanic/asgi.py

@@ -1,5 +1,6 @@
 import asyncio
 import warnings
+
 from inspect import isawaitable
 from typing import (
     Any,
@@ -15,6 +16,7 @@ from typing import (
 from urllib.parse import quote
 
 import sanic.app  # noqa
+
 from sanic.compat import Header
 from sanic.exceptions import InvalidUsage, ServerError
 from sanic.log import logger
@@ -23,6 +25,7 @@ from sanic.response import HTTPResponse, StreamingHTTPResponse
 from sanic.server import StreamBuffer
 from sanic.websocket import WebSocketConnection
 
+
 ASGIScope = MutableMapping[str, Any]
 ASGIMessage = MutableMapping[str, Any]
 ASGISend = Callable[[ASGIMessage], Awaitable[None]]
@@ -65,7 +68,9 @@ class MockProtocol:
 class MockTransport:
     _protocol: Optional[MockProtocol]
 
-    def __init__(self, scope: ASGIScope, receive: ASGIReceive, send: ASGISend) -> None:
+    def __init__(
+        self, scope: ASGIScope, receive: ASGIReceive, send: ASGISend
+    ) -> None:
         self.scope = scope
         self._receive = receive
         self._send = send
@@ -141,7 +146,9 @@ class Lifespan:
         ) + self.asgi_app.sanic_app.listeners.get("after_server_start", [])
 
         for handler in listeners:
-            response = handler(self.asgi_app.sanic_app, self.asgi_app.sanic_app.loop)
+            response = handler(
+                self.asgi_app.sanic_app, self.asgi_app.sanic_app.loop
+            )
             if isawaitable(response):
                 await response
 
@@ -159,7 +166,9 @@ class Lifespan:
         ) + self.asgi_app.sanic_app.listeners.get("after_server_stop", [])
 
         for handler in listeners:
-            response = handler(self.asgi_app.sanic_app, self.asgi_app.sanic_app.loop)
+            response = handler(
+                self.asgi_app.sanic_app, self.asgi_app.sanic_app.loop
+            )
             if isawaitable(response):
                 await response
 
@@ -204,13 +213,19 @@ class ASGIApp:
                 for key, value in scope.get("headers", [])
             ]
         )
-        instance.do_stream = True if headers.get("expect") == "100-continue" else False
+        instance.do_stream = (
+            True if headers.get("expect") == "100-continue" else False
+        )
         instance.lifespan = Lifespan(instance)
 
         if scope["type"] == "lifespan":
             await instance.lifespan(scope, receive, send)
         else:
-            path = scope["path"][1:] if scope["path"].startswith("/") else scope["path"]
+            path = (
+                scope["path"][1:]
+                if scope["path"].startswith("/")
+                else scope["path"]
+            )
             url = "/".join([scope.get("root_path", ""), quote(path)])
             url_bytes = url.encode("latin-1")
             url_bytes += b"?" + scope["query_string"]
@@ -233,11 +248,18 @@ class ASGIApp:
 
             request_class = sanic_app.request_class or Request
             instance.request = request_class(
-                url_bytes, headers, version, method, instance.transport, sanic_app,
+                url_bytes,
+                headers,
+                version,
+                method,
+                instance.transport,
+                sanic_app,
             )
 
             if sanic_app.is_request_stream:
-                is_stream_handler = sanic_app.router.is_stream_handler(instance.request)
+                is_stream_handler = sanic_app.router.is_stream_handler(
+                    instance.request
+                )
                 if is_stream_handler:
                     instance.request.stream = StreamBuffer(
                         sanic_app.config.REQUEST_BUFFER_QUEUE_SIZE
@@ -287,13 +309,17 @@ class ASGIApp:
         callback = None if self.ws else self.stream_callback
         await handler(self.request, None, callback)
 
+    _asgi_single_callable = True  # We conform to ASGI 3.0 single-callable
+
     async def stream_callback(self, response: HTTPResponse) -> None:
         """
         Write the response.
         """
         headers: List[Tuple[bytes, bytes]] = []
         cookies: Dict[str, str] = {}
+        content_length: List[str] = []
         try:
+            content_length = response.headers.popall("content-length", [])
             cookies = {
                 v.key: v
                 for _, v in list(
@@ -316,7 +342,9 @@ class ASGIApp:
                 type(response),
             )
             exception = ServerError("Invalid response type")
-            response = self.sanic_app.error_handler.response(self.request, exception)
+            response = self.sanic_app.error_handler.response(
+                self.request, exception
+            )
             headers = [
                 (str(name).encode("latin-1"), str(value).encode("latin-1"))
                 for name, value in response.headers.items()
@@ -324,14 +352,28 @@ class ASGIApp:
             ]
 
         response.asgi = True
-
-        if "content-length" not in response.headers and not isinstance(
-            response, StreamingHTTPResponse
-        ):
-            headers += [(b"content-length", str(len(response.body)).encode("latin-1"))]
+        is_streaming = isinstance(response, StreamingHTTPResponse)
+        if is_streaming and getattr(response, "chunked", False):
+            # disable sanic chunking, this is done at the ASGI-server level
+            setattr(response, "chunked", False)
+            # content-length header is removed to signal to the ASGI-server
+            # to use automatic-chunking if it supports it
+        elif len(content_length) > 0:
+            headers += [
+                (b"content-length", str(content_length[0]).encode("latin-1"))
+            ]
+        elif not is_streaming:
+            headers += [
+                (
+                    b"content-length",
+                    str(len(getattr(response, "body", b""))).encode("latin-1"),
+                )
+            ]
 
         if "content-type" not in response.headers:
-            headers += [(b"content-type", str(response.content_type).encode("latin-1"))]
+            headers += [
+                (b"content-type", str(response.content_type).encode("latin-1"))
+            ]
 
         if response.cookies:
             cookies.update(
@@ -343,7 +385,8 @@ class ASGIApp:
             )
 
         headers += [
-            (b"set-cookie", cookie.encode("utf-8")) for k, cookie in cookies.items()
+            (b"set-cookie", cookie.encode("utf-8"))
+            for k, cookie in cookies.items()
         ]
 
         await self.transport.send(

sanic/request.py

@@ -260,9 +260,12 @@ class Request:
         :type errors: str
         :return: RequestParameters
         """
-        if not self.parsed_args[
-            (keep_blank_values, strict_parsing, encoding, errors)
-        ]:
+        if (
+            keep_blank_values,
+            strict_parsing,
+            encoding,
+            errors,
+        ) not in self.parsed_args:
             if self.query_string:
                 self.parsed_args[
                     (keep_blank_values, strict_parsing, encoding, errors)
@@ -328,9 +331,12 @@ class Request:
         :type errors: str
         :return: list
         """
-        if not self.parsed_not_grouped_args[
-            (keep_blank_values, strict_parsing, encoding, errors)
-        ]:
+        if (
+            keep_blank_values,
+            strict_parsing,
+            encoding,
+            errors,
+        ) not in self.parsed_not_grouped_args:
             if self.query_string:
                 self.parsed_not_grouped_args[
                     (keep_blank_values, strict_parsing, encoding, errors)

sanic/response.py

@@ -80,6 +80,8 @@ class StreamingHTTPResponse(BaseHTTPResponse):
         if type(data) != bytes:
             data = self._encode_body(data)
 
+        # `chunked` will always be False in ASGI-mode, even if the underlying
+        # ASGI Transport implements Chunked transport. That does it itself.
         if self.chunked:
             await self.protocol.push_data(b"%x\r\n%b\r\n" % (len(data), data))
         else:

setup.py

@@ -5,6 +5,7 @@ import codecs
 import os
 import re
 import sys
+
 from distutils.util import strtobool
 
 from setuptools import setup
@@ -24,6 +25,7 @@ class PyTest(TestCommand):
 
     def run_tests(self):
         import shlex
+
         import pytest
 
         errno = pytest.main(shlex.split(self.pytest_args))
@@ -38,7 +40,9 @@ def open_local(paths, mode="r", encoding="utf8"):
 
 with open_local(["sanic", "__version__.py"], encoding="latin1") as fp:
     try:
-        version = re.findall(r"^__version__ = \"([^']+)\"\r?$", fp.read(), re.M)[0]
+        version = re.findall(
+            r"^__version__ = \"([^']+)\"\r?$", fp.read(), re.M
+        )[0]
     except IndexError:
         raise RuntimeError("Unable to determine version.")
 
@@ -68,9 +72,11 @@ setup_kwargs = {
     ],
 }
 
-env_dependency = '; sys_platform != "win32" ' 'and implementation_name == "cpython"'
+env_dependency = (
+    '; sys_platform != "win32" ' 'and implementation_name == "cpython"'
+)
 ujson = "ujson>=1.35" + env_dependency
-uvloop = "uvloop>=0.5.3" + env_dependency
+uvloop = "uvloop>=0.5.3,<0.15.0" + env_dependency
 
 requirements = [
     "httptools>=0.0.10",

tests/test_requests.py

@@ -244,6 +244,17 @@ def test_query_string(app):
     assert request.args.getlist("test1") == ["1"]
     assert request.args.get("test3", default="My value") == "My value"
 
+def test_popped_stays_popped(app):
+    @app.route("/")
+    async def handler(request):
+        return text("OK")
+
+    request, response = app.test_client.get(
+        "/", params=[("test1", "1")]
+    )
+
+    assert request.args.pop("test1") == ["1"]
+    assert "test1" not in request.args
 
 @pytest.mark.asyncio
 async def test_query_string_asgi(app):

tests/test_response.py

@@ -235,7 +235,7 @@ def test_chunked_streaming_returns_correct_content(streaming_app):
 @pytest.mark.asyncio
 async def test_chunked_streaming_returns_correct_content_asgi(streaming_app):
     request, response = await streaming_app.asgi_client.get("/")
-    assert response.text == "4\r\nfoo,\r\n3\r\nbar\r\n0\r\n\r\n"
+    assert response.text == "foo,bar"
 
 
 def test_non_chunked_streaming_adds_correct_headers(non_chunked_streaming_app):

tox.ini

@@ -14,7 +14,7 @@ deps =
     pytest-sugar
     httpcore==0.3.0
     httpx==0.9.3
-    chardet<=2.3.0
+    multidict==5.0.0
     beautifulsoup4
     gunicorn
     pytest-benchmark