Author: rhuijben
Date: Wed Nov 25 10:56:57 2015
New Revision: 1716352

URL: http://svn.apache.org/viewvc?rev=1716352&view=rev
Log:
Following up on r1716346, fix hpack decoding to work like the other decode 
buckets
and don't return eof from the processing when the bucket is not done yet. Remove
unneeded callback as callers just want to read the headers.

* buckets/hpack_buckets.c
  (serf_hpack_decode_ctx_t): Remove callback.
  (serf__bucket_hpack_decode_create): Remove two arguments.
  (handle_read_entry_and_clear): Remove callback handling.
  (hpack_process): Return success on EOF.

* protocols/fcgi_protocol.c
  (fcgi_process): Handle empty read.

* protocols/http2_buckets.h
  (serf__bucket_hpack_decode_create): Remove arguments. Tweak comment.

* protocols/http2_protocol.c
  (http2_process): Handle empty read.
  (http2_process): Update caller.

* protocols/http2_stream.c
  (stream_promise_item): Remove function.
  (serf_http2__stream_handle_hpack): Update caller.

Modified:
    serf/trunk/buckets/hpack_buckets.c
    serf/trunk/protocols/fcgi_protocol.c
    serf/trunk/protocols/http2_buckets.h
    serf/trunk/protocols/http2_protocol.c
    serf/trunk/protocols/http2_stream.c

Modified: serf/trunk/buckets/hpack_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/hpack_buckets.c?rev=1716352&r1=1716351&r2=1716352&view=diff
==============================================================================
--- serf/trunk/buckets/hpack_buckets.c (original)
+++ serf/trunk/buckets/hpack_buckets.c Wed Nov 25 10:56:57 2015
@@ -1118,13 +1118,6 @@ typedef struct serf_hpack_decode_ctx_t
     serf_bucket_t *stream;
     apr_size_t header_allowed;
 
-    apr_status_t(*item_callback)(void *baton,
-                                 const char *key,
-                                 apr_size_t key_size,
-                                 const char *value,
-                                 apr_size_t value_size);
-    void *item_baton;
-
     char *buffer;
     apr_size_t buffer_size;
     apr_size_t buffer_used;
@@ -1165,13 +1158,6 @@ typedef struct serf_hpack_decode_ctx_t
 
 serf_bucket_t *
 serf__bucket_hpack_decode_create(serf_bucket_t *stream,
-                                 apr_status_t(*item_callback)(
-                                     void *baton,
-                                     const char *key,
-                                     apr_size_t key_size,
-                                     const char *value,
-                                     apr_size_t value_size),
-                                 void *item_baton,
                                  apr_size_t max_header_size,
                                  serf_hpack_table_t *hpack_table,
                                  serf_bucket_alloc_t *alloc)
@@ -1194,19 +1180,8 @@ serf__bucket_hpack_decode_create(serf_bu
     ctx->buffer_used = 0;
     ctx->buffer = serf_bucket_mem_alloc(alloc, ctx->buffer_size);
 
-    if (item_callback)
-    {
-        ctx->item_callback = item_callback;
-        ctx->item_baton = item_baton;
-        ctx->agg = NULL;
-    }
-    else
-    {
-        ctx->item_callback = NULL;
-        ctx->item_baton = NULL;
-        ctx->agg = serf_bucket_aggregate_create(alloc);
-        ctx->headers = NULL;
-    }
+    ctx->agg = serf_bucket_aggregate_create(alloc);
+    ctx->headers = NULL;
 
     return serf_bucket_create(&serf_bucket_type__hpack_decode, alloc, ctx);
 }
@@ -1390,16 +1365,7 @@ handle_read_entry_and_clear(serf_hpack_d
               "Parsed from HPACK: %.*s: %.*s\n",
               ctx->key_size, ctx->key, ctx->val_size, ctx->val);
 
-    if (ctx->item_callback)
-    {
-        status = ctx->item_callback(ctx->item_baton,
-                                    ctx->key, ctx->key_size,
-                                    ctx->val, ctx->val_size);
-
-        if (status)
-            return status;
-    }
-    else if (!ctx->headers)
+    if (!ctx->headers)
     {
         serf_bucket_t *b;
 
@@ -1597,7 +1563,7 @@ hpack_process(serf_bucket_t *bucket)
     apr_status_t status = APR_SUCCESS;
 
     if (ctx->hit_eof)
-        return APR_EOF;
+        return APR_SUCCESS;
 
     while (status == APR_SUCCESS)
     {
@@ -1895,6 +1861,7 @@ hpack_process(serf_bucket_t *bucket)
                                &tbl->rl_last, &tbl->rl_size,
                                tbl->rl_max_table_size, tbl->alloc);
         }
+        return APR_SUCCESS;
     }
 
     return status;

Modified: serf/trunk/protocols/fcgi_protocol.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/protocols/fcgi_protocol.c?rev=1716352&r1=1716351&r2=1716352&view=diff
==============================================================================
--- serf/trunk/protocols/fcgi_protocol.c (original)
+++ serf/trunk/protocols/fcgi_protocol.c Wed Nov 25 10:56:57 2015
@@ -206,7 +206,8 @@ static apr_status_t fcgi_process(serf_fc
             {
                 SERF_FCGI_assert(fcgi->read_frame != NULL);
                 SERF_FCGI_assert(!fcgi->in_frame);
-                return status;
+                return (status == SERF_ERROR_EMPTY_READ) ? APR_SUCCESS
+                                                         : status;
             }
             else
             {

Modified: serf/trunk/protocols/http2_buckets.h
URL: 
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_buckets.h?rev=1716352&r1=1716351&r2=1716352&view=diff
==============================================================================
--- serf/trunk/protocols/http2_buckets.h (original)
+++ serf/trunk/protocols/http2_buckets.h Wed Nov 25 10:56:57 2015
@@ -177,22 +177,13 @@ serf__hpack_table_set_max_table_size(ser
 extern const serf_bucket_type_t serf_bucket_type__hpack_decode;
 #define SERF_BUCKET_IS_HPACK_DECODE(b) SERF_BUCKET_CHECK((b), hpack_decode)
 
-/* If ITEM_CALLBACK is not null calls it for every item while reading, and
-   the bucket will just return no data until EOF.
-
-   If ITEM_CALLBACK is NULL, the bucket will read as a HTTP/1 like header 
block,
-   starting with a status line and ending with "\r\n\r\n", which allows using
-   the result as the start of the result for a response_bucket.
+/* The bucket will read as a HTTP/1 like header block,
+   starting with either a status or a request line and ending with "\r\n\r\n",
+   which allows using the result as the start of the result for a response
+   or request bucket.
  */
 serf_bucket_t *
 serf__bucket_hpack_decode_create(serf_bucket_t *stream,
-                                 apr_status_t(*item_callback)(
-                                                  void *baton,
-                                                  const char *key,
-                                                  apr_size_t key_size,
-                                                  const char *value,
-                                                  apr_size_t value_size),
-                                 void *item_baton,
                                  apr_size_t max_header_size,
                                  serf_hpack_table_t *hpack_table,
                                  serf_bucket_alloc_t *alloc);

Modified: serf/trunk/protocols/http2_protocol.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_protocol.c?rev=1716352&r1=1716351&r2=1716352&view=diff
==============================================================================
--- serf/trunk/protocols/http2_protocol.c (original)
+++ serf/trunk/protocols/http2_protocol.c Wed Nov 25 10:56:57 2015
@@ -1071,7 +1071,8 @@ http2_process(serf_http2_protocol_t *h2)
             {
                 SERF_H2_assert(h2->read_frame != NULL);
                 SERF_H2_assert(!h2->in_frame);
-                return status;
+                return (status == SERF_ERROR_EMPTY_READ) ? APR_SUCCESS
+                                                         : status;
             }
             else
             {
@@ -1255,7 +1256,7 @@ http2_process(serf_http2_protocol_t *h2)
                           /* Even when we don't want to process the headers we
                               must read them to update the HPACK state */
                             body = serf__bucket_hpack_decode_create(
-                                body, NULL, NULL,
+                                body,
                                 HTTP2_MAX_HEADER_ENTRYSIZE,
                                 h2->hpack_tbl, h2->allocator);
                         }

Modified: serf/trunk/protocols/http2_stream.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/protocols/http2_stream.c?rev=1716352&r1=1716351&r2=1716352&view=diff
==============================================================================
--- serf/trunk/protocols/http2_stream.c (original)
+++ serf/trunk/protocols/http2_stream.c Wed Nov 25 10:56:57 2015
@@ -622,31 +622,6 @@ stream_setup_response(serf_http2_stream_
 }
 
 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)
 {
@@ -697,8 +672,7 @@ serf_http2__stream_handle_hpack(serf_htt
 
         stream->data->tbl = hpack_tbl;
 
-        bucket = serf__bucket_hpack_decode_create(bucket, NULL, NULL,
-                                                  max_entry_size,
+        bucket = serf__bucket_hpack_decode_create(bucket, max_entry_size,
                                                   hpack_tbl, allocator);
 
         serf_bucket_aggregate_append(stream->data->response_agg, bucket);
@@ -718,9 +692,18 @@ serf_http2__stream_handle_hpack(serf_htt
         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,
+
+     /* 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.
+     */
+        bucket = serf__bucket_hpack_decode_create(bucket, max_entry_size,
                                                   hpack_tbl, allocator);
 
         /* And now wrap around it the easiest way to get an EOF callback */


Reply via email to