Author: rhuijben
Date: Sat Oct 17 17:02:26 2015
New Revision: 1709198
URL: http://svn.apache.org/viewvc?rev=1709198&view=rev
Log:
Following up on r1709185, add a serf bucket that can remove the optional
padding from a http2 frame when needed.
Fix some issues in the unframe buckets while working on this.
* serf-dev/dev/buckets/http2_frame_buckets.c
(http2_unframe_context_t,
serf_bucket_http2_unframe_create): Record if we should destroy the inner
stream. It is very unlikely that we should, but in general buckets do
this.
(serf_http2_unframe_read,
serf_http2_unframe_read_iovec,
serf_http2_unframe_peek): Properly set len when exiting early.
(serf_http2_unframe_destroy): New function.
(serf_bucket_type_http2_unframe): Add serf_http2_unframe_destroy.
(*): Add unpad bucket.
* serf-dev/dev/serf_bucket_types.h
(serf_bucket_http2_unframe_create): Add destroy_stream flag.
(serf_bucket_type_http2_unpad): New bucket type.
(SERF_BUCKET_IS_HTTP2_UNPAD): New define.
(serf_bucket_http2_unpad_create): New function.
* serf-dev/dev/test/test_buckets.c
(test_http2_unframe_buckets): Tweak expectations. Add type test.
(test_http2_unpad_buckets): New function.
(test_buckets): Add test_http2_unpad_buckets.
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=1709198&r1=1709197&r2=1709198&view=diff
==============================================================================
--- serf/trunk/buckets/http2_frame_buckets.c (original)
+++ serf/trunk/buckets/http2_frame_buckets.c Sat Oct 17 17:02:26 2015
@@ -42,10 +42,12 @@ typedef struct http2_unframe_context_t
unsigned char flags;
apr_size_t payload_remaining;
+ int destroy_stream;
} http2_unframe_context_t;
serf_bucket_t *
serf_bucket_http2_unframe_create(serf_bucket_t *stream,
+ int destroy_stream,
apr_size_t max_payload_size,
serf_bucket_alloc_t *allocator)
{
@@ -55,7 +57,7 @@ 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;
return serf_bucket_create(&serf_bucket_type_http2_unframe, allocator, ctx);
}
@@ -139,7 +141,10 @@ serf_http2_unframe_read(serf_bucket_t *b
NULL, NULL);
if (status)
- return status;
+ {
+ *len = 0;
+ return status;
+ }
if (ctx->payload_remaining == 0)
{
@@ -176,7 +181,10 @@ serf_http2_unframe_read_iovec(serf_bucke
NULL, NULL);
if (status)
- return status;
+ {
+ *vecs_used = 0;
+ return status;
+ }
if (ctx->payload_remaining == 0)
{
@@ -218,7 +226,10 @@ serf_http2_unframe_peek(serf_bucket_t *b
NULL, NULL);
if (status)
- return status;
+ {
+ *len = 0;
+ return status;
+ }
status = serf_bucket_peek(ctx->stream, data, len);
if (!SERF_BUCKET_READ_ERROR(status))
@@ -230,6 +241,17 @@ serf_http2_unframe_peek(serf_bucket_t *b
return status;
}
+static void
+serf_http2_unframe_destroy(serf_bucket_t *bucket)
+{
+ http2_unframe_context_t *ctx = bucket->data;
+
+ if (ctx->destroy_stream)
+ serf_bucket_destroy(ctx->stream);
+
+ serf_default_destroy_and_data(bucket);
+}
+
static apr_uint64_t
serf_http2_unframe_get_remaining(serf_bucket_t *bucket)
{
@@ -256,8 +278,245 @@ const serf_bucket_type_t serf_bucket_typ
serf_default_read_for_sendfile,
serf_buckets_are_v2,
serf_http2_unframe_peek,
- serf_default_destroy_and_data,
+ serf_http2_unframe_destroy,
serf_default_read_bucket,
serf_http2_unframe_get_remaining,
serf_default_ignore_config
};
+
+typedef struct http2_unpad_context_t
+{
+ serf_bucket_t *stream;
+ apr_size_t payload_remaining;
+ apr_size_t pad_remaining;
+ apr_size_t pad_length;
+ int padsize_read;
+ int destroy_stream;
+} http2_unpad_context_t;
+
+serf_bucket_t *
+serf_bucket_http2_unpad_create(serf_bucket_t *stream,
+ int destroy_stream,
+ serf_bucket_alloc_t *allocator)
+{
+ http2_unpad_context_t *ctx;
+
+ ctx = serf_bucket_mem_alloc(allocator, sizeof(*ctx));
+ ctx->stream = stream;
+ ctx->padsize_read = FALSE;
+ ctx->destroy_stream = destroy_stream;
+
+ return serf_bucket_create(&serf_bucket_type_http2_unpad, allocator, ctx);
+}
+
+static apr_status_t
+serf_http2_unpad_read_padsize(serf_bucket_t *bucket)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+ apr_status_t status;
+ const char *data;
+ apr_size_t len;
+
+ if (ctx->padsize_read)
+ return APR_SUCCESS;
+
+ status = serf_bucket_read(ctx->stream, 1, &data, &len);
+ if (! SERF_BUCKET_READ_ERROR(status))
+ {
+ apr_int64_t remaining;
+ ctx->pad_length = *(unsigned char *)data;
+ ctx->pad_remaining = ctx->pad_length;
+ ctx->padsize_read = TRUE;
+
+ /* We call get_remaining() *after* reading from ctx->stream,
+ to allow the framing above us to be read before we call this */
+ remaining = serf_bucket_get_remaining(ctx->stream);
+
+ if (remaining == SERF_LENGTH_UNKNOWN
+ || remaining > APR_SIZE_MAX)
+ return APR_EGENERAL; /* Can't calculate padding size */
+
+ 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 status;
+}
+
+static apr_status_t
+serf_http2_unpad_read_padding(serf_bucket_t *bucket)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+ apr_status_t status;
+
+ /* ### What is the most efficient way to skip data?
+ Should we use serf_bucket_read_iovec()? */
+
+ while (ctx->pad_remaining > 0)
+ {
+ apr_size_t pad_read;
+ const char *pad_data;
+
+ status = serf_bucket_read(ctx->stream, ctx->pad_remaining,
+ &pad_data, &pad_read);
+
+ if (! SERF_BUCKET_READ_ERROR(status))
+ ctx->pad_remaining -= pad_read;
+
+ if (status)
+ return status;
+ }
+
+ return APR_EOF;
+}
+
+static apr_status_t
+serf_http2_unpad_read(serf_bucket_t *bucket,
+ apr_size_t requested,
+ const char **data,
+ apr_size_t *len)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+ apr_status_t status;
+
+ status = serf_http2_unpad_read_padsize(bucket);
+
+ if (status)
+ {
+ *len = 0;
+ return status;
+ }
+ else if (ctx->payload_remaining == 0)
+ {
+ *len = 0;
+ return serf_http2_unpad_read_padding(bucket);
+ }
+
+ if (requested > ctx->payload_remaining)
+ requested = ctx->payload_remaining;
+
+ status = serf_bucket_read(ctx->stream, requested, data, len);
+ if (! SERF_BUCKET_READ_ERROR(status))
+ {
+ ctx->payload_remaining -= *len;
+ }
+
+ return status;
+}
+
+static apr_status_t
+serf_http2_unpad_read_iovec(serf_bucket_t *bucket,
+ apr_size_t requested,
+ int vecs_size,
+ struct iovec *vecs,
+ int *vecs_used)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+ apr_status_t status;
+
+ status = serf_http2_unpad_read_padsize(bucket);
+
+ if (status)
+ {
+ *vecs_used = 0;
+ return status;
+ }
+ else if (ctx->payload_remaining == 0)
+ {
+ *vecs_used = 0;
+ return serf_http2_unpad_read_padding(bucket);
+ }
+
+ if (requested > ctx->payload_remaining)
+ requested = ctx->payload_remaining;
+
+ status = serf_bucket_read_iovec(ctx->stream, requested,
+ vecs_size, vecs, vecs_used);
+ if (! SERF_BUCKET_READ_ERROR(status))
+ {
+ int i;
+ apr_size_t len = 0;
+
+ for (i = 0; i < *vecs_used; i++)
+ len += vecs[i].iov_len;
+
+ ctx->payload_remaining -= len;
+ }
+
+ return status;
+}
+
+static apr_status_t
+serf_http2_unpad_peek(serf_bucket_t *bucket,
+ const char **data,
+ apr_size_t *len)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+ apr_status_t status;
+
+ status = serf_http2_unpad_read_padsize(bucket);
+
+ if (status)
+ {
+ *len = 0;
+ return status;
+ }
+
+ status = serf_bucket_peek(ctx->stream, data, len);
+ if (!SERF_BUCKET_READ_ERROR(status))
+ {
+ if (*len > ctx->payload_remaining)
+ *len = ctx->payload_remaining;
+ }
+
+ return status;
+}
+
+static void
+serf_http2_unpad_destroy(serf_bucket_t *bucket)
+{
+ http2_unpad_context_t *ctx = bucket->data;
+
+ if (ctx->destroy_stream)
+ serf_bucket_destroy(ctx->stream);
+
+ serf_default_destroy_and_data(bucket);
+}
+
+static apr_uint64_t
+serf_http2_unpad_get_remaining(serf_bucket_t *bucket)
+{
+ http2_unframe_context_t *ctx = bucket->data;
+ apr_status_t status;
+
+ status = serf_http2_unpad_read_padsize(bucket);
+
+ if (status)
+ return SERF_LENGTH_UNKNOWN;
+
+ return ctx->payload_remaining;
+}
+
+/* ### need to implement */
+#define serf_h2_dechunk_readline NULL
+
+const serf_bucket_type_t serf_bucket_type_http2_unpad = {
+ "H2-UNPAD",
+ serf_http2_unpad_read,
+ serf_h2_dechunk_readline /* ### TODO */,
+ serf_http2_unpad_read_iovec,
+ serf_default_read_for_sendfile,
+ serf_buckets_are_v2,
+ serf_http2_unpad_peek,
+ serf_http2_unpad_destroy,
+ serf_default_read_bucket,
+ serf_http2_unpad_get_remaining,
+ serf_default_ignore_config
+};
+
Modified: serf/trunk/serf_bucket_types.h
URL:
http://svn.apache.org/viewvc/serf/trunk/serf_bucket_types.h?rev=1709198&r1=1709197&r2=1709198&view=diff
==============================================================================
--- serf/trunk/serf_bucket_types.h (original)
+++ serf/trunk/serf_bucket_types.h Sat Oct 17 17:02:26 2015
@@ -761,6 +761,7 @@ extern const serf_bucket_type_t serf_buc
serf_bucket_t *
serf_bucket_http2_unframe_create(serf_bucket_t *stream,
+ int destroy_stream,
apr_size_t max_payload_size,
serf_bucket_alloc_t *allocator);
@@ -780,6 +781,16 @@ serf_http2_unframe_bucket_read_info(serf
/* ==================================================================== */
+extern const serf_bucket_type_t serf_bucket_type_http2_unpad;
+#define SERF_BUCKET_IS_HTTP2_UNPAD(b) SERF_BUCKET_CHECK((b), http2_unpad)
+
+serf_bucket_t *
+serf_bucket_http2_unpad_create(serf_bucket_t *stream,
+ int destroy_stream,
+ serf_bucket_alloc_t *allocator);
+
+/* ==================================================================== */
+
/* ### do we need a PIPE bucket type? they are simple apr_file_t objects */
Modified: serf/trunk/test/test_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1709198&r1=1709197&r2=1709198&view=diff
==============================================================================
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Sat Oct 17 17:02:26 2015
@@ -1780,12 +1780,14 @@ static void test_http2_unframe_buckets(C
raw = serf_bucket_simple_create(raw_frame1, sizeof(raw_frame1),
NULL, NULL, alloc);
- unframe = serf_bucket_http2_unframe_create(raw, SERF_READ_ALL_AVAIL,
+ unframe = serf_bucket_http2_unframe_create(raw, TRUE, SERF_READ_ALL_AVAIL,
alloc);
+ CuAssertTrue(tc, SERF_BUCKET_IS_HTTP2_UNFRAME(unframe));
+
status = read_all(unframe, result1, sizeof(result1), &read_len);
CuAssertIntEquals(tc, APR_EOF, status);
- CuAssertIntEquals(tc, read_len, sizeof(result1));
+ CuAssertIntEquals(tc, sizeof(result1), read_len);
CuAssertIntEquals(tc, 0, memcmp(result1, "\x00\x01\x00\x00\x00\x00"
"\x00\x02\x00\x00\x00\x00", read_len));
@@ -1810,12 +1812,12 @@ static void test_http2_unframe_buckets(C
raw = serf_bucket_simple_create(raw_frame2, sizeof(raw_frame2),
NULL, NULL, alloc);
- unframe = serf_bucket_http2_unframe_create(raw, SERF_READ_ALL_AVAIL,
+ unframe = serf_bucket_http2_unframe_create(raw, TRUE, SERF_READ_ALL_AVAIL,
alloc);
status = read_all(unframe, result2, sizeof(result2), &read_len);
CuAssertIntEquals(tc, APR_EOF, status);
- CuAssertIntEquals(tc, read_len, sizeof(result2));
+ CuAssertIntEquals(tc, sizeof(result2), read_len);
CuAssertIntEquals(tc, 0, memcmp(result2, "\x00\x01\x00\x00\x00\x00",
read_len));
@@ -1841,13 +1843,69 @@ static void test_http2_unframe_buckets(C
raw = serf_bucket_simple_create(raw_frame2, sizeof(raw_frame2),
NULL, NULL, alloc);
- unframe = serf_bucket_http2_unframe_create(raw, 5,
- alloc);
+ unframe = serf_bucket_http2_unframe_create(raw, TRUE, 5, alloc);
status = read_all(unframe, result2, sizeof(result2), &read_len);
CuAssertIntEquals(tc, SERF_ERROR_HTTP2_FRAME_SIZE_ERROR, status);
}
+/* Basic test for unframe buckets. */
+static void test_http2_unpad_buckets(CuTest *tc)
+{
+ test_baton_t *tb = tc->testBaton;
+ const char raw_frame[] = "\x00\x00\x18" /* 24 bytes payload */
+ "\x00\x08" /* Data frame, padding flag */
+ "\x00\x00\x00\x07" /* Stream 7 */
+
+ "\x07" /* 7 bytes padding at end */
+
+ "\x01\x03\x05\x07\x09\x0B\x0D\x0F" /* 16 bytes */
+ "\x00\x02\x04\x06\x08\x0A\x0C\x0E"
+
+ "\x00\x00\x00\x00\x00\x00\x00" /* 7 bytes padding*/
+
+ "Extra"
+ "";
+ serf_bucket_alloc_t *alloc;
+ serf_bucket_t *raw;
+ serf_bucket_t *unframe;
+ serf_bucket_t *unpad;
+ char result1[16];
+ char result2[5];
+ apr_status_t status;
+ apr_size_t read_len;
+
+ alloc = serf_bucket_allocator_create(tb->pool, NULL, NULL);
+
+ raw = serf_bucket_simple_create(raw_frame, sizeof(raw_frame)-1,
+ NULL, NULL, alloc);
+
+ unframe = serf_bucket_http2_unframe_create(raw, FALSE, SERF_READ_ALL_AVAIL,
+ alloc);
+
+ {
+ apr_size_t streamid;
+ unsigned char frame_type, flags;
+
+ status = serf_http2_unframe_bucket_read_info(unframe, NULL, &streamid,
+ &frame_type, &flags);
+
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+ CuAssertIntEquals(tc, 7, streamid);
+ CuAssertIntEquals(tc, 0, frame_type);
+ CuAssertIntEquals(tc, 8, flags);
+ }
+
+ unpad = serf_bucket_http2_unpad_create(unframe, TRUE, alloc);
+
+ CuAssertTrue(tc, SERF_BUCKET_IS_HTTP2_UNPAD(unpad));
+
+ status = read_all(unpad, result1, sizeof(result1), &read_len);
+ CuAssertIntEquals(tc, APR_EOF, status);
+ CuAssertIntEquals(tc, sizeof(result1), read_len);
+
+ read_and_check_bucket(tc, raw, "Extra");
+}
CuSuite *test_buckets(void)
{
@@ -1879,6 +1937,7 @@ CuSuite *test_buckets(void)
SUITE_ADD_TEST(suite, test_dechunk_buckets);
SUITE_ADD_TEST(suite, test_deflate_buckets);
SUITE_ADD_TEST(suite, test_http2_unframe_buckets);
+ SUITE_ADD_TEST(suite, test_http2_unpad_buckets);
#if 0
/* This test for issue #152 takes a lot of time generating 4GB+ of random
data so it's disabled by default. */
