From 74cccba97a0184f4431b387ae00a9bf6a1c58832 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Fri, 3 Apr 2020 12:17:10 -0700 Subject: [PATCH 1/9] cookie header concatemating --- include/aws/http/private/http_impl.h | 3 ++ source/h2_decoder.c | 57 ++++++++++++++++++++++++++-- source/http.c | 1 + 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/include/aws/http/private/http_impl.h b/include/aws/http/private/http_impl.h index 61561ccce..e113abd7b 100644 --- a/include/aws/http/private/http_impl.h +++ b/include/aws/http/private/http_impl.h @@ -46,6 +46,9 @@ enum aws_http_header_name { /* Response pseudo-headers */ AWS_HTTP_HEADER_STATUS, + /* Cookie header */ + AWS_HTTP_HEADER_COOKIE, + /* Regular headers */ AWS_HTTP_HEADER_CONNECTION, AWS_HTTP_HEADER_CONTENT_LENGTH, diff --git a/source/h2_decoder.c b/source/h2_decoder.c index 6f5a38269..b1191d340 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -36,6 +36,9 @@ static const size_t s_scratch_space_size = 9; /* Stream ids & dependencies should only write the bottom 31 bits */ static const uint32_t s_31_bit_mask = UINT32_MAX >> 1; +/* initial size for cookie buffer, buffer will grow if needed */ +static const size_t s_decoder_cookie_buffer_initial_size = 512; + #define DECODER_LOGF(level, decoder, text, ...) \ AWS_LOGF_##level(AWS_LS_HTTP_DECODER, "id=%p " text, (decoder)->logging_id, __VA_ARGS__) #define DECODER_LOG(level, decoder, text) DECODER_LOGF(level, decoder, "%s", text) @@ -245,6 +248,10 @@ struct aws_h2_decoder { * A malformed header-block is not a connection error, it's a Stream Error (RFC-7540 5.4.2). * We continue decoding and report that it's malformed in on_headers_end(). */ bool malformed; + + /* Buffer up cookie header fields to concatenate separate ones */ + struct aws_byte_buf cookies; + } header_block_in_progress; /* Settings for decoder, which is based on the settings sent to the peer and ACKed by peer */ @@ -314,10 +321,16 @@ struct aws_h2_decoder *aws_h2_decoder_new(struct aws_h2_decoder_params *params) goto array_list_failed; } + if (aws_byte_buf_init( + &decoder->header_block_in_progress.cookies, decoder->alloc, s_decoder_cookie_buffer_initial_size)) { + goto buffer_init_failed; + } + return decoder; -failed_new_hpack: +buffer_init_failed: array_list_failed: +failed_new_hpack: aws_mem_release(params->alloc, allocation); failed_alloc: return NULL; @@ -327,7 +340,10 @@ static void s_reset_header_block_in_progress(struct aws_h2_decoder *decoder) { for (size_t i = 0; i < PSEUDOHEADER_COUNT; ++i) { aws_string_destroy(decoder->header_block_in_progress.pseudoheader_values[i]); } + struct aws_byte_buf cookie_backup = decoder->header_block_in_progress.cookies; AWS_ZERO_STRUCT(decoder->header_block_in_progress); + decoder->header_block_in_progress.cookies = cookie_backup; + aws_byte_buf_reset(&decoder->header_block_in_progress.cookies); } void aws_h2_decoder_destroy(struct aws_h2_decoder *decoder) { @@ -337,6 +353,7 @@ void aws_h2_decoder_destroy(struct aws_h2_decoder *decoder) { aws_array_list_clean_up(&decoder->settings_buffer_list); aws_hpack_context_destroy(decoder->hpack); s_reset_header_block_in_progress(decoder); + aws_byte_buf_clean_up(&decoder->header_block_in_progress.cookies); aws_mem_release(decoder->alloc, decoder); } @@ -1254,6 +1271,26 @@ static int s_process_header_field(struct aws_h2_decoder *decoder, const struct a /* #TODO Validate characters used in header_field->value */ + if (name_enum == AWS_HTTP_HEADER_COOKIE) { + /* for a header cookie, we will not fire callback until we concatenate them all, let's store it at the + * buffer */ + if (current_block->cookies.len) { + /* add a delimiter */ + struct aws_byte_cursor delimiter = aws_byte_cursor_from_c_str("; "); + if (aws_byte_buf_append_dynamic(¤t_block->cookies, &delimiter)) { + DECODER_LOG(ERROR, decoder, "Store cookie delimiter to buffer failed"); + return AWS_OP_ERR; + } + } + if (aws_byte_buf_append_dynamic(¤t_block->cookies, &header_field->value)) { + DECODER_LOG(ERROR, decoder, "Store the current cookie value to buffer failed"); + DECODER_LOGF( + DEBUG, decoder, "Value of cookie is '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(header_field->value)); + return AWS_OP_ERR; + } + return AWS_OP_SUCCESS; + } + /* Deliver header-field via callback */ if (current_block->is_push_promise) { DECODER_CALL_VTABLE_STREAM_ARGS(decoder, on_push_promise_i, header_field, name_enum); @@ -1289,6 +1326,22 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a return AWS_OP_ERR; } + if (decoder->header_block_in_progress.cookies.len) { + /* before we end the header block, we still have cookies finished concatenating and the callback needs + * to be fired */ + struct aws_http_header concatenated_cookie; + concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); + concatenated_cookie.value = aws_byte_cursor_from_buf(&decoder->header_block_in_progress.cookies); + struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; + if (current_block->is_push_promise) { + DECODER_CALL_VTABLE_STREAM_ARGS( + decoder, on_push_promise_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE); + } else { + DECODER_CALL_VTABLE_STREAM_ARGS( + decoder, on_headers_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE, current_block->block_type); + } + } + bool malformed = decoder->header_block_in_progress.malformed; DECODER_LOGF(TRACE, decoder, "Done decoding header-block, malformed=%d", malformed); @@ -1380,8 +1433,6 @@ static int s_state_fn_header_block_entry(struct aws_h2_decoder *decoder, struct * If dynamic table size changed via SETTINGS frame, next header-block must start with DYNAMIC_TABLE_RESIZE entry. * Is it illegal to receive a resize entry at other times? */ - /* #TODO Cookie headers must be concatenated into single delivery RFC-7540 8.1.2.5 */ - /* #TODO The TE header field ... MUST NOT contain any value other than "trailers" */ if (result.type == AWS_HPACK_DECODE_T_HEADER_FIELD) { diff --git a/source/http.c b/source/http.c index d378de2b2..01b0d6bc7 100644 --- a/source/http.c +++ b/source/http.c @@ -253,6 +253,7 @@ static void s_headers_init(struct aws_allocator *alloc) { s_header_enum_to_str[AWS_HTTP_HEADER_AUTHORITY] = aws_byte_cursor_from_c_str(":authority"); s_header_enum_to_str[AWS_HTTP_HEADER_PATH] = aws_byte_cursor_from_c_str(":path"); s_header_enum_to_str[AWS_HTTP_HEADER_STATUS] = aws_byte_cursor_from_c_str(":status"); + s_header_enum_to_str[AWS_HTTP_HEADER_COOKIE] = aws_byte_cursor_from_c_str("cookie"); s_header_enum_to_str[AWS_HTTP_HEADER_CONNECTION] = aws_byte_cursor_from_c_str("connection"); s_header_enum_to_str[AWS_HTTP_HEADER_CONTENT_LENGTH] = aws_byte_cursor_from_c_str("content-length"); s_header_enum_to_str[AWS_HTTP_HEADER_EXPECT] = aws_byte_cursor_from_c_str("expect"); From 509573e687499f6855d3f3a9ca66be632a3ba94d Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Fri, 3 Apr 2020 14:49:45 -0700 Subject: [PATCH 2/9] Compression type for concatenated cookie&unit test --- source/h2_decoder.c | 11 +++++++++-- tests/CMakeLists.txt | 1 + tests/test_h2_decoder.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/source/h2_decoder.c b/source/h2_decoder.c index cd81d9d27..79ccb35eb 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -251,7 +251,9 @@ struct aws_h2_decoder { /* Buffer up cookie header fields to concatenate separate ones */ struct aws_byte_buf cookies; - + /* Compression type for the concatenated cookie header. Let's say if any separate one is not cached, the last + * one will not be cached */ + enum aws_http_header_compression cookie_header_compression_type; } header_block_in_progress; /* Settings for decoder, which is based on the settings sent to the peer and ACKed by peer */ @@ -343,7 +345,7 @@ static void s_reset_header_block_in_progress(struct aws_h2_decoder *decoder) { struct aws_byte_buf cookie_backup = decoder->header_block_in_progress.cookies; AWS_ZERO_STRUCT(decoder->header_block_in_progress); decoder->header_block_in_progress.cookies = cookie_backup; - aws_byte_buf_reset(&decoder->header_block_in_progress.cookies); + aws_byte_buf_reset(&decoder->header_block_in_progress.cookies, false); } void aws_h2_decoder_destroy(struct aws_h2_decoder *decoder) { @@ -1283,6 +1285,10 @@ static int s_process_header_field(struct aws_h2_decoder *decoder, const struct a if (name_enum == AWS_HTTP_HEADER_COOKIE) { /* for a header cookie, we will not fire callback until we concatenate them all, let's store it at the * buffer */ + if (header_field->compression > current_block->cookie_header_compression_type) { + current_block->cookie_header_compression_type = header_field->compression; + } + if (current_block->cookies.len) { /* add a delimiter */ struct aws_byte_cursor delimiter = aws_byte_cursor_from_c_str("; "); @@ -1342,6 +1348,7 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); concatenated_cookie.value = aws_byte_cursor_from_buf(&decoder->header_block_in_progress.cookies); struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; + concatenated_cookie.compression = current_block->cookie_header_compression_type; if (current_block->is_push_promise) { DECODER_CALL_VTABLE_STREAM_ARGS( decoder, on_push_promise_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0e185aaa8..a0ba2fa4b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -239,6 +239,7 @@ add_h2_decoder_test_set(h2_decoder_err_data_requires_stream_id) add_h2_decoder_test_set(h2_decoder_err_payload_too_small_for_pad_length) add_h2_decoder_test_set(h2_decoder_stream_id_ignores_reserved_bit) add_h2_decoder_test_set(h2_decoder_headers) +add_h2_decoder_test_set(h2_decoder_headers_cookies) add_h2_decoder_test_set(h2_decoder_headers_padded) add_h2_decoder_test_set(h2_decoder_headers_priority) add_h2_decoder_test_set(h2_decoder_headers_ignores_unknown_flags) diff --git a/tests/test_h2_decoder.c b/tests/test_h2_decoder.c index 8d3761446..31d683b56 100644 --- a/tests/test_h2_decoder.c +++ b/tests/test_h2_decoder.c @@ -413,6 +413,45 @@ H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers) { return AWS_OP_SUCCESS; } +H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers_cookies) { + (void)allocator; + struct fixture *fixture = ctx; + + /* clang-format off */ + uint8_t input[] = { + /* HEADERS FRAME*/ + 0x00, 0x00, 0x05, /* Length (24) */ + AWS_H2_FRAME_T_HEADERS, /* Type (8) */ + AWS_H2_FRAME_F_END_STREAM, /* Flags (8) */ + 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ + /* PAYLOAD */ + 0x60, 0x03, 'a', '=', 'b', /* "cache: a=b" - indexed name, uncompressed value */ + + /* CONTINUATION FRAME*/ + 0x00, 0x00, 0x05, /* Length (24) */ + AWS_H2_FRAME_T_CONTINUATION,/* Type (8) */ + AWS_H2_FRAME_F_END_HEADERS, /* Flags (8) */ + 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ + /* PAYLOAD */ + 0x60, 0x03, 'c', '=', 'd', /* "cache: c=d" - indexed name, uncompressed value */ + }; + /* clang-format on */ + + /* Decode */ + ASSERT_SUCCESS(s_decode_all(fixture, aws_byte_cursor_from_array(input, sizeof(input)))); + + /* Validate */ + struct h2_decoded_frame *frame = h2_decode_tester_latest_frame(&fixture->decode); + ASSERT_SUCCESS(h2_decoded_frame_check_finished(frame, AWS_H2_FRAME_T_HEADERS, 0x76543210 /*stream_id*/)); + ASSERT_FALSE(frame->headers_malformed); + ASSERT_UINT_EQUALS(1, aws_http_headers_count(frame->headers)); + ASSERT_SUCCESS(s_check_header(frame, 0, "cookie", "a=b; c=d", AWS_HTTP_HEADER_COMPRESSION_USE_CACHE)); + ASSERT_INT_EQUALS(AWS_HTTP_HEADER_BLOCK_TRAILING, frame->header_block_type); + ASSERT_TRUE(frame->end_stream); + + return AWS_OP_SUCCESS; +} + /* Test a HEADERS frame with padding */ H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers_padded) { (void)allocator; From f74b11ba8a05d68cf965b7d49fb23b007c72cf1b Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Fri, 3 Apr 2020 15:30:05 -0700 Subject: [PATCH 3/9] update the unit test --- tests/test_h2_decoder.c | 85 ++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/tests/test_h2_decoder.c b/tests/test_h2_decoder.c index 31d683b56..ebe5e1322 100644 --- a/tests/test_h2_decoder.c +++ b/tests/test_h2_decoder.c @@ -413,45 +413,6 @@ H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers) { return AWS_OP_SUCCESS; } -H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers_cookies) { - (void)allocator; - struct fixture *fixture = ctx; - - /* clang-format off */ - uint8_t input[] = { - /* HEADERS FRAME*/ - 0x00, 0x00, 0x05, /* Length (24) */ - AWS_H2_FRAME_T_HEADERS, /* Type (8) */ - AWS_H2_FRAME_F_END_STREAM, /* Flags (8) */ - 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ - /* PAYLOAD */ - 0x60, 0x03, 'a', '=', 'b', /* "cache: a=b" - indexed name, uncompressed value */ - - /* CONTINUATION FRAME*/ - 0x00, 0x00, 0x05, /* Length (24) */ - AWS_H2_FRAME_T_CONTINUATION,/* Type (8) */ - AWS_H2_FRAME_F_END_HEADERS, /* Flags (8) */ - 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ - /* PAYLOAD */ - 0x60, 0x03, 'c', '=', 'd', /* "cache: c=d" - indexed name, uncompressed value */ - }; - /* clang-format on */ - - /* Decode */ - ASSERT_SUCCESS(s_decode_all(fixture, aws_byte_cursor_from_array(input, sizeof(input)))); - - /* Validate */ - struct h2_decoded_frame *frame = h2_decode_tester_latest_frame(&fixture->decode); - ASSERT_SUCCESS(h2_decoded_frame_check_finished(frame, AWS_H2_FRAME_T_HEADERS, 0x76543210 /*stream_id*/)); - ASSERT_FALSE(frame->headers_malformed); - ASSERT_UINT_EQUALS(1, aws_http_headers_count(frame->headers)); - ASSERT_SUCCESS(s_check_header(frame, 0, "cookie", "a=b; c=d", AWS_HTTP_HEADER_COMPRESSION_USE_CACHE)); - ASSERT_INT_EQUALS(AWS_HTTP_HEADER_BLOCK_TRAILING, frame->header_block_type); - ASSERT_TRUE(frame->end_stream); - - return AWS_OP_SUCCESS; -} - /* Test a HEADERS frame with padding */ H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers_padded) { (void)allocator; @@ -618,6 +579,52 @@ H2_DECODER_ON_SERVER_TEST(h2_decoder_headers_request) { return AWS_OP_SUCCESS; } + +H2_DECODER_ON_SERVER_TEST(h2_decoder_headers_cookies) { + (void)allocator; + struct fixture *fixture = ctx; + + /* clang-format off */ + uint8_t input[] = { + /* HEADERS FRAME*/ + 0x00, 0x00, 0x06, /* Length (24) */ + AWS_H2_FRAME_T_HEADERS, /* Type (8) */ + AWS_H2_FRAME_F_END_STREAM, /* Flags (8) */ + 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ + /* HEADERS */ + 0x82, /* ":method: GET" - indexed */ + 0x60, 0x03, 'a', '=', 'b', /* "cache: a=b" - indexed name, uncompressed value */ + + /* CONTINUATION FRAME*/ + 0x00, 0x00, 16, /* Length (24) */ + AWS_H2_FRAME_T_CONTINUATION,/* Type (8) */ + AWS_H2_FRAME_F_END_HEADERS, /* Flags (8) */ + 0x76, 0x54, 0x32, 0x10, /* Reserved (1) | Stream Identifier (31) */ + /* PAYLOAD */ + 0x7a, 0x04, 't', 'e', 's', 't', /* "user-agent: test" - indexed name, uncompressed value */ + 0x60, 0x03, 'c', '=', 'd', /* "cache: c=d" - indexed name, uncompressed value */ + 0x60, 0x03, 'e', '=', 'f', /* "cache: e=f" - indexed name, uncompressed value */ + }; + /* clang-format on */ + + /* Decode */ + ASSERT_SUCCESS(s_decode_all(fixture, aws_byte_cursor_from_array(input, sizeof(input)))); + + /* Validate */ + struct h2_decoded_frame *frame = h2_decode_tester_latest_frame(&fixture->decode); + ASSERT_SUCCESS(h2_decoded_frame_check_finished(frame, AWS_H2_FRAME_T_HEADERS, 0x76543210 /*stream_id*/)); + ASSERT_FALSE(frame->headers_malformed); + /* two sepaprate cookie headers are concatenated and moved as the last header*/ + ASSERT_UINT_EQUALS(3, aws_http_headers_count(frame->headers)); + ASSERT_SUCCESS(s_check_header(frame, 0, ":method", "GET", AWS_HTTP_HEADER_COMPRESSION_USE_CACHE)); + ASSERT_SUCCESS(s_check_header(frame, 1, "user-agent", "test", AWS_HTTP_HEADER_COMPRESSION_USE_CACHE)); + ASSERT_SUCCESS(s_check_header(frame, 2, "cookie", "a=b; c=d; e=f", AWS_HTTP_HEADER_COMPRESSION_USE_CACHE)); + ASSERT_INT_EQUALS(AWS_HTTP_HEADER_BLOCK_MAIN, frame->header_block_type); + ASSERT_TRUE(frame->end_stream); + + return AWS_OP_SUCCESS; +} + /* A trailing header has no psuedo-headers, and always ends the stream */ H2_DECODER_ON_CLIENT_TEST(h2_decoder_headers_trailer) { (void)allocator; From 2e4d8ec3a23755dc056486e1008083a64bc78c45 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Fri, 3 Apr 2020 15:32:26 -0700 Subject: [PATCH 4/9] order of makelist update --- tests/CMakeLists.txt | 2 +- tests/test_h2_decoder.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a0ba2fa4b..ecbe699e9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -239,12 +239,12 @@ add_h2_decoder_test_set(h2_decoder_err_data_requires_stream_id) add_h2_decoder_test_set(h2_decoder_err_payload_too_small_for_pad_length) add_h2_decoder_test_set(h2_decoder_stream_id_ignores_reserved_bit) add_h2_decoder_test_set(h2_decoder_headers) -add_h2_decoder_test_set(h2_decoder_headers_cookies) add_h2_decoder_test_set(h2_decoder_headers_padded) add_h2_decoder_test_set(h2_decoder_headers_priority) add_h2_decoder_test_set(h2_decoder_headers_ignores_unknown_flags) add_h2_decoder_test_set(h2_decoder_headers_response_informational) add_h2_decoder_test_set(h2_decoder_headers_request) +add_h2_decoder_test_set(h2_decoder_headers_cookies) add_h2_decoder_test_set(h2_decoder_headers_trailer) add_h2_decoder_test_set(h2_decoder_headers_empty_trailer) add_h2_decoder_test_set(h2_decoder_err_headers_requires_stream_id) diff --git a/tests/test_h2_decoder.c b/tests/test_h2_decoder.c index ebe5e1322..11320aff1 100644 --- a/tests/test_h2_decoder.c +++ b/tests/test_h2_decoder.c @@ -579,7 +579,6 @@ H2_DECODER_ON_SERVER_TEST(h2_decoder_headers_request) { return AWS_OP_SUCCESS; } - H2_DECODER_ON_SERVER_TEST(h2_decoder_headers_cookies) { (void)allocator; struct fixture *fixture = ctx; From 86b5942d05c4fed13ffb786e2461dfcefa59cbf7 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Mon, 6 Apr 2020 15:57:06 -0700 Subject: [PATCH 5/9] Update based on comments --- include/aws/http/private/http_impl.h | 4 +- source/h2_decoder.c | 95 +++++++++++++++------------- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/include/aws/http/private/http_impl.h b/include/aws/http/private/http_impl.h index e113abd7b..ecdabe179 100644 --- a/include/aws/http/private/http_impl.h +++ b/include/aws/http/private/http_impl.h @@ -46,14 +46,12 @@ enum aws_http_header_name { /* Response pseudo-headers */ AWS_HTTP_HEADER_STATUS, - /* Cookie header */ - AWS_HTTP_HEADER_COOKIE, - /* Regular headers */ AWS_HTTP_HEADER_CONNECTION, AWS_HTTP_HEADER_CONTENT_LENGTH, AWS_HTTP_HEADER_EXPECT, AWS_HTTP_HEADER_TRANSFER_ENCODING, + AWS_HTTP_HEADER_COOKIE, AWS_HTTP_HEADER_COUNT, /* Number of enums */ }; diff --git a/source/h2_decoder.c b/source/h2_decoder.c index 79ccb35eb..b8d1f0c8b 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -251,8 +251,8 @@ struct aws_h2_decoder { /* Buffer up cookie header fields to concatenate separate ones */ struct aws_byte_buf cookies; - /* Compression type for the concatenated cookie header. Let's say if any separate one is not cached, the last - * one will not be cached */ + /* If separate cookie fields have different compression types, the concatenated cookie uses the strictest type. + */ enum aws_http_header_compression cookie_header_compression_type; } header_block_in_progress; @@ -289,7 +289,7 @@ struct aws_h2_decoder *aws_h2_decoder_new(struct aws_h2_decoder_params *params) void *allocation = aws_mem_acquire_many( params->alloc, 2, &decoder, sizeof(struct aws_h2_decoder), &scratch_buf, s_scratch_space_size); if (!allocation) { - goto failed_alloc; + goto error; } AWS_ZERO_STRUCT(*decoder); @@ -304,7 +304,7 @@ struct aws_h2_decoder *aws_h2_decoder_new(struct aws_h2_decoder_params *params) decoder->hpack = aws_hpack_context_new(params->alloc, AWS_LS_HTTP_DECODER, decoder); if (!decoder->hpack) { - goto failed_new_hpack; + goto error; } if (decoder->is_server && !params->skip_connection_preface) { @@ -320,21 +320,23 @@ struct aws_h2_decoder *aws_h2_decoder_new(struct aws_h2_decoder_params *params) if (aws_array_list_init_dynamic( &decoder->settings_buffer_list, decoder->alloc, 0, sizeof(struct aws_h2_frame_setting))) { - goto array_list_failed; + goto error; } if (aws_byte_buf_init( &decoder->header_block_in_progress.cookies, decoder->alloc, s_decoder_cookie_buffer_initial_size)) { - goto buffer_init_failed; + goto error; } return decoder; -buffer_init_failed: -array_list_failed: -failed_new_hpack: +error: + if (decoder) { + aws_hpack_context_destroy(decoder->hpack); + aws_array_list_clean_up(&decoder->settings_buffer_list); + aws_byte_buf_clean_up(&decoder->header_block_in_progress.cookies); + } aws_mem_release(params->alloc, allocation); -failed_alloc: return NULL; } @@ -1093,10 +1095,6 @@ static int s_state_fn_frame_unknown(struct aws_h2_decoder *decoder, struct aws_b static int s_flush_pseudoheaders(struct aws_h2_decoder *decoder) { struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; - if (current_block->malformed) { - goto already_malformed; - } - if (current_block->pseudoheaders_done) { return AWS_OP_SUCCESS; } @@ -1189,8 +1187,6 @@ static int s_flush_pseudoheaders(struct aws_h2_decoder *decoder) { * We continue decoding and report that it's malformed in on_headers_end(). */ current_block->malformed = true; return AWS_OP_SUCCESS; -already_malformed: - return AWS_OP_SUCCESS; } /* Process single header-field. @@ -1282,35 +1278,35 @@ static int s_process_header_field(struct aws_h2_decoder *decoder, const struct a /* #TODO Validate characters used in header_field->value */ - if (name_enum == AWS_HTTP_HEADER_COOKIE) { - /* for a header cookie, we will not fire callback until we concatenate them all, let's store it at the - * buffer */ - if (header_field->compression > current_block->cookie_header_compression_type) { - current_block->cookie_header_compression_type = header_field->compression; - } + switch (name_enum) { + case AWS_HTTP_HEADER_COOKIE: + /* for a header cookie, we will not fire callback until we concatenate them all, let's store it at the + * buffer */ + if (header_field->compression > current_block->cookie_header_compression_type) { + current_block->cookie_header_compression_type = header_field->compression; + } - if (current_block->cookies.len) { - /* add a delimiter */ - struct aws_byte_cursor delimiter = aws_byte_cursor_from_c_str("; "); - if (aws_byte_buf_append_dynamic(¤t_block->cookies, &delimiter)) { - DECODER_LOG(ERROR, decoder, "Store cookie delimiter to buffer failed"); + if (current_block->cookies.len) { + /* add a delimiter */ + struct aws_byte_cursor delimiter = aws_byte_cursor_from_c_str("; "); + if (aws_byte_buf_append_dynamic(¤t_block->cookies, &delimiter)) { + return AWS_OP_ERR; + } + } + if (aws_byte_buf_append_dynamic(¤t_block->cookies, &header_field->value)) { return AWS_OP_ERR; } - } - if (aws_byte_buf_append_dynamic(¤t_block->cookies, &header_field->value)) { - DECODER_LOG(ERROR, decoder, "Store the current cookie value to buffer failed"); - DECODER_LOGF( - DEBUG, decoder, "Value of cookie is '" PRInSTR "'", AWS_BYTE_CURSOR_PRI(header_field->value)); - return AWS_OP_ERR; - } - return AWS_OP_SUCCESS; - } + break; - /* Deliver header-field via callback */ - if (current_block->is_push_promise) { - DECODER_CALL_VTABLE_STREAM_ARGS(decoder, on_push_promise_i, header_field, name_enum); - } else { - DECODER_CALL_VTABLE_STREAM_ARGS(decoder, on_headers_i, header_field, name_enum, current_block->block_type); + default: + /* Deliver header-field via callback */ + if (current_block->is_push_promise) { + DECODER_CALL_VTABLE_STREAM_ARGS(decoder, on_push_promise_i, header_field, name_enum); + } else { + DECODER_CALL_VTABLE_STREAM_ARGS( + decoder, on_headers_i, header_field, name_enum, current_block->block_type); + } + break; } } @@ -1336,14 +1332,22 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a /* If this is the end of the header-block, invoke callback and clear header_block_in_progress */ if (decoder->frame_in_progress.flags.end_headers) { + + bool malformed = decoder->header_block_in_progress.malformed; + if (malformed) { + goto already_malformed; + } /* Ensure pseudo-headers have been flushed */ if (s_flush_pseudoheaders(decoder)) { return AWS_OP_ERR; } - + malformed = decoder->header_block_in_progress.malformed; + /* might have realized that header-block is malformed during flush */ + if (malformed) { + goto already_malformed; + } if (decoder->header_block_in_progress.cookies.len) { - /* before we end the header block, we still have cookies finished concatenating and the callback needs - * to be fired */ + /* Flush the cookie header */ struct aws_http_header concatenated_cookie; concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); concatenated_cookie.value = aws_byte_cursor_from_buf(&decoder->header_block_in_progress.cookies); @@ -1358,7 +1362,8 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a } } - bool malformed = decoder->header_block_in_progress.malformed; + /* if the headers are malformed, we skip flushing the cookie and pseudoheaders */ + already_malformed: DECODER_LOGF(TRACE, decoder, "Done decoding header-block, malformed=%d", malformed); if (decoder->header_block_in_progress.is_push_promise) { From 909db2427090f6f1c8cbea383296a585dc7abba0 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Mon, 6 Apr 2020 16:44:07 -0700 Subject: [PATCH 6/9] function to flush cookie header --- source/h2_decoder.c | 58 +++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/source/h2_decoder.c b/source/h2_decoder.c index b8d1f0c8b..c3b62620d 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -1095,6 +1095,10 @@ static int s_state_fn_frame_unknown(struct aws_h2_decoder *decoder, struct aws_b static int s_flush_pseudoheaders(struct aws_h2_decoder *decoder) { struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; + if (current_block->malformed) { + goto already_malformed; + } + if (current_block->pseudoheaders_done) { return AWS_OP_SUCCESS; } @@ -1187,6 +1191,8 @@ static int s_flush_pseudoheaders(struct aws_h2_decoder *decoder) { * We continue decoding and report that it's malformed in on_headers_end(). */ current_block->malformed = true; return AWS_OP_SUCCESS; +already_malformed: + return AWS_OP_SUCCESS; } /* Process single header-field. @@ -1321,6 +1327,28 @@ static int s_process_header_field(struct aws_h2_decoder *decoder, const struct a return AWS_OP_SUCCESS; } +static int s_flush_cookie_header(struct aws_h2_decoder *decoder) { + struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; + if (current_block->malformed) { + return AWS_OP_SUCCESS; + } + if (current_block->cookies.len == 0) { + /* Nothing to flush */ + return AWS_OP_SUCCESS; + } + struct aws_http_header concatenated_cookie; + concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); + concatenated_cookie.value = aws_byte_cursor_from_buf(¤t_block->cookies); + concatenated_cookie.compression = current_block->cookie_header_compression_type; + if (current_block->is_push_promise) { + DECODER_CALL_VTABLE_STREAM_ARGS(decoder, on_push_promise_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE); + } else { + DECODER_CALL_VTABLE_STREAM_ARGS( + decoder, on_headers_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE, current_block->block_type); + } + return AWS_OP_SUCCESS; +} + /* This state checks whether we've consumed the current frame's entire header-block fragment. * We revisit this state after each entry is decoded. * This state consumes no data. */ @@ -1332,38 +1360,16 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a /* If this is the end of the header-block, invoke callback and clear header_block_in_progress */ if (decoder->frame_in_progress.flags.end_headers) { - - bool malformed = decoder->header_block_in_progress.malformed; - if (malformed) { - goto already_malformed; - } /* Ensure pseudo-headers have been flushed */ if (s_flush_pseudoheaders(decoder)) { return AWS_OP_ERR; } - malformed = decoder->header_block_in_progress.malformed; - /* might have realized that header-block is malformed during flush */ - if (malformed) { - goto already_malformed; - } - if (decoder->header_block_in_progress.cookies.len) { - /* Flush the cookie header */ - struct aws_http_header concatenated_cookie; - concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); - concatenated_cookie.value = aws_byte_cursor_from_buf(&decoder->header_block_in_progress.cookies); - struct aws_header_block_in_progress *current_block = &decoder->header_block_in_progress; - concatenated_cookie.compression = current_block->cookie_header_compression_type; - if (current_block->is_push_promise) { - DECODER_CALL_VTABLE_STREAM_ARGS( - decoder, on_push_promise_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE); - } else { - DECODER_CALL_VTABLE_STREAM_ARGS( - decoder, on_headers_i, &concatenated_cookie, AWS_HTTP_HEADER_COOKIE, current_block->block_type); - } + /* flush the concatenated cookie header */ + if(s_flush_cookie_header(decoder)) { + return AWS_OP_ERR; } - /* if the headers are malformed, we skip flushing the cookie and pseudoheaders */ - already_malformed: + bool malformed = decoder->header_block_in_progress.malformed; DECODER_LOGF(TRACE, decoder, "Done decoding header-block, malformed=%d", malformed); if (decoder->header_block_in_progress.is_push_promise) { From 6c01cb57c25e198d50f2227a94d32255d612c9dc Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Mon, 6 Apr 2020 16:45:27 -0700 Subject: [PATCH 7/9] clang format --- source/h2_decoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/h2_decoder.c b/source/h2_decoder.c index c3b62620d..ab3cd4430 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -1365,7 +1365,7 @@ static int s_state_fn_header_block_loop(struct aws_h2_decoder *decoder, struct a return AWS_OP_ERR; } /* flush the concatenated cookie header */ - if(s_flush_cookie_header(decoder)) { + if (s_flush_cookie_header(decoder)) { return AWS_OP_ERR; } From cb89db5dbff759e8cb1eee01901db2eed6dccd07 Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Mon, 6 Apr 2020 17:06:49 -0700 Subject: [PATCH 8/9] aws_byte_cursor_from_c_str -> AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL --- source/h2_decoder.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/h2_decoder.c b/source/h2_decoder.c index ab3cd4430..e4e76f075 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -1294,7 +1294,7 @@ static int s_process_header_field(struct aws_h2_decoder *decoder, const struct a if (current_block->cookies.len) { /* add a delimiter */ - struct aws_byte_cursor delimiter = aws_byte_cursor_from_c_str("; "); + struct aws_byte_cursor delimiter = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("; "); if (aws_byte_buf_append_dynamic(¤t_block->cookies, &delimiter)) { return AWS_OP_ERR; } @@ -1337,7 +1337,8 @@ static int s_flush_cookie_header(struct aws_h2_decoder *decoder) { return AWS_OP_SUCCESS; } struct aws_http_header concatenated_cookie; - concatenated_cookie.name = aws_byte_cursor_from_c_str("cookie"); + struct aws_byte_cursor cookie_name = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("cookie"); + concatenated_cookie.name = cookie_name; concatenated_cookie.value = aws_byte_cursor_from_buf(¤t_block->cookies); concatenated_cookie.compression = current_block->cookie_header_compression_type; if (current_block->is_push_promise) { From c7eeb6c2a73fa0848370050aaca8f4eb074f1d5c Mon Sep 17 00:00:00 2001 From: Dengke Tang Date: Mon, 6 Apr 2020 17:08:10 -0700 Subject: [PATCH 9/9] trivial change of naming --- source/h2_decoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/h2_decoder.c b/source/h2_decoder.c index e4e76f075..106318842 100644 --- a/source/h2_decoder.c +++ b/source/h2_decoder.c @@ -1337,8 +1337,8 @@ static int s_flush_cookie_header(struct aws_h2_decoder *decoder) { return AWS_OP_SUCCESS; } struct aws_http_header concatenated_cookie; - struct aws_byte_cursor cookie_name = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("cookie"); - concatenated_cookie.name = cookie_name; + struct aws_byte_cursor header_name = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("cookie"); + concatenated_cookie.name = header_name; concatenated_cookie.value = aws_byte_cursor_from_buf(¤t_block->cookies); concatenated_cookie.compression = current_block->cookie_header_compression_type; if (current_block->is_push_promise) {