Ping frame for H2 (#205)

* revamp encoder API (wip)

* hpack pre-encode cmd.
Do encoding in 2 passes, so we know exactly how much buffer space we'll need. I will probably revert this. I thought this was clever, but seeing it implemented it's just so much more complicated than writing everything to a dynamic buffer, then copying it into the aws_io_mesage.

* simplify decoder state machine loop

* hpack encoding expands output buffer
ditch 2 pass approach, it was just too complicated

* Revamp HEADERS/PUSH_PROMISE/DATA frames

* all tests passing again

* Exposed options for toggling read back pressure behavior, updated to match aws-c-io api changes. (#194)

* Fix bug when new request has same memory address as old request. (#195)

As seen in real life

* Enabled compilation on VS 2015 (#196)

* Enabled compilation on VS 2015

* Fix VS narrowing warning

* Updated to v0.5.3 of builder

* Fix proxies connect and tests (#198)

Ignore connection: close on 200/OK responses to a CONNECT Request, since the proxy is obviously drunk and needs to hail an uber to get home from the bar safely.

Fix the broken tests from the tcp back pressure refactor in aws-c-io.

* Fix fuzz tests

* clang-format

* tweaks

* tweaks

* Simplify how most frames are encoded:
- Use common struct
- Pre-encode the entire frame
- Incrementally copy that to aws_io_message whenever encode() is called.

This is simpler/better because:
1) more shared code
2) unique payload-writing code all goes in the one new() function, instead of being spread across the new() and encode() functions
3) less chance of incorrect size calculations, since we're encoding to a buffer of the exact correct length

* THANK YOU MSVC COMPILER WARNING YOU SAVED MY ASS

* replaced lots of AWS_ERROR_SHORT_BUFFER error-handling with asserts. If this kind of error happens now, it's programmer error

* compiler warning

* insert ping ACK to the front of the queue

* auto send ping ack frame back

* h2_test_helper.h
Extract useful machinery from test_h2_decoder.c for use with other tests.

* merge with master

* test_h2_client.c is using fake peer

* test_for_ping_ack

* more h2_fake_peer functionality

* msvc warning

* delete the message

* prototype function

* tiny changes for making the code clear

* clang format

* Stop just the reading direction.

* error check&udpate read window

* move comments

* remove redundant connection id and style change

* LOGF->LOG

Co-authored-by: Michael Graeb <graebm@amazon.com>
Co-authored-by: Jonathan M. Henson <henso@amazon.com>
Co-authored-by: Justin Boswell <boswej@amazon.com>
Co-authored-by: Dengke Tang <dengket@amazon.com>

Author: 4 people

include/aws/http/private/h2_frames.h

@@ -115,6 +115,10 @@ struct aws_h2_frame {
     struct aws_linked_list_node node;
     enum aws_h2_frame_type type;
     uint32_t stream_id;
+
+    /* If true, frame will be sent before those with normal priority.
+     * Useful for frames like PING ACK where low latency is important. */
+    bool high_priority;
 };
 
 /* A h2 setting and its value, used in SETTINGS frame */

source/h2_connection.c

@@ -61,6 +61,8 @@ static struct aws_http_stream *s_connection_make_request(
 static void s_cross_thread_work_task(struct aws_channel_task *task, void *arg, enum aws_task_status status);
 static void s_outgoing_frames_task(struct aws_channel_task *task, void *arg, enum aws_task_status status);
 
+static int s_decoder_on_ping(uint8_t opaque_data[AWS_H2_PING_DATA_SIZE], void *userdata);
+
 static struct aws_http_connection_vtable s_h2_connection_vtable = {
     .channel_handler_vtable =
         {
@@ -84,6 +86,7 @@ static struct aws_http_connection_vtable s_h2_connection_vtable = {
 
 static const struct aws_h2_decoder_vtable s_h2_decoder_vtable = {
     .on_data = NULL,
+    .on_ping = s_decoder_on_ping,
 };
 
 static void s_lock_synced_data(struct aws_h2_connection *connection) {
@@ -294,7 +297,22 @@ void aws_h2_connection_enqueue_outgoing_frame(struct aws_h2_connection *connecti
     AWS_PRECONDITION(frame->type != AWS_H2_FRAME_T_DATA);
     AWS_PRECONDITION(aws_channel_thread_is_callers_thread(connection->base.channel_slot->channel));
 
-    aws_linked_list_push_back(&connection->thread_data.outgoing_frames_queue, &frame->node);
+    if (frame->high_priority) {
+        /* Check from the head of the queue, and find a node with normal priority, and insert before it */
+        struct aws_linked_list_node *iter = aws_linked_list_begin(&connection->thread_data.outgoing_frames_queue);
+        /* one past the last element */
+        const struct aws_linked_list_node *end = aws_linked_list_end(&connection->thread_data.outgoing_frames_queue);
+        while (iter != end) {
+            struct aws_h2_frame *frame_i = AWS_CONTAINER_OF(iter, struct aws_h2_frame, node);
+            if (!frame_i->high_priority) {
+                break;
+            }
+            iter = iter->next;
+        }
+        aws_linked_list_insert_before(iter, &frame->node);
+    } else {
+        aws_linked_list_push_back(&connection->thread_data.outgoing_frames_queue, &frame->node);
+    }
 }
 
 static void s_on_channel_write_complete(
@@ -492,6 +510,24 @@ static void s_try_write_outgoing_frames(struct aws_h2_connection *connection) {
     s_outgoing_frames_task(&connection->outgoing_frames_task, connection, AWS_TASK_STATUS_RUN_READY);
 }
 
+/* Decoder callbacks */
+static int s_decoder_on_ping(uint8_t opaque_data[AWS_H2_PING_DATA_SIZE], void *userdata) {
+    struct aws_h2_connection *connection = userdata;
+
+    /* send a PING frame with the ACK flag set in response, with an identical payload. */
+    struct aws_h2_frame *ping_ack_frame = aws_h2_frame_new_ping(connection->base.alloc, true, opaque_data);
+    if (!ping_ack_frame) {
+        goto error;
+    }
+
+    aws_h2_connection_enqueue_outgoing_frame(connection, ping_ack_frame);
+    s_try_write_outgoing_frames(connection);
+    return AWS_OP_SUCCESS;
+error:
+    CONNECTION_LOGF(ERROR, connection, "Ping ACK frame failed to be sent, error %s", aws_error_name(aws_last_error()));
+    return AWS_OP_ERR;
+}
+
 static int s_send_connection_preface_client_string(struct aws_h2_connection *connection) {
 
     /* Just send the magic string on its own aws_io_message. */
@@ -735,16 +771,51 @@ static int s_handler_process_read_message(
     struct aws_channel_handler *handler,
     struct aws_channel_slot *slot,
     struct aws_io_message *message) {
-
-    (void)handler;
     (void)slot;
-    (void)message;
+    struct aws_h2_connection *connection = handler->impl;
+
+    CONNECTION_LOGF(TRACE, connection, "Begin processing message of size %zu.", message->message_data.len);
+
+    if (connection->thread_data.is_reading_stopped) {
+        CONNECTION_LOG(ERROR, connection, "Cannot process message because connection is shutting down.");
+        aws_raise_error(AWS_ERROR_HTTP_CONNECTION_CLOSED);
+        goto shutdown;
+    }
+
+    struct aws_byte_cursor message_cursor = aws_byte_cursor_from_buf(&message->message_data);
+    if (aws_h2_decode(connection->thread_data.decoder, &message_cursor)) {
+        CONNECTION_LOGF(
+            ERROR,
+            connection,
+            "Decoding message failed, error %d (%s). Closing connection",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+    }
 
     /* HTTP/2 protocol uses WINDOW_UPDATE frames to coordinate data rates with peer,
      * so we can just keep the aws_channel's read-window wide open */
-    /* #TODO update read window by however much we just read */
+    if (aws_channel_slot_increment_read_window(slot, message->message_data.len)) {
+        CONNECTION_LOGF(
+            ERROR,
+            connection,
+            "Incrementing read window failed, error %d (%s). Closing connection",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+    }
 
-    return aws_raise_error(AWS_ERROR_UNIMPLEMENTED);
+    /* release message */
+    if (message) {
+        aws_mem_release(message->allocator, message);
+        message = NULL;
+    }
+    return AWS_OP_SUCCESS;
+shutdown:
+    if (message) {
+        aws_mem_release(message->allocator, message);
+    }
+    /* Stop reading, because the reading error happans here */
+    s_stop(connection, true /*stop_reading*/, false /*stop_writing*/, true /*schedule_shutdown*/, aws_last_error());
+    return AWS_OP_SUCCESS;
 }
 
 static int s_handler_process_write_message(

source/h2_frames.c

@@ -985,6 +985,8 @@ struct aws_h2_frame *aws_h2_frame_new_ping(
     writes_ok &= aws_byte_buf_write(&frame->encoded_buf, opaque_data, AWS_H2_PING_DATA_SIZE);
     AWS_ASSERT(writes_ok);
 
+    /* PING responses SHOULD be given higher priority than any other frame */
+    frame->base.high_priority = ack;
     return &frame->base;
 }
 

tests/CMakeLists.txt

@@ -300,6 +300,7 @@ add_h2_decoder_test_set(h2_decoder_err_bad_preface_from_client_3)
 add_test_case(h2_client_sanity_check)
 add_test_case(h2_client_request_create)
 add_test_case(h2_client_connection_preface_sent)
+add_test_case(h2_client_ping_ack)
 
 add_test_case(server_new_destroy)
 add_test_case(connection_setup_shutdown)

tests/test_h2_client.c

@@ -126,3 +126,32 @@ TEST_CASE(h2_client_connection_preface_sent) {
 
     return s_tester_clean_up();
 }
+
+/* Test that client will automatically send the PING ACK frame back, when the PING frame is received */
+TEST_CASE(h2_client_ping_ack) {
+    ASSERT_SUCCESS(s_tester_init(allocator, ctx));
+
+    /* Connection preface requires that SETTINGS be sent first (RFC-7540 3.5). */
+    ASSERT_SUCCESS(h2_fake_peer_send_connection_preface_default_settings(&s_tester.peer));
+
+    uint8_t opaque_data[AWS_H2_PING_DATA_SIZE] = {0, 1, 2, 3, 4, 5, 6, 7};
+
+    struct aws_h2_frame *frame = aws_h2_frame_new_ping(allocator, false /*ack*/, opaque_data);
+    ASSERT_NOT_NULL(frame);
+
+    ASSERT_SUCCESS(h2_fake_peer_send_frame(&s_tester.peer, frame));
+
+    /* Have the fake peer to run its decoder on what the client has written.
+     * The decoder will raise an error if it doesn't receive the "client connection preface string" first. */
+    ASSERT_SUCCESS(h2_fake_peer_decode_messages_from_testing_channel(&s_tester.peer));
+
+    /* Now check that client sent PING ACK frame, it should be the latest frame received by peer
+     * The last frame should be a ping type with ack on, and identical payload */
+    struct h2_decoded_frame *latest_frame = h2_decode_tester_latest_frame(&s_tester.peer.decode);
+    ASSERT_UINT_EQUALS(AWS_H2_FRAME_T_PING, latest_frame->type);
+    ASSERT_TRUE(latest_frame->ack);
+    ASSERT_BIN_ARRAYS_EQUALS(opaque_data, AWS_H2_PING_DATA_SIZE, latest_frame->ping_opaque_data, AWS_H2_PING_DATA_SIZE);
+
+    return s_tester_clean_up();
+}
+/* TODO: test that ping response is sent with higher priority than any other frame */