Revert part of 16c46114dc to fix race
condition that incoming stream after sending GOAWAY causes connection
error. The strict stream handling introduced in the above commit does
not handle several cases well (e.g., GOAWAY race, and refusing streams
because of concurrency limit).
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.
This function sets the maximum length of header block (a set of header
fields per HEADERS frame) to send. The length of given set of header
fields is calculated using nghttp2_hd_deflate_bound(). Previously,
this is hard-coded, and is 64KiB.
after a call to nghttp2_session_mem_send_internal() which should
set it, however in nghttp2_session_mem_send_internal() it is
possible to return before setting the pointer.
This change initializes the variable to NULL where delcared and
sets the variable in nghttp2_session_mem_send_internal() to
NULL before possibly returning rather than after.
both options are not necessary but are both ideal practice
Tatsuhiro Tsujikawa
Wenfeng Liu
Alexis La Goutte
Mike Conlen