serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1712013 - in /serf/trunk/protocols: http2_protocol.c http2_protocol.h http2_stream.c
Date Mon, 02 Nov 2015 14:31:58 GMT
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 *



Mime
View raw message