-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
HTTP/3 QPACK - do not expect section ack for zero required insert count #7802
Conversation
Signed-off-by: Lachlan Roberts <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test case?
Signed-off-by: Lachlan Roberts <[email protected]>
{ | ||
// Encode a header with only a value contained in the static table. | ||
ByteBuffer buffer = encode(_encoder, 0, toMetaData("GET", "/", "http")); | ||
System.err.println(BufferUtil.toDetailString(buffer)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the System.err.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
// Parsing a section ack instruction on the encoder when we are not expecting it should result in QPACK_DECODER_STREAM_ERROR. | ||
SectionAcknowledgmentInstruction instruction = new SectionAcknowledgmentInstruction(0); | ||
ByteBuffer instructionBuffer = toBuffer(instruction); | ||
QpackException error = assertThrows(QpackException.class, () -> _encoder.parseInstructions(instructionBuffer)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want to test for QpackException.StreamException
here to be sure it's not a SESSION error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a SessionException
and thats what I would expect it to be. The rfc says it must be treated "as a connection error".
I have added this check to the test case.
Signed-off-by: Lachlan Roberts <[email protected]>
https://datatracker.ietf.org/doc/html/draft-ietf-quic-qpack-21#section-4.4.1