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

Move setting registrations into QPACK #1358

Merged
merged 2 commits into from May 25, 2018
Merged

Move setting registrations into QPACK #1358

merged 2 commits into from May 25, 2018

Conversation

MikeBishop
Copy link
Contributor

Fixes #1323. Trying to describe how the max table size was used made me realize we never actually say, which is why I also wrote #1357; there's a reference that needs to be updated in whichever PR lands second.

@MikeBishop MikeBishop added editorial An issue that does not affect the design of the protocol; does not require consensus. -http -qpack labels May 18, 2018
SETTINGS_HEADER_TABLE_SIZE (0x1):
: An integer with a maximum value of 2^30 - 1. The default value is 4,096
bytes.
The following setting is defined in HTTP/QUIC:

SETTINGS_MAX_HEADER_LIST_SIZE (0x6):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does anything in this document explain what SETTINGS_MAX_HEADER_LIST_SIZE does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. RFC7540 doesn't have much, but it has more than HQ does currently.


SETTINGS_MAX_HEADER_LIST_SIZE (0x6):
: An integer with a maximum value of 2^30 - 1. The default value is
unlimited.

SETTINGS_QPACK_BLOCKED_STREAMS (0x7):
: An integer with a maximum value of 2^16 - 1. The default value is 100.
Additional settings MAY be defined by extensions to HTTP/QUIC.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we calling QPACK an extension to HTTP/QUIC? Or it is for purposes of SETTINGS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kind of. It's required to parse HEADERS and PUSH_PROMISE, but it is its own thing. Kind of a mandatory extension, I suppose?

@@ -1418,13 +1413,11 @@ The entries in the following table are registered by this document.
|----------------------------|------|-------------------------|
| Setting Name | Code | Specification |
|----------------------------|:----:|-------------------------|
| HEADER_TABLE_SIZE | 0x1 | {{settings-parameters}} |
Copy link
Contributor

Choose a reason for hiding this comment

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

It strikes me as bizarre that 0x1 is omitted from this table / or that QPACK - as an extension or subordinate document - gets to define the lowest numbered setting value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. That's because we're trying to keep consistency with the setting codes in HTTP/2, where this is defined in the HTTP draft and the value passed to HPACK as a parameter.

@afrind afrind merged commit 3946efc into master May 25, 2018
@MikeBishop MikeBishop deleted the qpack_independent branch June 14, 2018 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http -qpack editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants