Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure introduced from 3.8.5 --> 3.8.6; Passing on 3.9+ #8122

Closed
1 task done
Jibola opened this issue Jan 31, 2024 · 4 comments
Closed
1 task done

Failure introduced from 3.8.5 --> 3.8.6; Passing on 3.9+ #8122

Jibola opened this issue Jan 31, 2024 · 4 comments
Labels

Comments

@Jibola
Copy link

Jibola commented Jan 31, 2024

Describe the bug

We ran into a bug on our tests and started getting this error. After upgrading from 3.8.5 --> 3.8.6 on py311. However, once we updated to 3.9 the error disappeared. Are folks aware of why this issue occurred or does anyone know what it was related to?

To Reproduce

Our test creates a AIOHTTP server and then issues a get requests against a route we add that just holds a filename.

Expected behavior

We expected a 200 type response but instead received a bad HTTP error.

Logs/tracebacks

[2023/11/08 11:25:53.741] FAILURE: aiohttp.client_exceptions.ClientResponseError: 400, message="Expected HTTP/:\n\n  b'0'\n    ^", url=URL('http://localhost:8088/fs/foo') ()
 [2023/11/08 11:25:53.741] self = <ClientResponse(http://localhost:8088/fs/foo) [None None]>
 [2023/11/08 11:25:53.741] None
 [2023/11/08 11:25:53.741] connection = Connection<ConnectionKey(host='localhost', port=8088, is_ssl=False, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=None)>
 [2023/11/08 11:25:53.741]     async def start(self, connection: "Connection") -> "ClientResponse":
 [2023/11/08 11:25:53.741]         """Start response processing."""
 [2023/11/08 11:25:53.741]         self._closed = False
 [2023/11/08 11:25:53.741]         self._protocol = connection.protocol
 [2023/11/08 11:25:53.741]         self._connection = connection
 [2023/11/08 11:25:53.741] 
 [2023/11/08 11:25:53.741]         with self._timer:
 [2023/11/08 11:25:53.741]             while True:
 [2023/11/08 11:25:53.741]                 # read response
 [2023/11/08 11:25:53.741]                 try:
 [2023/11/08 11:25:53.741]                     protocol = self._protocol
 [2023/11/08 11:25:53.741] >                   message, payload = await protocol.read()  # type: ignore[union-attr]
 [2023/11/08 11:25:53.741] .tox/test-pymongo-latest/lib/python3.7/site-packages/aiohttp/client_reqrep.py:905:
 [2023/11/08 11:25:53.741] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2023/11/08 11:25:53.741] .tox/test-pymongo-latest/lib/python3.7/site-packages/aiohttp/streams.py:616: in read
 [2023/11/08 11:25:53.741]     await self._waiter
 [2023/11/08 11:25:53.741] .tox/test-pymongo-latest/lib/python3.7/site-packages/aiohttp/client_proto.py:213: in data_received
 [2023/11/08 11:25:53.741]     messages, upgraded, tail = self._parser.feed_data(data)
 [2023/11/08 11:25:53.741] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2023/11/08 11:25:53.741] >   ???
 [2023/11/08 11:25:53.741] E   aiohttp.http_exceptions.BadHttpMessage: 400, message:
 [2023/11/08 11:25:53.741] E     Expected HTTP/:
 [2023/11/08 11:25:53.741] E
 [2023/11/08 11:25:53.741] E       b'0'
 [2023/11/08 11:25:53.741] E         ^
 [2023/11/08 11:25:53.741] aiohttp/_http_parser.pyx:557: BadHttpMessage
 [2023/11/08 11:25:53.741] The above exception was the direct cause of the following exception:
 [2023/11/08 11:25:53.741] self = <test.asyncio_tests.test_aiohttp_gridfs.AIOHTTPGridFSHandlerTest testMethod=test_basic>
 [2023/11/08 11:25:53.741]     @asyncio_test
 [2023/11/08 11:25:53.741]     async def test_basic(self):
 [2023/11/08 11:25:53.741]         await self.start_app()
 [2023/11/08 11:25:53.741]         # First request
 [2023/11/08 11:25:53.741]         response = await self.get("/fs/foo")
 [2023/11/08 11:25:53.741] 
 [2023/11/08 11:25:53.741]         self.assertEqual(200, response.status)
 [2023/11/08 11:25:53.741]         self.assertEqual(self.contents, (await response.read()))
 [2023/11/08 11:25:53.741]         self.assertEqual(len(self.contents), int(response.headers["Content-Length"]))
 [2023/11/08 11:25:53.741]         self.assertEqual("my type", response.headers["Content-Type"])
 [2023/11/08 11:25:53.741]         self.assertEqual("public", response.headers["Cache-Control"])
 [2023/11/08 11:25:53.741]         self.assertTrue("Expires" not in response.headers)
 [2023/11/08 11:25:53.741] 
 [2023/11/08 11:25:53.741]         etag = response.headers["Etag"]
 [2023/11/08 11:25:53.741]         last_mod_dt = parse_date(response.headers["Last-Modified"])
 [2023/11/08 11:25:53.741]         self.assertEqual(self.contents_hash, etag.strip('"'))
 [2023/11/08 11:25:53.741]         self.assertTrue(self.put_start <= last_mod_dt <= self.put_end)
 [2023/11/08 11:25:53.741] 
 [2023/11/08 11:25:53.741]         # Now check we get 304 NOT MODIFIED responses as appropriate
 [2023/11/08 11:25:53.741]         for ims_value in (last_mod_dt, last_mod_dt + datetime.timedelta(seconds=1)):
 [2023/11/08 11:25:53.741] >           response = await self.get("/fs/foo", if_modified_since=ims_value)
 [2023/11/08 11:25:53.741] test/asyncio_tests/test_aiohttp_gridfs.py:149:
 [2023/11/08 11:25:53.741] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2023/11/08 11:25:53.741] test/asyncio_tests/test_aiohttp_gridfs.py:107: in request
 [2023/11/08 11:25:53.741]     resp = await method("http://localhost:8088%s" % path, headers=headers)
 [2023/11/08 11:25:53.741] .tox/test-pymongo-latest/lib/python3.7/site-packages/aiohttp/client.py:586: in _request
 [2023/11/08 11:25:53.741]     await resp.start(conn)
 [2023/11/08 11:25:53.741] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2023/11/08 11:25:53.741] self = <ClientResponse(http://localhost:8088/fs/foo) [None None]>
 [2023/11/08 11:25:53.741] None
 [2023/11/08 11:25:53.741] connection = Connection<ConnectionKey(host='localhost', port=8088, is_ssl=False, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=None)>
 [2023/11/08 11:25:53.741]     async def start(self, connection: "Connection") -> "ClientResponse":
 [2023/11/08 11:25:53.741]         """Start response processing."""
 [2023/11/08 11:25:53.741]         self._closed = False
 [2023/11/08 11:25:53.741]         self._protocol = connection.protocol
 [2023/11/08 11:25:53.741]         self._connection = connection
 [2023/11/08 11:25:53.741] 
 [2023/11/08 11:25:53.741]         with self._timer:
 [2023/11/08 11:25:53.741]             while True:
 [2023/11/08 11:25:53.741]                 # read response
 [2023/11/08 11:25:53.741]                 try:
 [2023/11/08 11:25:53.741]                     protocol = self._protocol
 [2023/11/08 11:25:53.741]                     message, payload = await protocol.read()  # type: ignore[union-attr]
 [2023/11/08 11:25:53.741]                 except http.HttpProcessingError as exc:
 [2023/11/08 11:25:53.741]                     raise ClientResponseError(
 [2023/11/08 11:25:53.741]                         self.request_info,
 [2023/11/08 11:25:53.741]                         self.history,
 [2023/11/08 11:25:53.741]                         status=exc.code,
 [2023/11/08 11:25:53.741]                         message=exc.message,
 [2023/11/08 11:25:53.741]                         headers=exc.headers,
 [2023/11/08 11:25:53.741] >                   ) from exc
 [2023/11/08 11:25:53.741] E                   aiohttp.client_exceptions.ClientResponseError: 400, message="Expected HTTP/:\n\n  b'0'\n    ^", url=URL('http://localhost:8088/fs/foo')
 [2023/11/08 11:25:53.741] .tox/test-pymongo-latest/lib/python3.7/site-packages/aiohttp/client_reqrep.py:913: ClientResponseError 


### Python Version

```console
$ python --version 
3.11

aiohttp Version

$ python -m pip show aiohttp
Version: 3.8.6
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /opt/homebrew/Caskroom/miniforge/base/envs/example/lib/python3.11/site-packages
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /opt/homebrew/Caskroom/miniforge/base/envs/example/lib/python3.11/site-packages
Requires: 
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Version: 1.9.4
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Location: /opt/homebrew/Caskroom/miniforge/base/envs/example/lib/python3.11/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

MacOS Sonoma 14.3 | Red Hat Enterprise Linux 8.4

Related component

Server, Client

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@Jibola Jibola added the bug label Jan 31, 2024
@webknjaz
Copy link
Member

Looks relate to the security fixes. Do you have a reproducer. Also, the current stream is 3.9.x. We won't be resurrecting 3.8. So I'm going to close this as wontfix.

@webknjaz webknjaz closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2024
@ShaneHarvey
Copy link

Here is a repro script:

import asyncio
import datetime
import email
import hashlib
import sys
import time

import aiohttp
import aiohttp.web


def format_date(d):
    return time.strftime("%a, %d %b %Y %H:%M:%S GMT", d.utctimetuple())


def parse_date(d):
    date_tuple = email.utils.parsedate(d)
    return datetime.datetime.fromtimestamp(time.mktime(date_tuple))


async def start_app(loop):
    app = aiohttp.web.Application()
    resource = app.router.add_resource("/fs/{filename}")
    handler = AioHttpHandler({"foo": {"data": b"foo", "upload_date": datetime.datetime.now()}})
    resource.add_route("GET", handler)
    resource.add_route("HEAD", handler)

    app_handler = app.make_handler()
    server = loop.create_server(app_handler, host="localhost", port=8088)

    srv, _ = await asyncio.gather(server, app.startup())
    return app, app_handler, srv


async def request(method, path, if_modified_since=None, headers=None):
    headers = headers or {}
    if if_modified_since:
        headers["If-Modified-Since"] = format_date(if_modified_since)

    session = aiohttp.ClientSession()

    try:
        method = getattr(session, method)

        resp = await method("http://localhost:8088%s" % path, headers=headers)
        await resp.read()
        return resp
    finally:
        await session.close()


class AioHttpHandler:
    def __init__(self, files):
        self.files = files

    async def __call__(self, request):
        """Send filepath to client using request."""
        try:
            filename = request.match_info["filename"]
        except KeyError:
            raise aiohttp.web.HTTPInternalServerError(text="Missing filename on request")

        if request.method not in ("GET", "HEAD"):
            raise aiohttp.web.HTTPMethodNotAllowed(
                method=request.method, allowed_methods={"GET", "HEAD"}
            )

        try:
            file = self.files[filename]
        except KeyError:
            raise aiohttp.web.HTTPNotFound(text=request.path)

        data = file["data"]
        upload_date = file["upload_date"]

        resp = aiohttp.web.StreamResponse()

        checksum = hashlib.sha256(data).hexdigest()

        resp.last_modified = upload_date
        resp.headers["Etag"] = '"%s"' % checksum
        resp.headers["Cache-Control"] = "public"

        # Check the If-Modified-Since, and don't send the result if the
        # content has not been modified
        ims_value = request.if_modified_since
        if ims_value is not None:
            # If our MotorClient is tz-aware, assume the naive ims_value is in
            # its time zone.
            if_since = ims_value.replace(tzinfo=upload_date.tzinfo)
            modified = upload_date.replace(microsecond=0)
            if if_since >= modified:
                resp.set_status(304)
                return resp

        # Same for Etag
        etag = request.headers.get("If-None-Match")
        if etag is not None and etag.strip('"') == checksum:
            resp.set_status(304)
            return resp

        resp.content_length = len(data)
        await resp.prepare(request)

        if request.method == "GET":
            await resp.write(data)
        return resp


async def test():
    # First request
    print('request: get "/fs/foo"')
    response = await request("get", "/fs/foo")

    assert response.status == 200
    assert (await response.read()) == b"foo"
    assert int(response.headers["Content-Length"]) == len(b"foo")
    assert response.headers["Cache-Control"] == "public"
    assert "Expires" not in response.headers

    last_mod_dt = parse_date(response.headers["Last-Modified"])

    # If-Modified-Since in the past, get whole response back
    past = last_mod_dt - datetime.timedelta(seconds=10)
    print(f'request: get "/fs/foo" if_modified_since={past}')
    response = await request("get", "/fs/foo", if_modified_since=past)
    assert response.status == 200
    assert (await response.read()) == b"foo"

    # Now check we get 304 NOT MODIFIED responses as appropriate
    future = last_mod_dt + datetime.timedelta(seconds=10)
    print(f'request: get "/fs/foo" if_modified_since={future}')
    response = await request("get", "/fs/foo", if_modified_since=future)
    assert response.status == 304
    assert (await response.read()) == b""


async def main():
    loop = asyncio.get_running_loop()
    app, app_handler, srv = await start_app(loop)
    await test()
    # aiohttp.rtfd.io/en/stable/web.html#aiohttp-web-graceful-shutdown
    srv.close()
    await srv.wait_closed()
    await app.shutdown()
    await app_handler.shutdown(timeout=1)
    await app.cleanup()


if __name__ == "__main__":
    print(f"Running with python: {sys.version}, aiohttp: {aiohttp.__version__}")
    asyncio.run(main())

Output on 3.8.6:

$ python repro-if-modified-since.py                 
Running with python: 3.10.11 (v3.10.11:7d4cc5aa85, Apr  4 2023, 19:05:19) [Clang 13.0.0 (clang-1300.0.29.30)], aiohttp: 3.8.6
/Users/shane/git/motor/repro-if-modified-since.py:28: DeprecationWarning: Application.make_handler(...) is deprecated, use AppRunner API instead
  app_handler = app.make_handler()
request: get "/fs/foo"
request: get "/fs/foo" if_modified_since=2024-01-31 16:10:35
request: get "/fs/foo" if_modified_since=2024-01-31 16:10:55
Traceback (most recent call last):
  File "/Users/shane/work/pycharm/motor-py310/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 905, in start
    message, payload = await protocol.read()  # type: ignore[union-attr]
  File "/Users/shane/work/pycharm/motor-py310/lib/python3.10/site-packages/aiohttp/streams.py", line 616, in read
    await self._waiter
  File "/Users/shane/work/pycharm/motor-py310/lib/python3.10/site-packages/aiohttp/client_proto.py", line 213, in data_received
    messages, upgraded, tail = self._parser.feed_data(data)
  File "aiohttp/_http_parser.pyx", line 557, in aiohttp._http_parser.HttpParser.feed_data
aiohttp.http_exceptions.BadHttpMessage: 400, message:
  Expected HTTP/:

    b'0'
      ^

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/shane/git/motor/repro-if-modified-since.py", line 152, in <module>
    asyncio.run(main())
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "/Users/shane/git/motor/repro-if-modified-since.py", line 141, in main
    await test()
  File "/Users/shane/git/motor/repro-if-modified-since.py", line 133, in test
    response = await request("get", "/fs/foo", if_modified_since=future)
  File "/Users/shane/git/motor/repro-if-modified-since.py", line 45, in request
    resp = await method("http://localhost:8088%s" % path, headers=headers)
  File "/Users/shane/work/pycharm/motor-py310/lib/python3.10/site-packages/aiohttp/client.py", line 586, in _request
    await resp.start(conn)
  File "/Users/shane/work/pycharm/motor-py310/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 907, in start
    raise ClientResponseError(
aiohttp.client_exceptions.ClientResponseError: 400, message="Expected HTTP/:\n\n  b'0'\n    ^", url=URL('http://localhost:8088/fs/foo')

On 3.8.5 or >=3.9.0

$ python repro-if-modified-since.py
Running with python: 3.10.11 (v3.10.11:7d4cc5aa85, Apr  4 2023, 19:05:19) [Clang 13.0.0 (clang-1300.0.29.30)], aiohttp: 3.8.5
/Users/shane/git/motor/repro-if-modified-since.py:28: DeprecationWarning: Application.make_handler(...) is deprecated, use AppRunner API instead
  app_handler = app.make_handler()
request: get "/fs/foo"
request: get "/fs/foo" if_modified_since=2024-01-31 16:11:53
request: get "/fs/foo" if_modified_since=2024-01-31 16:12:13

It appears the bug is related to if_modified_since and HTTP 304 status responses.

Is there a aiohttp test to ensure this bug doesn't regress?

@webknjaz
Copy link
Member

webknjaz commented Feb 1, 2024

No idea. But feel free to submit PRs improving the test coverage.

@Dreamsorcerer
Copy link
Member

Maybe to do with empty body? #7756
Otherwise, no idea, llhttp hasn't been updated since then..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants