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

H3Settings is a bit underspecified #382

Open
LPardue opened this issue Jan 25, 2024 · 4 comments · May be fixed by #383
Open

H3Settings is a bit underspecified #382

LPardue opened this issue Jan 25, 2024 · 4 comments · May be fixed by #383

Comments

@LPardue
Copy link
Member

LPardue commented Jan 25, 2024

https://quicwg.org/qlog/draft-ietf-quic-qlog-h3-events.html#section-5.3.4

H3Setting = {
    name: text
    value: uint64
}

Looking at IANA https://www.iana.org/assignments/http3-parameters/http3-parameters.xhtml, I think qlog nudges implementations to log the setting name. When an implementation receives an unknown setting (not a greased one) it won't know the name. It could log the setting type value code in the name field, but that is not specified. This makes things annoying for parsers since they need to do some thinking.

One suggestion is to add another field like type that could contain the numerical type value. It would be duplicative to have both, so that might require making type and name optional.

@LPardue
Copy link
Member Author

LPardue commented Jan 25, 2024

Also we might want to state that the name should be the name IANA use, possiblyeven define a explicit names in addition to free form string.

LPardue added a commit that referenced this issue Jan 28, 2024
@LPardue LPardue linked a pull request Jan 28, 2024 that will close this issue
LPardue added a commit that referenced this issue Jan 28, 2024
@rmarx
Copy link
Contributor

rmarx commented Feb 5, 2024

Discussed during meeting.
Agree to add this option, similar to how we changed ALPN in #385

@rmarx
Copy link
Contributor

rmarx commented Feb 5, 2024

Another thing I'm wondering is if we should clarify what the name field is exactly.

i.e., I've previously logged this myself as QPACK_MAX_TABLE_CAPACITY but the IANA name is prefixed with SETTINGS_, so SETTINGS_QPACK_MAX_TABLE_CAPACITY instead.

You could argue this should be obvious, but e.g., in RFC9204, they use the unprefixed version (see https://www.rfc-editor.org/rfc/rfc9204.html#section-8.1) with a note For formatting reasons, the setting names here are abbreviated by removing the 'SETTINGS_' prefix.... could be quite confusing for novice implementers, so maybe 1 example or clear statement that the name is the IANA registered name would be good.

@LPardue
Copy link
Member Author

LPardue commented Feb 5, 2024

Agree 100%, I'll add some text

LPardue added a commit that referenced this issue Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants