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
Fix Windows build by defining `ssize_t` when missing and adjusting the
install commands.
Add support for ENABLE_WERROR=1 while at it.
Tested with MSVC 2013 on Windows 7 x64.
The COMPILE_LANGUAGE generator expression is only supported since CMake
3.3. Moreover, it does not work with all generators (works with Makefile
and Ninja, but not with Visual Studio).
target_compile_options would only work if a target does not mix C and
C++ sources, since the flags are intended to be set for a specific
language, use set_source_files_properties instead. This approach is also
less repetitive.
Drop the idea of using lists and COMPILE_OPTIONS,
set_source_files_properties only understands COMPILE_FLAGS (a single
string, not a list).
This option prevents the nghttp2 library from sending PING frame with
ACK flag set in the reply to incoming PING frame. To allow the
application to send PING with ACK flag set, nghttp2_submit_ping() now
recognizes NGHTTP2_FLAG_PING in its flags parameter.
libnghttp2.so was missing -fvisibility=hidden. libnghttp2_asio.so on the
other hand had hidden visibility which resulted in no exported symbols
and a broken asio client examples.
Just build a static nghttp2 library to solve this issue.
Split the nghttp2 library into objects and a shared library from those
objects. This is needed because of symbol visibility. An advantage over
the autotools build is that there are no worries about static versus
static library builds.
Test:
cmake $srcdir
make nghttpx-unittest main failmalloc
make test
Disallow request from server, and response from client respectively.
When the violation is detected, return NGHTTP2_ERR_PROTO from
nghttp2_submit_request, nghttp2_submit_response,
nghttp2_submit_headers.
We also did some refactoring, and now self-dependency detection is
placed where it is only required.
Previously, we use session->next_stream_id to detect that given stream
ID was idle or not. But this was suboptimal, since it was updated
when stream ID was assigned, and it did not necessarily mean that it
actually has been sent to the peer. Now we introduced
session->sent_stream_id, which only updated when HEADERS/PUSH_PROMISE
has sent. Using sent_stream_id instead of next_stream_id tightens
idle stream detection, and misbehaved peer which sends frame with
stream ID that has not been generated.
This commit also overhauls test code which involves opening streams.
Now we have some wrapper functions for nghttp2_session_open_stream()
which also take care of updating next_stream_id and
last_recv_stream_id. They are crucial for some tests.
Return NGHTTP2_ERR_INVALID_ARGUMENT from nghttp2_submit_headers() if
given stream ID and pri_spec->stream_id are the same (thus trying to
depend on itself).
Also return NGHTTP2_ERR_INVALID_ARGUMENT from nghttp2_submit_request()
and nghttp2_submit_headers() with stream_id == 1, when new stream ID
equals to pri_spec->stream_id.
Previously, these cases are not checked, and just sent to peer.
With the presence of idle stream related API (e.g.,
nghttp2_create_idle_stream()), it is more predictable for client to
create idle streams with its dependency to another idle stream.
Previously, we didn't create complete parent idle stream in this case.
Now we create idle streams as we do on server side.
Previously we scheduled the transmission of response HEADERS using
priority tree in the belief that it allows more better utilization of
bandwidth for prioritized streams. But to reduce the overhead of
reconstruction of priority queue when connection level flow control
window is depleted, we just don't check priority tree in this case.
This means that response HEADERS frames are not sent even though they
are not flow controlled. This could waste bandwidth. To improve this
situation, we stop scheduling response HEADERS with priority tree for
now. Now they are just sent in the order they submitted. The
response body DATA continued to be scheduled with priority tree as
before.
return session_inflate_handle_invalid_stream(...) case is for streams
for INITIAL state, but this is rare case. In general, we'd like to
reduce RST_STREAM transmission, and it is suffice to ignore this frame
for now.
Previously, we only updated stream's weight field when only weight was
changed by PRIORITY frame. If stream is queued, it would be better to
actually reschedule it based on new weight. This could be especially
useful if weight is increased.
Previously, we updated descendant_last_cycle in
nghttp2_stream_reschedule, which is called after non-zero DATA frame.
But this was not optimal since we still had old descendant_last_cycle,
and new stream was scheduled based on it. Now descendant_last_cycle
is updated in nghttp2_stream_next_outbound_item, which is called when
stream with highest priority is selected from queue. And new stream
is scheduled based on it. This commit also removes 0-reset of
descendant_last_cycle and cycle in nghttp2_stream_reschedule. This
could help making them lower, so that they are not overflow. But
there is a pattern that it doesn't work, and we are not sure they are
really useful at this moment.
Previously, stream object for pushed resource was not created during
nghttp2_submit_push_promise(). It was created just before
nghttp2_before_frame_send_callback was called for that PUSH_PROMISE
frame. This means that application could not call
nghttp2_submit_response for the pushed resource before
nghttp2_before_frame_send_callback was called. This could be solved
by callback chaining, but for web server with back pressure from
backend stream, it is a bit unnecessarily hard to use.
This commit changes nghttp2_submit_push_promise() behaviour so that
stream object is created during that call. It makes application call
nghttp2_submit_response right after successful
nghttp2_submit_push_promise call.
Previously, nghttp2_session_end_request_headers_received assumes
stream is still writable (in other words, local endpoint has not sent
END_STREAM). But this assumption is false, because application can
send response in nghttp2_on_begin_frame_callback. Probably, this
assumption was made before the callback was introduced. This commit
addresses this issue. Since all
nghttp2_session_end_*_headers_received functions are identical, we
refactored them into one function.
Because of the nature of heap based priority queue, and our compare
function, streams with the same weight and same parent are handled in
the reverse order they pushed to the queue. To allow stream pushed
earlier to be served first, secondary key "seq" is introduced to break
a tie. "seq" is monotonically increased integer per parent stream,
and it is assigned to stream when it is pused to the queue, and gets
incremented.
Previously, nghttp2_session_find_stream(session, 0) returned NULL
despite the fact that documentation said that it should return root
stream. Now it is corrected, and it returns root stream as
documented.
The added API is nghttp2_session_change_stream_priority(). This
provides the same functionality to re-prioritize stream when PRIORITY
frame. is received, but we do it without PRIORITY frame. This could
be useful for server to change pushed stream's priority silently.
When stream is removed from tree, stream is either closed, or its
remote flow control window is depleted. In the latter case, we
schedule this stream as fast as possible if its remote window gets
positive, since it did not sent anything in its turn. To achieve
this, reset last_writelen to 0 when stream is removed from tree.
It has no usecase at the moment. It is most likely that applications
know the flags when it submitted extension frame, no need to modify it
later. Possibly feature bloat.
For clients, CANCEL is more appropriate for both incoming/outgoing
streams. For servers, CANCEL is appropriate for its pushed stream
(outgoing), but REFUSED_STREAM is more appropriate for incoming
stream.
To validate actual response body length against the value declared in
content-length response header field, we first check request method.
If request method is HEAD, respose body must be 0 regardless of the
value in content-length. nghttp2_session_upgrade() has no parameter
to indicate the request method is HEAD, so we failed to validate
response body if HEAD is used with HTTP Upgrade. New
nghttp2_session_upgrade2() accepts new parameter to indicate that
request method is HEAD or not to fix this issue. Although, this issue
affects client side only, we deprecate nghttp2_session_upgrade() in
favor of nghttp2_session_upgrade2() for both client and server side.
By default, we check the length of response body matches
content-length. For HEAD request, this is not necessarily true, so we
sniff request method, and if it is HEAD, make sure that response body
length is 0. But this does not work for HTTP Upgrade, since
nghttp2_session_upgrade() has no parameter to tell the request method
was HEAD. This commit disables this response body length validation
for the stream upgraded by HTTP Upgrade. We will add new version of
nghttp2_session_upgrade with the parameter to pass the request method
information so that we can handle this situation properly.
This function is useful for the client application to know that there
is a chance that request can be sent. If this function returns 0,
there is zero chance to make a request.
This commit also set error_code passed to
nghttp2_on_stream_close_callback to NGHTTP2_REFUSED_STREAM if request
is not sent.
The encoder is not required to send dynamic table size update if the
table size is not changed from the previous value after accepting new
maximum value.
This will improve performance since we can avoid indirect call of
internal functions. The downside is we now require libnghttp2 static
library to run unit tests.
Application can utilize this framework to send/receive user defined
extension frames. These frames are expected not to change existing
protocol behaviour.
If application returns NGHTTP2_ERR_PAUSE from send_data_callback, it
means application processed all data, but wants to make
nghttp2_session_mem_send or nghttp2_session_send return immediately.
This is useful if application writes to fixed sized buffers, and there
is no room to write more data.
This change adds new return error code from nghttp2_session_mem_recv
and nghttp2_session_recv functions, namely NGHTTP2_ERR_FLOODED. It is
fatal error, and is returned when flooding was detected.
If it is called through libnghttp2 internally, name/value pairs are
all NULL-terminated. But it is one of public API, and we cannot
expect that applications always make NULL-terminated string for
name/value pairs.
RFC 7540 does not enforce any limit on the number of incoming reserved
streams (in RFC 7540 terms, streams in reserved (remote) state). This
only affects client side, since only server can push streams.
Malicious server can push arbitrary number of streams, and make
client's memory exhausted. The new option,
nghttp2_set_max_reserved_remote_streams, can set the maximum number of
such incoming streams to avoid possible memory exhaustion. If this
option is set, and pushed streams are automatically closed on
reception, without calling user provided callback, if they exceed the
given limit. The default value is 200. If session is configured as
server side, this option has no effect. Server can control the number
of streams to push.
The intention of this stream API is give server application about
stream dependency information, so that it can utilize it for better
scheduling of stream processing. We have no plan to add object
oriented API based on stream object.
Tatsuhiro Tsujikawa
Wenfeng Liu
Alexis La Goutte
Mike Conlen
Peter Wu
Jan-E
Reza Tavakoli
kumagi