-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Comments
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. |
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
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? |
No idea. But feel free to submit PRs improving the test coverage. |
Maybe to do with empty body? #7756 |
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
aiohttp Version
multidict Version
yarl Version
OS
MacOS Sonoma 14.3 | Red Hat Enterprise Linux 8.4
Related component
Server, Client
Additional context
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: