quicwg / qlog

The IETF I-D documents for the qlog format
Other
86 stars 13 forks source link

Improve quic:alpn_information #368

Closed rmarx closed 9 months ago

rmarx commented 10 months ago

As reported by @hlandau on the mailing list:

quic:alpn_information:

RFC 7301 states that ALPN protocol strings are byte strings, not text. How should ALPN protocol strings which are not valid UTF-8 or not valid text be logged?

LPardue commented 10 months ago

This issue is correct. Implementations that treat ALPN IDs as if they were strings can hit issues. Although qlog is not in the hot path, assuming sequence is safe to encode is not robust (especially as it is uncontrolled data).

My inclination would be to define a ALPN ID type for qlog. That could carry the raw bytes encoded as hexstring and/or the "friendly" representation if its safe. e.g.

ALPNIdentifier = {
  ? bytes: hexstring
  ? string: string
}

The reason being that most people would recognise h3 on sight, but scratch their heads about what 0x68 0x33 is.

Furthermore, while looking at the section, I think we should actually cite the ALPN RFC and tighten up the text at the same time.

hlandau commented 10 months ago

Personally I would just go with the hex encoding. Providing the ability to use either seems overkill. Not being able to eyeball a qlog file to read the ALPN might be annoying, but that will be the case anyway if an implementation does the robust thing and only serialises the hex encoding.

marten-seemann commented 10 months ago

Not being able to quickly read the "h3" ALPN from a qlog seems like a non-starter to me. I don't want to remember (or rather, look up 1000 times) what that is in hex.

LPardue commented 10 months ago

Agree with Marten here.

HTTP headers might actually suffer the same issues now I think about it.