Broken HTTP request parsing: Upgrade: h2c header leads to discarded body about aiohttp HOT 7 OPEN

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)?

from aiohttp.

With AIOHTTP_NO_EXTENSIONS=1 there seems to be no issue

from aiohttp.

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).

from aiohttp.

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()

from aiohttp.

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.

from aiohttp.

We are faced with the same problem: http-body gets mangled.
aiohttp 3.9.3 works.

from aiohttp.

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.

from aiohttp.