Previously, we did not handle PRIORITY frame which depends on itself
and for idle stream. As a result, nghttp2_session_mem_recv (or
nghttp2_session_recv) returne NGHTTP2_ERR_NOMEM. The error code was
still misleading. It was not out of memory, and we failed to insert
hash map because of duplicated key, which was treated as out of
memory. This commit fixes this issue, by explicitly checking
dependency for incoming PRIORITY for all cases.
When we know that stream is closed at time we read DATA frame header,
we use NGHTTP2_IB_IGN_DATA, and consume data for connection if
nghttp2_option_set_no_auto_window_update() is used. However, if
stream is closed while we are in NGHTTP2_IB_READ_DATA, those bytes are
not consumed for connection, nor notified to application via callback,
so it eventually fills up connection window and connection will
freeze. This commit fixes this issue by consuming these data for
connection when stream is closed or does not exist.
After reviewing codebase, only queue for DATA frames requires
priorities. Other frames can be replaced multiple linear queues.
Replacing priority queue with linear queue allows us to simplify
codebase a bit; for example, now nghttp2_session.next_seq is gone.
Since application most likely allocates the stream object in
nghttp2_on_begin_headers_callback, it is desirable to handle its
failure as stream error. But previously it only signals success or
fatal error. Submitting RST_STREAM does not prevent
nghttp2_on_header_callback from being invoked. This commit improves
this situation by allowing NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE from
nghttp2_on_begin_headers_callback. If that value is returned, library
submits RST_STREAM with error code INTERNAL_ERROR, and
nghttp2_on_header_callback and nghttp2_on_frame_recv_callback for that
frame are not invoked. Note that for PUSH_PROMISE frame, the stream
to be reset is promised stream.
Previously nghttp2_session_send() and nghttp2_session_mem_send() did
not send 24 bytes client magic byte string (MAGIC). We made
nghttp2_session_recv() and nghttp2_session_mem_recv() process MAGIC by
default, so it is natural to make library send MAGIC as well. This
commit makes nghttp2_session_send() and nghttp2_session_mem_send()
send MAGIC. This commit also replace "connection preface" with
"client magic", since we call MAGIC as "connection preface" but it is
just a part of connection preface. NGHTTP2_CLIENT_CONNECTION_PREFACE
macro was replaced with NGHTTP2_CLIENT_MAGIC. The already deprecated
NGHTTP2_CLIENT_CONNECTION_HEADER macro was removed permanently.
nghttp2_option_set_no_recv_client_preface() was renamed as
nghttp2_option_set_no_recv_client_magic(). NGHTTP2_ERR_BAD_PREFACE
was renamed as NGHTTP2_ERR_BAD_CLIENT_MAGIC.
Since HTTP/2 spec requires for client to send connection preface, it
is reasonable to make this option enabled by default. It is still a
use case to disable this, so replace this option with
nghttp2_option_set_no_recv_client_preface().
To avoid buffer copy in nghttp2_data_source_read_callback, this commit
introduces NGHTTP2_DATA_FLAG_NO_COPY and nghttp2_send_data_callback.
By using NGHTTP2_DATA_FLAG_NO_COPY in
nghttp2_data_source_read_callback, application can avoid to copy
application data to given buffer. Instead, application has to
implement nghttp2_send_data_callback to send complete DATA frame by
itself. We see noticeable performance increase in nghttpd and
tiny-nghttpd using this new feature. On the other hand, nghttpx does
not show such difference, probably because buffer copy is not
bottleneck. Using nghttp2_send_data_callback adds complexity, so it
is recommended to measure the performance to see whether this extra
complexity worth it.
For "http" or "https" URIs, :path header field must start with "/".
The only exception is OPTIONS method, which can contain "*" to
represent system-wide OPTIONS request.
nghttp2_submit_request and nghttp2_submit_response will set
NGHTTP2_FLAG_END_STREAM after all given data is sent (data could be
0). This means we have no way to send trailers. In this commit, we
added NGHTTP2_DATA_FLAG_NO_END_STREAM flag. The application can set
this flag in *data_flags inside nghttp2_data_source_read_callback. If
NGHTTP2_DATA_FLAG_EOF is set, library automatically set
NGHTTP2_FLAG_END_STREAM. But if both NGHTTP2_DATA_FLAG_EOF and
NGHTTP2_DATA_FLAG_NO_END_STREAM are set, NGHTTP2_FLAG_END_STREAM will
not set by library. Then application can use new
nghttp2_submit_trailer() to send trailers. nghttp2_submit_trailer()
will set NGHTTP2_FLAG_END_STREAM and it is actually thing wrapper of
nghttp2_submit_headers().
After we sent SETTINGS including ENABLE_PUSH = 0, peer may already
issue PUSH_PROMISE before receiving our SETTINGS and react it to
SETTINGS ACK. Previously we accept this PUSH_PROMISE. In this
commit, we check the pending ENABLE_PUSH value and if it means
disabling push, we refuse PUSH_PROMISE with RST_STREAM of error
REFUSED_STREAM.
This commit only affects the library behaviour unless
nghttp2_option_set_no_http_messaging() is used.
We like strict validation against header field name and value against
RFC 7230, but we have already so much web sites and libraries in
public internet which do not obey these rules. Simply just
terminating stream because of this may break web sites and it is too
disruptive. So we decided that we should be conservative here so
those header fields containing illegal characters are just ignored.
But we are conservative only for regular headers. We are strict for
pseudo headers since it is new to HTTP/2 and new implementations
should know the rules better.
Previously we did not check HTTP semantics and it is left out for
application. Although checking is relatively easy, but they are
scattered and error prone. We have implemented these checks in our
applications and also feel they are tedious. To make application
development a bit easier, this commit adds basic HTTP semantics
validation to library code. We do following checks:
server:
* HEADERS is either request header or trailer header. Other type of
header is disallowed.
client:
* HEADERS is either zero or more non-final response header or final
response header or trailer header. Other type of header is
disallowed.
For both:
* Check mandatory pseudo header fields.
* Make sure that content-length matches the amount of DATA we
received.
If validation fails, RST_STREAM of type PROTOCOL_ERROR is issued.
Previously we did not handle the situation where RST_STREAM is
submitted against a stream while requet HEADERS which opens that
stream is still in queue. Due to max concurrent streams limit,
RST_STREAM is sent first, and then request HEADERS, which effectively
voids RST_STREAM.
In this commit, we checks RST_STREAM against currently pending request
HEADERS in queue and if stream ID matches, we mark that HEADERS as
canceled and RST_STREAM is not sent in this case. The library will
call on_frame_not_sent_callback for the canceled HEADERS with error
code from RST_STREAM.
Previously we use 16K - 9 bytes (frame header) as frame payload size
so that whole frame fits in 1 TLS record size (16K). But it turns out
that in proxy use case, we will receive 16K payload from backend and
we have to split it into 2 odd looking frames (16K - 9 and 9), and
latter is highly inefficient. To avoid this situation, we decided to
use min frame payload size to 16K. Since we operates on TLS as stream
of data, we are not so much restricted in its record size.
Previously we treat stream in NGHTTP2_STREAM_RESERVED state specially,
that is we don't increment or decrement streams counts if stream is in
that state. Because of this, we don't change the stream state to
NGHTTP2_STREAM_CLOSING if stream is in NGHTTP2_STREAM_RESERVED. But
it turns out that it causes a problem. If client canceled pushed
stream before push response HEADERS, stream is still in
NGHTTP2_STREAM_RESERVED state. If push response HEADERS arrived in
this state, library happily accepts it and passed to application.
With this commit, this bug was corrected. We now change stream state
to NGHTTP2_STREAM_CLOSING even if it was in NGHTTP2_STREAM_RESERVED
state. We now use NGHTTP2_STREAM_FLAG_PUSH to determine whether we
have to increase/decrase stream count.
nghttp2_submit_shutdown_notice() is used to notify the client that
graceful shutdown is started. We expect that after this call, the
server application should send another GOAWAY using
nghttp2_submit_goaway() with appropriate last_stream_id. In this
commit, we also added nghttp2_session_get_last_proc_stream_id(), which
can be used as last_stream_id parameter.
This commit implements graceful shutdown in nghttpx. The integration
test for graceful shutdown is also added.
Initially, we use nghttp2_stream.data_item to refer only item with
DATA frame. But recently we use it to refer HEADERS frame as well.
So it is better to call just item rather than data_item. This applies
to all related functions.
Previously we handle idle streams as closed streams. We only keeps
sum of closed streams and active streams under max concurrent streams
limit, idle streams gets deleted earlier than client expects.
In this change, idle streams are kept in separate list and not handled
as closed streams. To mitigate possible attack vector to make
unlimited idle streams, we cap the number of idle streams in a half of
max concurrent streams. This is arbitrary choice. It may be adjusted
in the future when we have interop experience.
nghttp2_mem structure is introduced to hold custom memory allocator
functions and user supplied pointer. nghttp2_mem object can be passed
to nghttp2_session_client_new3(), nghttp2_session_server_new3(),
nghttp2_hd_deflate_new2() and nghttp2_hd_inflate_new2() to replace
standard malloc(), free(), calloc() and realloc(). nghttp2_mem
structure has user supplied pointer mem_user_data which can be used as
per session/object memory pool.
This change will utilize last_stream_id in GOAWAY extensively. When
GOAWAY is received with a last_stream_id, library closes all outgoing
streams whose stream_id > received last_stream_id.
nghttp2_on_stream_callback is called for each stream to be closed.
When GOAWAY is sent with a last_stream_id, library closes all incoming
streams whose stream_id > sent last_stream_id.
nghttp2_on_stream_callback is called for each stream to be closed.
If stream ID is not idle, it might be valid HEADERS. If stream ID is
idle, it is invalid regardless stream ID is even or odd, since client
is not expected to recieve request from server. nghttp2 library
historically allows this, but now we forbids this.
Tatsuhiro Tsujikawa
Alexis La Goutte