search

private-octopus / picoquic

Minimal implementation of the QUIC protocol
MIT License
540 stars 159 forks source link

picoquicdemo no longer persists output to disk after 0a2efc52c4c376949bc2bd35c6d9a9732a37fe5b #1463

Closed moderation closed 1 year ago

moderation commented 1 year ago

The commit https://github.com/private-octopus/picoquic/commit/0a2efc52c4c376949bc2bd35c6d9a9732a37fe5b breaks the operation of picoquicdemo.

Before this commit the following requests google.com and persists the output to _: ./picoquicdemo www.google.com 443 /

After the commit, the request succeeds but the output is never persisted to disk. This issue remains on the master branch today.

huitema commented 1 year ago

Yes, there is a problem. The request to google.com does not in fact succeed. To capture a qlog, I ran:

.\picoquicdemo.exe -q qlog www.google.com 443 /

The log shows an error message from google.com:

[114559, "transport", "packet_received", { "packet_type": "handshake", "header": { "packet_size": 98, "packet_number": 6, "payload_length": 56, "scid": "f6cd26a86b32e1b9", "dcid": "ff6f42a93e064672" }, "frames": [{ 
    "frame_type": "connection_close", "error_space": "application", "error_code": 261, "reason": "134:Invalid frame type 0 received on control stream."}]}],
[114613, "info", "message", { "message": "Protocol error 0xa"}]

This message shows there is something wrong with the request. Analyzing further.

huitema commented 1 year ago

The PR #1462 introduced support for web transport, which requires announcing support for Web Transport in the "settings" parameters of the HTTP3 connection. There was an error in the encoding of these HTTP3 settings, which caused google.com to fail the connection.

huitema commented 1 year ago

@notification, Thanks for reporting the issue. The PR #1464 provides an immediate fix by fixing the encoding of the HTTP3 settings. I entered issue #1465 because the longer term fix requires parsing the HTTP3 settings in Picoquic/H3Zero, but this will take a bit of time.

moderation commented 1 year ago

Awesome @huitema. Thanks for the quick turn around. I can confirm the fix is working