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

header-table-size option breaks something #396

Closed
maskit opened this issue Oct 19, 2015 · 3 comments
Closed

header-table-size option breaks something #396

maskit opened this issue Oct 19, 2015 · 3 comments

Comments

@maskit
Copy link

maskit commented Oct 19, 2015

nghttp sends COMPRESSION_ERROR when I set header-table-size to 4096 and send a request to Google, but 4095 and 4097 are OK.

$ nghttp -v -H ":method: HEAD" -c 4095  https://www.google.com/ | tail -n2
[  0.074] send GOAWAY frame <length=8, flags=0x00, stream_id=0>
          (last_stream_id=0, error_code=NO_ERROR(0x00), opaque_data(0)=[])
$ nghttp -v -H ":method: HEAD" -c 4096  https://www.google.com/ | tail -n2
Some requests were not processed. total=1, processed=0
[  0.077] send GOAWAY frame <length=8, flags=0x00, stream_id=0>
          (last_stream_id=0, error_code=COMPRESSION_ERROR(0x09), opaque_data(0)=[])
$ nghttp -v -H ":method: HEAD" -c 4097  https://www.google.com/ | tail -n2
[  0.067] send GOAWAY frame <length=8, flags=0x00, stream_id=0>
          (last_stream_id=0, error_code=NO_ERROR(0x00), opaque_data(0)=[])

Sorry if G's implementation is wrong.

@tatsuhiro-t
Copy link
Member

It seems G's implementation does not send dynamic table size update if dynamic table size of their server does not change.
This explains why -c4096 makes error and other values are ok.

I think we should send dynamic table size update whether or not dynamic table size is actually changed. From specification, this is a bit grey area.

In https://tools.ietf.org/html/rfc7540#section-6.5.2

  SETTINGS_HEADER_TABLE_SIZE (0x1):  Allows the sender to inform the
      remote endpoint of the maximum size of the header compression
      table used to decode header blocks, in octets.  The encoder can
      select any size equal to or less than this value by using
      signaling specific to the header compression format inside a
      header block (see [COMPRESSION]).  The initial value is 4,096
      octets.

In https://tools.ietf.org/html/rfc7541#section-4.2

   A change in the maximum size of the dynamic table is signaled via a
   dynamic table size update (see Section 6.3).  This dynamic table size
   update MUST occur at the beginning of the first header block
   following the change to the dynamic table size.  In HTTP/2, this
   follows a settings acknowledgment (see Section 6.5.3 of [HTTP2]).

In https://tools.ietf.org/html/rfc7541#section-6.3

   The new maximum size MUST be lower than or equal to the limit
   determined by the protocol using HPACK.  A value that exceeds this
   limit MUST be treated as a decoding error.  In HTTP/2, this limit is
   the last value of the SETTINGS_HEADER_TABLE_SIZE parameter (see
   Section 6.5.2 of [HTTP2]) received from the decoder and acknowledged
   by the encoder (see Section 6.5.3 of [HTTP2]).

This is a good subject to ask httpbis working group.

@tatsuhiro-t
Copy link
Member

It seems the correct behaviour is that receiver of SETTINGS_HEADER_TABLE_SIZE is required to send dynamic table size update only if it really changed maximum header table size. So the google server is right here. We fixed this in 62a8132

@maskit
Copy link
Author

maskit commented Oct 25, 2015

v1.4.0 works fine. Thanks.

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

No branches or pull requests

2 participants