There is a discussion in httpbis mailing list which argues that
dependency tree is for client, and changing it in server side is not
what client expects.
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JulSep/0416.html
Currently, we make pushed stream depend on the parent stream of
associated stream (that is main HTML in most of the cases), so that
associated stream and pushed stream become siblings. In this case, we
also observed that these resources complete each other to get its
parent weight. This means that the delivery of associated stream is
delayed by pushed streams.
So at this moment, it is not a good idea to change pushed stream
priority in a way we do currently.
Previously, in server side, we used closed streams to detect the error
that the misbehaving client sends a frame on the incoming stream it
explicitly closed. With this commit, we make a further step, and
detect one more error case. Since we retain closed streams as long as
the sum of its size and the number of opened streams are equal or less
than max concurrent streams, we can safely say that if we get a frame
which is sent on the stream that is not found in either closed or
opened stream, it is already closed or has not existed. Then we can
send GOAWAY.
The previous code shrinks closed streams when we closed another
stream, but now it is removed. It is enough to adjust closed streams
when new incoming stream is created.
While creating this commit, we noticed that
NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS is defined as INT32_MAX. But
since SETTINGS can contain value up to UINT32_MAX, it is not enough.
However, since the stream ID space is limited to INT32_MAX, it is high
enough. We could keep this value, but this time we deprecate
NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS macro. While it is in public
header, the effect of deprecating it is negligible because of the
reason we wrote above, and usually application sets much smaller value
(say, 100) as SETTINGS_MAX_CONCURRENT_STREAMS.
https://tools.ietf.org/html/rfc7540#section-5.3.3 explains how to
transform dependency tree to avoid circular dependency. Previously,
we wrongly always moved the dependent stream under the root stream.
The correct destination is the parent stream of the stream to
reprioritize. This commit fixes this bug.
nghttp2_on_invalid_header_callback is similar to
nghttp2_on_header_callback, but the former is only called when the
invalid header field is received which is silently ignored when the
callback is not set. With this callback, application inspects the
incoming invalid field, and it also can reset stream from this
callback by returning NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE, or using
nghttp2_submit_rst_stream() directly with the error code of choice.
We also added nghttp2_on_invalid_header_callback2, which uses
reference counted header fields.
We have a code to call error callback when invalid header is received
and it is treated as stream error. But we didn't if the incoming
header is invalid, but just ignored. This generosity is required to
handle public Internet connections especially when nghttp2 is used as
forward proxy.
Tatsuhiro Tsujikawa
Wenfeng Liu
Wenfeng Liu