Remove whole-frame h2 decoder (#179)

We use the streaming decoder now. Most changes are rewriting tests that assumed the encoder/decoder were mirrors of each other.

**REMOVE**
- `aws_h2_frame_decoder_***()` functions
- `fuzz_h2_frames.c` which just tested the old decoder.

**ALTER**
- Rewrite encoder tests (delete `test_h2_frames.c`, add `test_h2_encoder.c`). Old tests relied on whole-frame decoder.
- Alter `test_h2_headers.c` to use new streaming decoder.
- Add `aws_h2_encode_frame()` function that accepts all frames types, instead of needing to call the appropriate aws_h2_frame_XYZ_encode() function per frame type.

**FIX**
- Fix encoder bug with padding in DATA frame.
- Fix decoder bug where header-value-string overwrote header-name-string.

Author: graebm

include/aws/http/private/h2_connection.h

@@ -17,6 +17,7 @@
  */
 
 #include <aws/common/atomics.h>
+#include <aws/common/hash_table.h>
 #include <aws/common/mutex.h>
 
 #include <aws/http/private/connection_impl.h>

include/aws/http/private/h2_frames.h

@@ -19,7 +19,6 @@
 #include <aws/http/request_response.h>
 
 #include <aws/common/byte_buf.h>
-#include <aws/common/hash_table.h>
 
 /* Ids for each frame type (RFC-7540 6) */
 enum aws_h2_frame_type {
@@ -159,6 +158,12 @@ struct aws_h2_frame_rst_stream {
     enum aws_h2_error_codes error_code;
 };
 
+/* A h2 setting and its value, used in SETTINGS frame */
+struct aws_h2_frame_setting {
+    uint16_t id; /* aws_h2_settings */
+    uint32_t value;
+};
+
 /* Represents a SETTINGS frame */
 struct aws_h2_frame_settings {
     /* Header */
@@ -168,8 +173,8 @@ struct aws_h2_frame_settings {
     bool ack; /* AWS_H2_FRAME_F_ACK */
 
     /* Payload */
-    /* uint16_t -> uint32_t */
-    struct aws_hash_table settings;
+    struct aws_h2_frame_setting *settings_array;
+    size_t settings_count;
 };
 
 /* Represents a PUSH_PROMISE frame */
@@ -186,6 +191,8 @@ struct aws_h2_frame_push_promise {
     struct aws_h2_frame_header_block header_block;
 };
 
+#define AWS_H2_PING_DATA_SIZE (8)
+
 /* Represents a PING frame */
 struct aws_h2_frame_ping {
     /* Header */
@@ -195,7 +202,7 @@ struct aws_h2_frame_ping {
     bool ack; /* AWS_H2_FRAME_F_ACK */
 
     /* Payload */
-    struct aws_byte_cursor opaque_data;
+    uint8_t opaque_data[AWS_H2_PING_DATA_SIZE];
 };
 
 /* Represents a GOAWAY frame */
@@ -238,19 +245,6 @@ struct aws_h2_frame_encoder {
     bool use_huffman;
 };
 
-/* Used to decode a frame */
-struct aws_h2_frame_decoder {
-    /* Larger state */
-    struct aws_allocator *allocator;
-    struct aws_byte_buf header_scratch;
-    struct aws_hpack_context *hpack;
-
-    /* Packet-in-progress */
-    struct aws_h2_frame_header header;
-    uint8_t flags;
-    struct aws_byte_cursor payload;
-};
-
 AWS_EXTERN_C_BEGIN
 
 AWS_HTTP_API
@@ -271,48 +265,34 @@ int aws_h2_frame_header_block_encode(
     const struct aws_h2_frame_header_block *header_block,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_header_block_decode(
-    struct aws_h2_frame_header_block *header_block,
-    struct aws_h2_frame_decoder *decoder);
 
 /**
  * The process of encoding a frame looks like:
  * 1. Create a encoder object on the stack and initialize with aws_h2_frame_encoder_init
  * 2. Encode the header using aws_h2_frame_*_encode
  */
-
 AWS_HTTP_API
 int aws_h2_frame_encoder_init(struct aws_h2_frame_encoder *encoder, struct aws_allocator *allocator);
 AWS_HTTP_API
 void aws_h2_frame_encoder_clean_up(struct aws_h2_frame_encoder *encoder);
 
-/**
- * The process of decoding a frame looks like:
- * 1. Create a decoder object on the stack and initialize with aws_h2_frame_decoder_init
- * 2. Decode the header using aws_h2_frame_decoder_begin
- * 3. Switch on header->type, and create a new instance of the appropriate frame type
- * 4. Call aws_h2_frame_*_decode to decode the rest of the frame
- */
-
-AWS_HTTP_API
-int aws_h2_frame_decoder_init(struct aws_h2_frame_decoder *decoder, struct aws_allocator *allocator);
+/* #TODO: remove each frame type's specific encode() function from API */
 AWS_HTTP_API
-void aws_h2_frame_decoder_clean_up(struct aws_h2_frame_decoder *decoder);
-AWS_HTTP_API
-int aws_h2_frame_decoder_begin(struct aws_h2_frame_decoder *decoder, struct aws_byte_cursor *data);
+int aws_h2_encode_frame(
+    struct aws_h2_frame_encoder *encoder,
+    struct aws_h2_frame_header *frame_header,
+    struct aws_byte_buf *output);
 
 AWS_HTTP_API
 int aws_h2_frame_data_init(struct aws_h2_frame_data *frame, struct aws_allocator *allocator);
 AWS_HTTP_API
 void aws_h2_frame_data_clean_up(struct aws_h2_frame_data *frame);
+
 AWS_HTTP_API
 int aws_h2_frame_data_encode(
     struct aws_h2_frame_data *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_data_decode(struct aws_h2_frame_data *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_headers_init(struct aws_h2_frame_headers *frame, struct aws_allocator *allocator);
@@ -323,8 +303,6 @@ int aws_h2_frame_headers_encode(
     struct aws_h2_frame_headers *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_headers_decode(struct aws_h2_frame_headers *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_priority_init(struct aws_h2_frame_priority *frame, struct aws_allocator *allocator);
@@ -335,8 +313,6 @@ int aws_h2_frame_priority_encode(
     struct aws_h2_frame_priority *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_priority_decode(struct aws_h2_frame_priority *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_rst_stream_init(struct aws_h2_frame_rst_stream *frame, struct aws_allocator *allocator);
@@ -347,24 +323,16 @@ int aws_h2_frame_rst_stream_encode(
     struct aws_h2_frame_rst_stream *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_rst_stream_decode(struct aws_h2_frame_rst_stream *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_settings_init(struct aws_h2_frame_settings *frame, struct aws_allocator *allocator);
 AWS_HTTP_API
 void aws_h2_frame_settings_clean_up(struct aws_h2_frame_settings *frame);
 AWS_HTTP_API
-int aws_h2_frame_settings_set(struct aws_h2_frame_settings *frame, uint16_t identifier, uint32_t value);
-AWS_HTTP_API
-int aws_h2_frame_settings_remove(struct aws_h2_frame_settings *frame, uint16_t identifier);
-AWS_HTTP_API
 int aws_h2_frame_settings_encode(
     struct aws_h2_frame_settings *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_settings_decode(struct aws_h2_frame_settings *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_push_promise_init(struct aws_h2_frame_push_promise *frame, struct aws_allocator *allocator);
@@ -375,8 +343,6 @@ int aws_h2_frame_push_promise_encode(
     struct aws_h2_frame_push_promise *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_push_promise_decode(struct aws_h2_frame_push_promise *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_ping_init(struct aws_h2_frame_ping *frame, struct aws_allocator *allocator);
@@ -387,8 +353,6 @@ int aws_h2_frame_ping_encode(
     struct aws_h2_frame_ping *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_ping_decode(struct aws_h2_frame_ping *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_goaway_init(struct aws_h2_frame_goaway *frame, struct aws_allocator *allocator);
@@ -399,8 +363,6 @@ int aws_h2_frame_goaway_encode(
     struct aws_h2_frame_goaway *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_goaway_decode(struct aws_h2_frame_goaway *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_window_update_init(struct aws_h2_frame_window_update *frame, struct aws_allocator *allocator);
@@ -411,8 +373,6 @@ int aws_h2_frame_window_update_encode(
     struct aws_h2_frame_window_update *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_window_update_decode(struct aws_h2_frame_window_update *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_HTTP_API
 int aws_h2_frame_continuation_init(struct aws_h2_frame_continuation *frame, struct aws_allocator *allocator);
@@ -423,8 +383,6 @@ int aws_h2_frame_continuation_encode(
     struct aws_h2_frame_continuation *frame,
     struct aws_h2_frame_encoder *encoder,
     struct aws_byte_buf *output);
-AWS_HTTP_API
-int aws_h2_frame_continuation_decode(struct aws_h2_frame_continuation *frame, struct aws_h2_frame_decoder *decoder);
 
 AWS_EXTERN_C_END
 

include/aws/http/private/h2_stream.h

@@ -78,9 +78,6 @@ struct aws_h2_stream *aws_h2_stream_new_request(
     struct aws_http_connection *client_connection,
     const struct aws_http_make_request_options *options);
 
-AWS_HTTP_API
-int aws_h2_stream_handle_frame(struct aws_h2_stream *stream, struct aws_h2_frame_decoder *decoder);
-
 AWS_EXTERN_C_END
 
 #endif /* AWS_HTTP_H2_STREAM_H */

source/h2_decoder.c

@@ -368,13 +368,16 @@ static enum aws_hpack_decode_status s_decode_string(
  * State functions
  **********************************************************************************************************************/
 
-static void s_decoder_run_state(
+static void s_decoder_run_state_ex(
     struct aws_h2_decoder *decoder,
     const struct decoder_state *state,
-    struct aws_byte_cursor *input) {
+    struct aws_byte_cursor *input,
+    bool preserve_scratch) {
 
     DECODER_LOGF(TRACE, decoder, "Moving from state %s to %s", decoder->state.name, state->name);
-    decoder->scratch.len = 0;
+    if (!preserve_scratch) {
+        decoder->scratch.len = 0;
+    }
 
     /* Special case for 0 length frames, otherwise frames could sit in incomplete until more data arrives */
     if (state->bytes_required == 0) {
@@ -384,6 +387,21 @@ static void s_decoder_run_state(
     }
 }
 
+static void s_decoder_run_state(
+    struct aws_h2_decoder *decoder,
+    const struct decoder_state *state,
+    struct aws_byte_cursor *input) {
+
+    s_decoder_run_state_ex(decoder, state, input, false /*preserve_scratch*/);
+}
+static void s_decoder_run_state_preserve_scratch(
+    struct aws_h2_decoder *decoder,
+    const struct decoder_state *state,
+    struct aws_byte_cursor *input) {
+
+    s_decoder_run_state_ex(decoder, state, input, true /*preserve_scratch*/);
+}
+
 static void s_decoder_run_frame_state(struct aws_h2_decoder *decoder, struct aws_byte_cursor *input) {
     AWS_ASSERT(decoder->frame_in_progress.type <= AWS_H2_FRAME_T_UNKNOWN);
     s_decoder_run_state(decoder, s_state_frames[decoder->frame_in_progress.type], input);
@@ -431,19 +449,12 @@ static int s_state_fn_header(struct aws_h2_decoder *decoder, struct aws_byte_cur
 
     AWS_ASSERT(input->len >= 9);
 
-    uint32_t payload_len = 0;
-    uint8_t *length_ptr = ((uint8_t *)&payload_len);
-
     /* Read the first 3 bytes */
-    bool succ = aws_byte_cursor_read(input, length_ptr, 3);
+    uint32_t payload_len = 0;
+    bool succ = aws_byte_cursor_read_be24(input, &payload_len);
     AWS_ASSERT(succ);
     (void)succ;
 
-    /* Reverse from network order */
-    payload_len = aws_ntoh24(payload_len);
-    /* Assert top byte isn't set */
-    AWS_ASSERT((payload_len & 0xFF000000) == 0);
-
     /* #TODO handle the SETTINGS_MAX_FRAME_SIZE setting */
     static const uint32_t MAX_FRAME_SIZE = 16384;
     if (payload_len > MAX_FRAME_SIZE) {
@@ -831,8 +842,8 @@ static int s_state_fn_frame_ping(struct aws_h2_decoder *decoder, struct aws_byte
 
     AWS_ASSERT(input->len >= 8);
 
-    uint8_t opaque_data[8] = {0};
-    bool succ = aws_byte_cursor_read(input, &opaque_data, AWS_ARRAY_SIZE(opaque_data));
+    uint8_t opaque_data[AWS_H2_PING_DATA_SIZE] = {0};
+    bool succ = aws_byte_cursor_read(input, &opaque_data, AWS_H2_PING_DATA_SIZE);
     AWS_ASSERT(succ);
     (void)succ;
 
@@ -1139,8 +1150,8 @@ static int s_state_fn_headers_literal_name(struct aws_h2_decoder *decoder, struc
     /* The value will start after the name, so save how long it is */
     progress->value_offset = decoder->scratch.len;
 
-    /* Name gotten, go to value */
-    s_decoder_run_state(decoder, &s_state_headers_literal_value, input);
+    /* Name is in scratch, preserve it and go to value */
+    s_decoder_run_state_preserve_scratch(decoder, &s_state_headers_literal_value, input);
 
     return AWS_OP_SUCCESS;
 }