Author: rhuijben
Date: Mon Nov 2 14:31:58 2015
New Revision: 1712013
URL: http://svn.apache.org/viewvc?rev=1712013&view=rev
Log:
In the http2 protocol handler: properly handle pushed requests, by setting
up all the stream state and then cleanly denying them.
* protocols/http2_protocol.c
(http2_handle_promise): Add implementation. Setup future stream and delegate
remaining handling to parent stream.
* protocols/http2_protocol.h
(serf_http2_stream_t): Add new variable.
* protocols/http2_stream.c
(serf_http2__stream_create): Tweak init code.
(stream_promise_item,
stream_promise_done): New functions.
(serf_http2__stream_handle_hpack): Add specific implementation for
promised streams.
Modified:
serf/trunk/protocols/http2_protocol.c
serf/trunk/protocols/http2_protocol.h
serf/trunk/protocols/http2_stream.c
Modified: serf/trunk/protocols/http2_protocol.c
URL:
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_protocol.c?rev=1712013&r1=1712012&r2=1712013&view=diff
==============================================================================
--- serf/trunk/protocols/http2_protocol.c (original)
+++ serf/trunk/protocols/http2_protocol.c Mon Nov 2 14:31:58 2015
@@ -392,13 +392,62 @@ http2_handle_promise(void *baton,
const char *data,
apr_size_t len)
{
- serf_http2_stream_t *stream = baton;
+ serf_http2_stream_t *parent_stream = baton;
+ serf_http2_protocol_t *h2= parent_stream->h2;
+ serf_http2_stream_t *promised_stream;
+ apr_int32_t streamid;
+ const struct promise_t
+ {
+ unsigned char s3, s2, s1, s0;
+ } *promise;
if (len != HTTP2_PROMISE_DATA_SIZE)
return SERF_ERROR_HTTP2_FRAME_SIZE_ERROR;
- /* ### TODO: Prepare reading promise on stream */
- SERF_H2_assert(stream->h2 != NULL);
+ SERF_H2_assert(h2 != NULL);
+
+ promise = (const void *)data;
+
+ /* Highest bit is reserved */
+ streamid = ((promise->s3 & 0x7F) << 24) | (promise->s2 << 16)
+ |(promise->s1 << 8) | promise->s0;
+
+ if (streamid == 0
+ || (streamid < h2->rl_next_streamid)
+ || (streamid & 0x01) != (h2->rl_next_streamid & 0x01))
+ {
+ /* The promised stream identifier MUST bet a valid choice for the
+ next stream sent by the sender */
+
+ /* A receiver MUST treat the receipt of a PUSH_PROMISE that promises an
+ illegal stream identifier (Section 5.1.1) as a connection error
+ (Section 5.4.1) of type PROTOCOL_ERROR. Note that an illegal stream
+ identifier is an identifier for a stream that is not currently in the
+ "idle" state.*/
+
+ return SERF_ERROR_HTTP2_PROTOCOL_ERROR;
+ }
+ else if (parent_stream->status != H2S_OPEN
+ && parent_stream->status != H2S_HALFCLOSED_LOCAL)
+ {
+ /* PUSH_PROMISE frames MUST only be sent on a peer-initiated stream that
+ is in either the "open" or "half-closed (remote)" state. The stream
+ identifier of a PUSH_PROMISE frame indicates the stream it is
+ associated with. If the stream identifier field specifies the value
+ 0x0, a recipient MUST respond with a connection error (Section 5.4.1)
+ of type PROTOCOL_ERROR.*/
+
+ return SERF_ERROR_HTTP2_PROTOCOL_ERROR;
+ }
+
+ promised_stream = serf_http2__stream_get(h2, streamid, TRUE, FALSE);
+ if (!promised_stream || promised_stream->status != H2S_IDLE)
+ return SERF_ERROR_HTTP2_PROTOCOL_ERROR;
+
+ promised_stream->status = H2S_RESERVED_REMOTE;
+
+ /* Store data to allow stream to handle the promise */
+ parent_stream->new_reserved_stream = promised_stream;
return APR_SUCCESS;
}
@@ -933,8 +982,7 @@ http2_process(serf_http2_protocol_t *h2)
http2_handle_priority,
stream, h2->allocator);
}
-
- if (frametype == HTTP2_FRAME_TYPE_PUSH_PROMISE)
+ else if (frametype == HTTP2_FRAME_TYPE_PUSH_PROMISE)
{
body = serf_bucket_prefix_create(body,
HTTP2_PROMISE_DATA_SIZE,
@@ -1026,8 +1074,9 @@ http2_process(serf_http2_protocol_t *h2)
h2->hpack_tbl, h2->allocator);
}
}
- else /* We have a data bucket */
+ else if (! reset_reason)
{
+ /* We have a data bucket */
body = serf_http2__stream_handle_data(
stream, body, frametype,
(frameflags & HTTP2_FLAG_END_STREAM),
@@ -1442,7 +1491,6 @@ serf_http2__stream_get(serf_http2_protoc
if (create_for_remote
&& (streamid & 0x01) == (h2->rl_next_streamid & 0x01))
{
- serf_http2_stream_t *rs;
stream = serf_http2__stream_create(h2, streamid,
h2->lr_default_window,
h2->rl_default_window,
Modified: serf/trunk/protocols/http2_protocol.h
URL:
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_protocol.h?rev=1712013&r1=1712012&r2=1712013&view=diff
==============================================================================
--- serf/trunk/protocols/http2_protocol.h (original)
+++ serf/trunk/protocols/http2_protocol.h Mon Nov 2 14:31:58 2015
@@ -142,6 +142,9 @@ typedef struct serf_http2_stream_t
H2S_CLOSED
} status;
+ /* Used while receiving a promise stream */
+ struct serf_http2_stream_t *new_reserved_stream;
+
/* TODO: Priority, etc. */
} serf_http2_stream_t;
Modified: serf/trunk/protocols/http2_stream.c
URL:
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_stream.c?rev=1712013&r1=1712012&r2=1712013&view=diff
==============================================================================
--- serf/trunk/protocols/http2_stream.c (original)
+++ serf/trunk/protocols/http2_stream.c Mon Nov 2 14:31:58 2015
@@ -47,10 +47,10 @@ serf_http2__stream_create(serf_http2_pro
stream->h2 = h2;
stream->alloc = alloc;
- stream->data = serf_bucket_mem_alloc(alloc, sizeof(*stream->data));
-
stream->next = stream->prev = NULL;
+ /* Delay creating this? */
+ stream->data = serf_bucket_mem_alloc(alloc, sizeof(*stream->data));
stream->data->request = NULL;
stream->data->response_agg = NULL;
@@ -63,6 +63,7 @@ serf_http2__stream_create(serf_http2_pro
stream->streamid = -1; /* Undetermined yet */
stream->status = (streamid >= 0) ? H2S_IDLE : H2S_INIT;
+ stream->new_reserved_stream = NULL;
return stream;
}
@@ -209,6 +210,65 @@ stream_setup_response(serf_http2_stream_
stream->data->response_agg = agg;
}
+static apr_status_t
+stream_promise_item(void *baton,
+ const char *key,
+ apr_size_t key_sz,
+ const char *value,
+ apr_size_t value_sz)
+{
+ serf_http2_stream_t *parent_stream = baton;
+ serf_http2_stream_t *stream = parent_stream->new_reserved_stream;
+
+ SERF_H2_assert(stream != NULL);
+
+ /* TODO: Store key+value somewhere to allow asking the application
+ if it is interested in the promised stream.
+
+ Most likely it is not interested *yet* as the HTTP/2 spec
+ recommends pushing promised items *before* the stream that
+ references them.
+
+ So we probably want to store the request anyway, to allow
+ matching this against a later added outgoing request.
+ */
+ return APR_SUCCESS;
+}
+
+static apr_status_t
+stream_promise_done(void *baton,
+ serf_bucket_t *done_agg)
+{
+ serf_http2_stream_t *parent_stream = baton;
+ serf_http2_stream_t *stream = parent_stream->new_reserved_stream;
+
+ SERF_H2_assert(stream != NULL);
+ SERF_H2_assert(stream->status == H2S_RESERVED_REMOTE);
+ parent_stream->new_reserved_stream = NULL; /* End of PUSH_PROMISE */
+
+ /* Anything else? */
+
+
+ /* ### Absolute minimal implementation.
+ Just sending that we are not interested in the initial SETTINGS
+ would be the easier approach. */
+ serf_http2__stream_reset(stream, SERF_ERROR_HTTP2_REFUSED_STREAM, TRUE);
+
+
+
+
+ /* Exit condition:
+ * Either we should accept the stream and are ready to receive
+ HEADERS and DATA on it.
+ * Or we aren't and reject the stream
+ */
+ SERF_H2_assert(stream->status == H2S_CLOSED
+ || stream->data->request != NULL);
+
+ /* We must return a proper error or EOF here! */
+ return APR_EOF;
+}
+
serf_bucket_t *
serf_http2__stream_handle_hpack(serf_http2_stream_t *stream,
serf_bucket_t *bucket,
@@ -219,23 +279,45 @@ serf_http2__stream_handle_hpack(serf_htt
serf_config_t *config,
serf_bucket_alloc_t *allocator)
{
- if (!stream->data->response_agg)
- stream_setup_response(stream, config);
+ if (frametype == HTTP2_FRAME_TYPE_HEADERS)
+ {
+ if (!stream->data->response_agg)
+ stream_setup_response(stream, config);
- bucket = serf__bucket_hpack_decode_create(bucket, NULL, NULL, max_entry_size,
- hpack_tbl, allocator);
+ bucket = serf__bucket_hpack_decode_create(bucket, NULL, NULL,
max_entry_size,
+ hpack_tbl, allocator);
- serf_bucket_aggregate_append(stream->data->response_agg, bucket);
+ serf_bucket_aggregate_append(stream->data->response_agg, bucket);
- if (end_stream)
- {
- if (stream->status == H2S_HALFCLOSED_LOCAL)
- stream->status = H2S_CLOSED;
- else
- stream->status = H2S_HALFCLOSED_REMOTE;
+ if (end_stream)
+ {
+ if (stream->status == H2S_HALFCLOSED_LOCAL)
+ stream->status = H2S_CLOSED;
+ else
+ stream->status = H2S_HALFCLOSED_REMOTE;
+ }
+ return NULL; /* We want to drain the bucket ourselves */
}
+ else
+ {
+ serf_bucket_t *agg;
+ SERF_H2_assert(frametype == HTTP2_FRAME_TYPE_PUSH_PROMISE);
+
+ /* First create the HPACK decoder as requested */
+ bucket = serf__bucket_hpack_decode_create(bucket,
+ stream_promise_item, stream,
+ max_entry_size,
+ hpack_tbl, allocator);
+
+ /* And now wrap around it the easiest way to get an EOF callback */
+ agg = serf_bucket_aggregate_create(allocator);
+ serf_bucket_aggregate_append(agg, bucket);
- return NULL;
+ serf_bucket_aggregate_hold_open(agg, stream_promise_done, stream);
+
+ /* And return the aggregate, so the bucket will be drained for us */
+ return agg;
+ }
}
serf_bucket_t *
