Closed BlobbyBob closed 2 months ago
Would be better if we can add a test to test_parser.py reproducing the problem. Also, does it work without C parser (AIOHTTP_NO_EXTENSIONS=1
)?
With AIOHTTP_NO_EXTENSIONS=1
there seems to be no issue
Then it likely needs a bug report at https://github.com/nodejs/llhttp/ with the actual message that trips the parser (they will need to reproduce it in isolation, without aiohttp or tlsanvil).
I've narrowed the issue a bit down. It has to do with the Upgrade: h2c
header in the request. A more minimal reproduction example:
Server
import aiohttp.web
async def printer(request):
print(request.path, await request.read())
return aiohttp.web.json_response({})
app = aiohttp.web.Application()
app.router.add_post("/", printer)
aiohttp.web.run_app(app, port=8000)
Client
import socket
import time
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 0)
sock.connect(("localhost", 8000))
sock.send(b"POST / HTTP/1.1\r\n"
b"Connection: Upgrade\r\n"
b"Content-Length: 2\r\n"
b"Upgrade: h2c\r\n"
b"Content-Type: application/json\r\n\r\n"
b"{}")
print(sock.recv(1024))
sock.close()
If you still think it is an issue with llhttp
, could you open an issue over there? I'm not familiar with that project, so the bug report would probably be not too precise and helpful.
We are faced with the same problem: http-body gets mangled. aiohttp 3.9.3 works.
I see the same result in 3.8. Test reproducer is:
def test_http_request_parser_upgrade(parser: Any) -> None:
text = b"POST / HTTP/1.1\r\nConnection: Upgrade\r\nContent-Length: 2\r\nUpgrade: h2c\r\nContent-Type: application/json\r\n\r\n"
msg = parser.feed_data(text)[0][0][0]
assert parser.feed_data(b"{}") == ([], False, b"")
assert msg.method == "POST"
assert msg.path == "/"
assert msg.url.path == "/"
assert msg.version == (1, 1)
assert not msg.should_close
assert msg.compression is None
assert msg.upgrade
assert not msg.chunked
It only happens when the message is split over 2 feed_data() calls though.
@BlobbyBob Would you be able to try out #8597? You'll need to follow the instructions to build the C parser: https://github.com/aio-libs/aiohttp/blob/fix-fail-upgrade/vendor/README.rst
The issue seems to be caused by the condition we used to decide when the request has been upgraded.
Describe the bug
When sending two TCP segments, the first containing all headers including the final
\r\n\r\n
and the second containing a JSON body, the server parses them as two request. The first has an empty body while the second fails withaiohttp.http_exceptions.BadStatusLine
as the body is interpreted as HTTP methodTo Reproduce
Use the following as client:
Use the following as server:
Expected behavior
HTTP Requests are decoded correctly
Logs/tracebacks
aiohttp Version
multidict Version
yarl Version
OS
Reproducible on both Arch Linux and
openjdk:11
docker image (debian based)Related component
Server
Additional context
Packet capture: test.pcap.gz
Code of Conduct