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

potential bug with N-bit set to 1 #51

Closed
marten-seemann opened this issue Sep 3, 2024 · 3 comments · Fixed by #52
Closed

potential bug with N-bit set to 1 #51

marten-seemann opened this issue Sep 3, 2024 · 3 comments · Fixed by #52
Labels
bug Something isn't working

Comments

@marten-seemann
Copy link
Member

          @marten-seemann It turns out, the issue was actually due to the setting of `N` bit at [Literal Field Line with Name Reference](https://www.rfc-editor.org/rfc/rfc9204.html#name-literal-field-line-with-nam)

When I encode that packet I set the N bit to 1, to ensure that literal string representation is sent across the subsequent hops by the intermediary.

RFC Reference

The condition buf[0]&0x20 > 0 here is triggering the errNoDynamicTable error. But in this case the N bit does not have anything to do with Dynamic table unless you have a reason.

I flipped the 'N' bit to 0 and that solved the issue. PR Reference for the QPACK that I use: PR

Originally posted by @DineshAdhi in #33 (comment)

@kixcord
Copy link

kixcord commented Sep 3, 2024

Yeah, I was hard-coding N=1 because I didn't care if a later intermediary converted the entry from literal to dynamic. I switched it to N=0 to unbreak quic-go but the correct fix is to remove that linked check.

Note that web-transport-quinn doesn't support a dynamic table either.

@marten-seemann
Copy link
Member Author

@kixcord I'm wondering if there'd be value in submitting QIF files to https://github.com/qpackers/qifs, so all implementations could run their interop test against an encoding that uses N=1. What do you think?

@kixcord
Copy link

kixcord commented Sep 4, 2024

https://github.com/qpackers/qifs

I didn't know that existed. Up to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants