serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1709256 - in /serf/trunk: buckets/http2_frame_buckets.c serf_bucket_types.h test/test_buckets.c
Date Sun, 18 Oct 2015 10:45:31 GMT
Author: rhuijben
Date: Sun Oct 18 10:45:31 2015
New Revision: 1709256

URL: http://svn.apache.org/viewvc?rev=1709256&view=rev
Log:
Tweak http2 buckets a bit. Remove an unneeded argument and the necessary
state for providing that value.

* serf-dev/dev/buckets/http2_frame_buckets.c
  (http2_unframe_context_t): Remove unused variable. Reorder variables
    based on their size to not unneededly keep a huge state.
  (serf_bucket_http2_unframe_create): Store boolean as char.
  (serf_http2_unframe_bucket_read_info): Rename to...
  (serf_bucket_http2_unframe_read_info): ... this and remove payload length
    argument. Update caller.

  (serf_http2_unframe_read,
   serf_http2_unframe_read_iovec,
   serf_http2_unframe_peek,
   serf_http2_unframe_get_remaining): Update caller.
  (serf_http2_unpad_read_padsize): Implement http2 MUST.

* serf-dev/dev/serf_bucket_types.h
  (serf_http2_unframe_bucket_read_info): Rename to...
  (serf_bucket_http2_unframe_read_info): ... this and remove argument.

* serf-dev/dev/test/test_buckets.c
  (test_http2_unframe_buckets): Update caller.
  (test_http2_unpad_buckets): Update caller. Extend test to test a few
    edge cases.

Modified:
    serf/trunk/buckets/http2_frame_buckets.c
    serf/trunk/serf_bucket_types.h
    serf/trunk/test/test_buckets.c

Modified: serf/trunk/buckets/http2_frame_buckets.c
URL: http://svn.apache.org/viewvc/serf/trunk/buckets/http2_frame_buckets.c?rev=1709256&r1=1709255&r2=1709256&view=diff
==============================================================================
--- serf/trunk/buckets/http2_frame_buckets.c (original)
+++ serf/trunk/buckets/http2_frame_buckets.c Sun Oct 18 10:45:31 2015
@@ -33,16 +33,15 @@ typedef struct http2_unframe_context_t
   apr_size_t max_payload_size;
 
   apr_size_t prefix_remaining;
-  unsigned char prefix_buffer[FRAME_PREFIX_SIZE];
 
   /* These fields are only set after prefix_remaining is 0 */
-  apr_size_t payload_length;  /* 0 <= payload_length < 2^24 */
-  apr_int32_t stream_id;      /* 0 <= stream_id < 2^31 */
+  apr_size_t payload_remaining;  /* 0 <= payload_length < 2^24 */
+  apr_int32_t stream_id;         /* 0 <= stream_id < 2^31 */
   unsigned char frame_type;
   unsigned char flags;
 
-  apr_size_t payload_remaining;
-  int destroy_stream;
+  unsigned char prefix_buffer[FRAME_PREFIX_SIZE];
+  char destroy_stream;
 } http2_unframe_context_t;
 
 serf_bucket_t *
@@ -57,14 +56,13 @@ serf_bucket_http2_unframe_create(serf_bu
   ctx->stream = stream;
   ctx->max_payload_size = max_payload_size;
   ctx->prefix_remaining = sizeof(ctx->prefix_buffer);
-  ctx->destroy_stream = destroy_stream;
+  ctx->destroy_stream = (destroy_stream != 0);
 
   return serf_bucket_create(&serf_bucket_type_http2_unframe, allocator, ctx);
 }
 
 apr_status_t
-serf_http2_unframe_bucket_read_info(serf_bucket_t *bucket,
-                                    apr_size_t *payload_length,
+serf_bucket_http2_unframe_read_info(serf_bucket_t *bucket,
                                     apr_int32_t *stream_id,
                                     unsigned char *frame_type,
                                     unsigned char *flags)
@@ -76,8 +74,6 @@ serf_http2_unframe_bucket_read_info(serf
 
   if (ctx->prefix_remaining == 0)
     {
-      if (payload_length)
-        *payload_length = ctx->payload_length;
       if (stream_id)
         *stream_id = ctx->stream_id;
       if (frame_type)
@@ -98,9 +94,9 @@ serf_http2_unframe_bucket_read_info(serf
 
       if (ctx->prefix_remaining == 0)
         {
-          ctx->payload_length = (ctx->prefix_buffer[0] << 16)
-                                | (ctx->prefix_buffer[1] << 8)
-                                | (ctx->prefix_buffer[2]);
+          apr_size_t payload_length = (ctx->prefix_buffer[0] << 16)
+                                    | (ctx->prefix_buffer[1] << 8)
+                                    | (ctx->prefix_buffer[2]);
           ctx->frame_type = ctx->prefix_buffer[3];
           ctx->flags = ctx->prefix_buffer[4];
           /* Highest bit of stream_id MUST be ignored */
@@ -109,11 +105,11 @@ serf_http2_unframe_bucket_read_info(serf
                            | (ctx->prefix_buffer[7] << 8)
                            | (ctx->prefix_buffer[8]);
 
-          ctx->payload_remaining = ctx->payload_length;
+          ctx->payload_remaining = payload_length;
 
           /* Use recursion to fill output arguments if necessary */
-          serf_http2_unframe_bucket_read_info(bucket, payload_length,
-                                              stream_id, frame_type, flags);
+          serf_bucket_http2_unframe_read_info(bucket, stream_id, frame_type,
+                                              flags);
 
           /* https://tools.ietf.org/html/rfc7540#section-4.2
             An endpoint MUST send an error code of FRAME_SIZE_ERROR if a frame
@@ -121,7 +117,7 @@ serf_http2_unframe_bucket_read_info(serf
             limit defined for the frame type, or is too small to contain
             mandatory frame data.
           */
-          if (ctx->max_payload_size < ctx->payload_remaining)
+          if (ctx->max_payload_size < payload_length)
               return SERF_ERROR_HTTP2_FRAME_SIZE_ERROR;
         }
     }
@@ -137,8 +133,7 @@ serf_http2_unframe_read(serf_bucket_t *b
   http2_unframe_context_t *ctx = bucket->data;
   apr_status_t status;
 
-  status = serf_http2_unframe_bucket_read_info(bucket, NULL, NULL,
-                                               NULL, NULL);
+  status = serf_bucket_http2_unframe_read_info(bucket, NULL, NULL, NULL);
 
   if (status)
     {
@@ -177,8 +172,7 @@ serf_http2_unframe_read_iovec(serf_bucke
   http2_unframe_context_t *ctx = bucket->data;
   apr_status_t status;
 
-  status = serf_http2_unframe_bucket_read_info(bucket, NULL, NULL,
-                                               NULL, NULL);
+  status = serf_bucket_http2_unframe_read_info(bucket, NULL, NULL, NULL);
 
   if (status)
     {
@@ -222,8 +216,7 @@ serf_http2_unframe_peek(serf_bucket_t *b
   http2_unframe_context_t *ctx = bucket->data;
   apr_status_t status;
 
-  status = serf_http2_unframe_bucket_read_info(bucket, NULL, NULL,
-                                               NULL, NULL);
+  status = serf_bucket_http2_unframe_read_info(bucket, NULL, NULL, NULL);
 
   if (status)
     {
@@ -258,8 +251,7 @@ serf_http2_unframe_get_remaining(serf_bu
   http2_unframe_context_t *ctx = bucket->data;
   apr_status_t status;
 
-  status = serf_http2_unframe_bucket_read_info(bucket, NULL, NULL,
-                                               NULL, NULL);
+  status = serf_bucket_http2_unframe_read_info(bucket, NULL, NULL, NULL);
 
   if (status)
     return SERF_LENGTH_UNKNOWN;
@@ -336,15 +328,17 @@ serf_http2_unpad_read_padsize(serf_bucke
           || remaining > APR_SIZE_MAX)
         return APR_EGENERAL; /* Can't calculate padding size */
 
+      /* http://tools.ietf.org/html/rfc7540#section-6.1
+         If the length of the padding is the length of the
+         frame payload or greater, the recipient MUST treat this as a
+         connection error (Section 5.4.1) of type PROTOCOL_ERROR.
+
+         The frame payload includes the length byte, so when remaining
+         is 0, that isn't a protocol error */
       if (remaining < ctx->pad_length)
-        {
-          ctx->payload_remaining = 0;
-          ctx->pad_remaining = (apr_size_t)remaining;
-        }
-      else
-        {
-          ctx->payload_remaining = (apr_size_t)remaining - ctx->pad_length;
-        }
+        return SERF_ERROR_HTTP2_PROTOCOL_ERROR;
+
+      ctx->payload_remaining = (apr_size_t)remaining - ctx->pad_length;
     }
   return status;
 }

Modified: serf/trunk/serf_bucket_types.h
URL: http://svn.apache.org/viewvc/serf/trunk/serf_bucket_types.h?rev=1709256&r1=1709255&r2=1709256&view=diff
==============================================================================
--- serf/trunk/serf_bucket_types.h (original)
+++ serf/trunk/serf_bucket_types.h Sun Oct 18 10:45:31 2015
@@ -767,14 +767,13 @@ serf_bucket_http2_unframe_create(serf_bu
 
 /* Obtains the frame header state, reading from the bucket if necessary.
    If the header was read successfully (or was already read before calling)
-   the *payload_length, *stream_id, * frame_type and *flags values
-   (when not pointing to NULL) will be set to the requested values.
+   the *stream_id, * frame_type and *flags values (when not pointing to NULL)
+   will be set to the requested values.
 
    returns APR_SUCCESS when the header was already read before calling this,
    function. Otherwise it will return the result of reading. */
 apr_status_t
-serf_http2_unframe_bucket_read_info(serf_bucket_t *bucket,
-                                    apr_size_t *payload_length,
+serf_bucket_http2_unframe_read_info(serf_bucket_t *bucket,
                                     apr_int32_t *stream_id,
                                     unsigned char *frame_type,
                                     unsigned char *flags);

Modified: serf/trunk/test/test_buckets.c
URL: http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1709256&r1=1709255&r2=1709256&view=diff
==============================================================================
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Sun Oct 18 10:45:31 2015
@@ -1792,17 +1792,14 @@ static void test_http2_unframe_buckets(C
                                   "\x00\x02\x00\x00\x00\x00", read_len));
 
   {
-    apr_size_t payload_len;
     apr_int32_t stream_id;
     unsigned char frame_type, flags;
 
     CuAssertIntEquals(tc, 0,
-                      serf_http2_unframe_bucket_read_info(unframe,
-                                                          &payload_len,
+                      serf_bucket_http2_unframe_read_info(unframe,
                                                           &stream_id,
                                                           &frame_type,
                                                           &flags));
-    CuAssertIntEquals(tc, 12, payload_len);
     CuAssertIntEquals(tc, 0, stream_id);
     CuAssertIntEquals(tc, 4, frame_type);
     CuAssertIntEquals(tc, 0, flags);
@@ -1822,17 +1819,14 @@ static void test_http2_unframe_buckets(C
                                   read_len));
 
   {
-    apr_size_t payload_len;
     apr_int32_t stream_id;
     unsigned char frame_type, flags;
 
     CuAssertIntEquals(tc, 0,
-                      serf_http2_unframe_bucket_read_info(unframe,
-                                                          &payload_len,
+                      serf_bucket_http2_unframe_read_info(unframe,
                                                           &stream_id,
                                                           &frame_type,
                                                           &flags));
-    CuAssertIntEquals(tc, 6, payload_len);
     CuAssertIntEquals(tc, 0x03040506, stream_id);
     CuAssertIntEquals(tc, 0x01, frame_type);
     CuAssertIntEquals(tc, 0x02, flags);
@@ -1885,7 +1879,7 @@ static void test_http2_unpad_buckets(CuT
     apr_int32_t streamid;
     unsigned char frame_type, flags;
 
-    status = serf_http2_unframe_bucket_read_info(unframe, NULL, &streamid,
+    status = serf_bucket_http2_unframe_read_info(unframe, &streamid,
                                                  &frame_type, &flags);
 
     CuAssertIntEquals(tc, APR_SUCCESS, status);
@@ -1903,6 +1897,22 @@ static void test_http2_unpad_buckets(CuT
   CuAssertIntEquals(tc, sizeof(result1), read_len);
 
   read_and_check_bucket(tc, raw, "Extra");
+
+  raw = serf_bucket_simple_create("\0a", 2, NULL, NULL, alloc);
+  unpad = serf_bucket_http2_unpad_create(raw, TRUE, alloc);
+  read_and_check_bucket(tc, unpad, "a");
+
+  raw = serf_bucket_simple_create("\5a", 2, NULL, NULL, alloc);
+  unpad = serf_bucket_http2_unpad_create(raw, TRUE, alloc);
+
+  {
+    const char *data;
+    apr_size_t sz;
+
+    CuAssertIntEquals(tc, SERF_ERROR_HTTP2_PROTOCOL_ERROR,
+                      serf_bucket_read(unpad, SERF_READ_ALL_AVAIL,
+                                       &data, &sz));
+  }
 }
 
 CuSuite *test_buckets(void)



Mime
View raw message