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

token-related fixes #123

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 4 additions & 8 deletions draft-marx-qlog-event-definitions-quic-h3.md
Expand Up @@ -670,7 +670,7 @@ Data:
original_destination_connection_id?:bytes,
initial_source_connection_id?:bytes,
retry_source_connection_id?:bytes,
stateless_reset_token?:Token,
stateless_reset_token?:bytes,
disable_active_migration?:boolean,

max_idle_timeout?:uint64,
Expand All @@ -697,7 +697,7 @@ interface PreferredAddress {
port_v6:uint16,

connection_id:bytes,
stateless_reset_token:Token
stateless_reset_token:bytes
}
~~~

Expand Down Expand Up @@ -750,8 +750,6 @@ Data:

is_coalesced?:boolean, // default value is false

retry_token?:Token, // only if header.packet_type === retry

stateless_reset_token?:bytes, // only if header.packet_type === stateless_reset. Is always 128 bits in length.

supported_versions:Array<bytes>, // only if header.packet_type === version_negotiation
Expand Down Expand Up @@ -789,8 +787,6 @@ Data:

is_coalesced?:boolean,

retry_token?:Token, // only if header.packet_type === retry

stateless_reset_token?:bytes, // only if header.packet_type === stateless_reset. Is always 128 bits in length.

supported_versions:Array<bytes>, // only if header.packet_type === version_negotiation
Expand Down Expand Up @@ -1817,7 +1813,7 @@ class PacketHeader {

flags?: uint8; // the bit flags of the packet headers (spin bit, key update bit, etc. up to and including the packet number length bits if present) interpreted as a single 8-bit integer

token?:Token; // only if packet_type == initial
token?:Token; // only if packet_type == initial || retry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a bit of a difficult one imo.
I'd put retry_token outside, since it's encapsulated in its own packet type, and so it can be considered to be part of the "payload" rather than the header.
This is different from the initial token, which is clearly part of the header.

If we do put the retry token in the PacketHeader struct, imo it would make sense to do the same for the stateless_reset_token as well, since from the same logic, you could say that that is part of the Stateless Reset Packet header as well, rater than its payload.

I think it's probably cleaner to have all three in the PacketHeader then, with token?:Token covering both initial and retry as you proposed here.


length?: uint16, // only if packet_type == initial || handshake || 0RTT. Signifies length of the packet_number plus the payload.

Expand Down Expand Up @@ -2086,7 +2082,7 @@ class NewConnectionIDFrame{
connection_id_length?:uint8;
connection_id:bytes;

stateless_reset_token?:Token;
stateless_reset_token?:bytes;
}
~~~

Expand Down