combine 2 callbacks into 1 (#25)

- `on_request()` instead of distinct `on_method()` and `on_uri()`
- rename `on_code()` -> `on_response()`

Author: graebm

include/aws/http/private/decode.h

@@ -49,26 +49,25 @@ typedef bool(aws_http_decoder_on_header_fn)(const struct aws_http_decoded_header
  */
 typedef bool(aws_http_decoder_on_body_fn)(const struct aws_byte_cursor *data, bool finished, void *user_data);
 
-typedef void(aws_http_decoder_on_uri_fn)(struct aws_byte_cursor *uri, void *user_data);
-typedef void(aws_http_decoder_on_response_code_fn)(int code, void *user_data);
-
-typedef void(aws_http_decoder_on_method_fn)(
-    enum aws_http_method method,
-    const struct aws_byte_cursor *method_data,
+typedef void(aws_http_decoder_on_request_fn)(
+    enum aws_http_method method_enum,
+    const struct aws_byte_cursor *method_str,
+    const struct aws_byte_cursor *uri,
     void *user_data);
 
+typedef void(aws_http_decoder_on_response_fn)(int status_code, void *user_data);
+
 typedef void(aws_http_decoder_done_fn)(void *user_data);
 
 struct aws_http_decoder_vtable {
     aws_http_decoder_on_header_fn *on_header;
     aws_http_decoder_on_body_fn *on_body;
 
     /* Only needed for requests, can be NULL for responses. */
-    aws_http_decoder_on_uri_fn *on_uri;
-    aws_http_decoder_on_method_fn *on_method;
+    aws_http_decoder_on_request_fn *on_request;
 
     /* Only needed for responses, can be NULL for requests. */
-    aws_http_decoder_on_response_code_fn *on_code;
+    aws_http_decoder_on_response_fn *on_response;
 
     aws_http_decoder_done_fn *on_done;
 };

source/connection_h1.c

@@ -15,6 +15,7 @@
 
 #include <aws/http/private/connection_impl.h>
 
+#include <aws/common/math.h>
 #include <aws/common/mutex.h>
 #include <aws/common/string.h>
 #include <aws/http/private/decode.h>
@@ -60,9 +61,12 @@ static void s_handler_destroy(struct aws_channel_handler *handler);
 static struct aws_http_stream *s_new_client_request_stream(const struct aws_http_request_options *options);
 static void s_stream_destroy(struct aws_http_stream *stream_base);
 static void s_stream_update_window(struct aws_http_stream *stream, size_t increment_size);
-static void s_decoder_on_method(enum aws_http_method method, const struct aws_byte_cursor *method_str, void *user_data);
-static void s_decoder_on_uri(struct aws_byte_cursor *uri, void *user_data);
-static void s_decoder_on_response_code(int status_code, void *user_data);
+static void s_decoder_on_request(
+    enum aws_http_method method_enum,
+    const struct aws_byte_cursor *method_str,
+    const struct aws_byte_cursor *uri,
+    void *user_data);
+static void s_decoder_on_response(int status_code, void *user_data);
 static bool s_decoder_on_header(const struct aws_http_decoded_header *header, void *user_data);
 static bool s_decoder_on_body(const struct aws_byte_cursor *data, bool finished, void *user_data);
 static void s_decoder_on_done(void *user_data);
@@ -88,9 +92,8 @@ static const struct aws_http_stream_vtable s_stream_vtable = {
 };
 
 static const struct aws_http_decoder_vtable s_decoder_vtable = {
-    .on_method = s_decoder_on_method,
-    .on_uri = s_decoder_on_uri,
-    .on_code = s_decoder_on_response_code,
+    .on_request = s_decoder_on_request,
+    .on_response = s_decoder_on_response,
     .on_header = s_decoder_on_header,
     .on_body = s_decoder_on_body,
     .on_done = s_decoder_on_done,
@@ -838,52 +841,50 @@ static void s_outgoing_stream_task(struct aws_channel_task *task, void *arg, enu
     s_shutdown_connection(connection, aws_last_error());
 }
 
-static void s_decoder_on_method(
-    enum aws_http_method method,
+static void s_decoder_on_request(
+    enum aws_http_method method_enum,
     const struct aws_byte_cursor *method_str,
+    const struct aws_byte_cursor *uri,
     void *user_data) {
 
-    (void)method;
-
     struct h1_connection *connection = user_data;
     struct h1_stream *incoming_stream = connection->thread_data.incoming_stream;
 
-    AWS_LOGF_TRACE(
-        AWS_LS_HTTP_STREAM,
-        "id=%p: Incoming request method: " PRInSTR,
-        (void *)&incoming_stream->base,
-        AWS_BYTE_CURSOR_PRI(*method_str));
-
     assert(incoming_stream->base.incoming_request_method_str.len == 0);
-    /* TODO: combine decoder on_uri & on_method callbacks so we can allocate buffer all at once
-    incoming_stream->base.incoming_request_method = method;
-    incoming_stream->base.incoming_request_method_str = aws_byte_cursor_from_c_str(aws_http_method_to_str(method));
-    */
-}
-
-static void s_decoder_on_uri(struct aws_byte_cursor *uri, void *user_data) {
-    struct h1_connection *connection = user_data;
-    struct h1_stream *incoming_stream = connection->thread_data.incoming_stream;
-
-    assert(!incoming_stream->base.incoming_request_uri.ptr);
+    assert(incoming_stream->base.incoming_request_uri.len == 0);
 
     AWS_LOGF_TRACE(
         AWS_LS_HTTP_STREAM,
-        "id=%p: Incoming request uri: " PRInSTR,
+        "id=%p: Incoming request: method=" PRInSTR " uri=" PRInSTR,
         (void *)&incoming_stream->base,
+        AWS_BYTE_CURSOR_PRI(*method_str),
         AWS_BYTE_CURSOR_PRI(*uri));
 
-    /* TODO: combine decoder on_uri & on_method callbacks so we can allocate buffer all at once */
-
     /* TODO: Limit on lengths of incoming data https://httpwg.org/specs/rfc7230.html#attack.protocol.element.length */
 
-    int err = aws_byte_buf_init(&incoming_stream->incoming_storage_buf, incoming_stream->base.alloc, uri->len);
+    /* Copy strings to internal buffer */
+    struct aws_byte_buf *storage_buf = &incoming_stream->incoming_storage_buf;
+    assert(storage_buf->capacity == 0);
+
+    size_t storage_size = 0;
+    int err = aws_add_size_checked(uri->len, method_str->len, &storage_size);
     if (err) {
         goto error;
     }
 
-    aws_byte_buf_write(&incoming_stream->incoming_storage_buf, uri->ptr, uri->len);
-    incoming_stream->base.incoming_request_uri = aws_byte_cursor_from_buf(&incoming_stream->incoming_storage_buf);
+    err = aws_byte_buf_init(storage_buf, incoming_stream->base.alloc, storage_size);
+    if (err) {
+        goto error;
+    }
+
+    aws_byte_buf_write_from_whole_cursor(storage_buf, *method_str);
+    incoming_stream->base.incoming_request_method_str = aws_byte_cursor_from_buf(storage_buf);
+
+    aws_byte_buf_write_from_whole_cursor(storage_buf, *uri);
+    incoming_stream->base.incoming_request_uri = aws_byte_cursor_from_buf(storage_buf);
+    aws_byte_cursor_advance(&incoming_stream->base.incoming_request_method_str, storage_buf->len - uri->len);
+
+    incoming_stream->base.incoming_request_method = method_enum;
 
     return;
 error:
@@ -895,7 +896,7 @@ static void s_decoder_on_uri(struct aws_byte_cursor *uri, void *user_data) {
     s_shutdown_connection(connection, aws_last_error());
 }
 
-static void s_decoder_on_response_code(int status_code, void *user_data) {
+static void s_decoder_on_response(int status_code, void *user_data) {
     struct h1_connection *connection = user_data;
 
     AWS_LOGF_TRACE(

source/decode.c

@@ -475,12 +475,8 @@ static int s_linestate_request(struct aws_http_decoder *decoder, struct aws_byte
         return aws_raise_error(AWS_ERROR_HTTP_PARSE);
     }
 
-    if (decoder->vtable.on_method) {
-        decoder->vtable.on_method(aws_http_str_to_method(method), &method, decoder->user_data);
-    }
-
-    if (decoder->vtable.on_uri) {
-        decoder->vtable.on_uri(&uri, decoder->user_data);
+    if (decoder->vtable.on_request) {
+        decoder->vtable.on_request(aws_http_str_to_method(method), &method, &uri, decoder->user_data);
     }
 
     s_set_line_state(decoder, s_linestate_header);
@@ -515,8 +511,8 @@ static int s_linestate_response(struct aws_http_decoder *decoder, struct aws_byt
         return aws_raise_error(AWS_ERROR_HTTP_PARSE);
     }
 
-    if (decoder->vtable.on_code) {
-        decoder->vtable.on_code((int)code_val, decoder->user_data);
+    if (decoder->vtable.on_response) {
+        decoder->vtable.on_response((int)code_val, decoder->user_data);
     }
 
     s_set_line_state(decoder, s_linestate_header);

tests/CMakeLists.txt

@@ -5,10 +5,9 @@ file(GLOB TEST_HDRS "*.h")
 file(GLOB TEST_SRC "*.c")
 file(GLOB TESTS ${TEST_HDRS} ${TEST_SRC})
 
-add_test_case(http_test_get_method)
+add_test_case(http_test_get_request)
 add_test_case(http_test_request_bad_version)
 add_test_case(http_test_response_bad_version)
-add_test_case(http_test_get_uri)
 add_test_case(http_test_get_status_code)
 add_test_case(http_test_overflow_scratch_space)
 add_test_case(http_test_receive_request_headers)

tests/test_decode.c

@@ -49,50 +49,54 @@ static bool s_on_body_stub(const struct aws_byte_cursor *data, bool finished, vo
     return true;
 }
 
-static void s_on_uri(struct aws_byte_cursor *uri, void *user_data) {
-    struct aws_byte_cursor *ptr = (struct aws_byte_cursor *)user_data;
-    if (ptr) {
-        size_t len = ptr->len < uri->len ? ptr->len : uri->len;
-        memcpy(ptr->ptr, uri->ptr, len);
-        ptr->len = len;
-    }
-}
-
-static void s_on_uri_stub(struct aws_byte_cursor *uri, void *user_data) {
-    (void)uri;
-    (void)user_data;
-}
-
-static void s_on_code(int code, void *user_data) {
+static void s_on_response(int code, void *user_data) {
     int *ptr = (int *)user_data;
     if (ptr) {
         *ptr = code;
     }
 }
 
-static void s_on_code_stub(int code, void *user_data) {
+static void s_on_response_stub(int code, void *user_data) {
     (void)code;
     (void)user_data;
 }
 
 struct request_data {
     enum aws_http_method method_enum;
     struct aws_byte_cursor method_str;
+    struct aws_byte_cursor uri;
     uint8_t buffer[1024];
 };
 
-static void s_on_method(enum aws_http_method method, const struct aws_byte_cursor *method_str, void *user_data) {
+static void s_on_request(
+    enum aws_http_method method_enum,
+    const struct aws_byte_cursor *method_str,
+    const struct aws_byte_cursor *uri,
+    void *user_data) {
+
     struct request_data *request_data = (struct request_data *)user_data;
+    assert(sizeof(request_data->buffer) >= uri->len + method_str->len);
     if (request_data) {
-        request_data->method_enum = method;
+        request_data->method_enum = method_enum;
+
         memcpy(request_data->buffer, method_str->ptr, method_str->len);
         request_data->method_str = aws_byte_cursor_from_array(request_data->buffer, method_str->len);
+
+        uint8_t *uri_dst = request_data->buffer + method_str->len;
+        memcpy(uri_dst, uri->ptr, uri->len);
+        request_data->uri = aws_byte_cursor_from_array(uri_dst, uri->len);
     }
 }
 
-static void s_on_method_stub(enum aws_http_method method, const struct aws_byte_cursor *method_str, void *user_data) {
-    (void)method;
+static void s_on_request_stub(
+    enum aws_http_method method_enum,
+    const struct aws_byte_cursor *method_str,
+    const struct aws_byte_cursor *uri,
+    void *user_data) {
+
+    (void)method_enum;
     (void)method_str;
+    (void)uri;
     (void)user_data;
 }
 
@@ -115,14 +119,13 @@ static void s_common_test_setup(
     params->user_data = user_data;
     params->vtable.on_header = s_on_header_stub;
     params->vtable.on_body = s_on_body_stub;
-    params->vtable.on_uri = s_on_uri_stub;
-    params->vtable.on_code = s_on_code_stub;
-    params->vtable.on_method = s_on_method_stub;
+    params->vtable.on_request = s_on_request_stub;
+    params->vtable.on_response = s_on_response_stub;
     params->vtable.on_done = s_on_done;
 }
 
-AWS_TEST_CASE(http_test_get_method, s_http_test_get_method);
-static int s_http_test_get_method(struct aws_allocator *allocator, void *ctx) {
+AWS_TEST_CASE(http_test_get_request, s_http_test_get_request);
+static int s_http_test_get_request(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
 
     struct request_data request_data;
@@ -131,7 +134,7 @@ static int s_http_test_get_method(struct aws_allocator *allocator, void *ctx) {
 
     struct aws_http_decoder_params params;
     s_common_test_setup(allocator, 1024, &params, s_request, &request_data);
-    params.vtable.on_method = s_on_method;
+    params.vtable.on_request = s_on_request;
     struct aws_http_decoder *decoder = aws_http_decoder_new(&params);
 
     size_t len = strlen(msg);
@@ -140,6 +143,8 @@ static int s_http_test_get_method(struct aws_allocator *allocator, void *ctx) {
 
     ASSERT_TRUE(aws_byte_cursor_eq_c_str(&request_data.method_str, "HEAD"));
 
+    ASSERT_TRUE(aws_byte_cursor_eq_c_str(&request_data.uri, "/"));
+
     aws_http_decoder_destroy(decoder);
     aws_http_library_clean_up();
     return AWS_OP_SUCCESS;
@@ -179,28 +184,6 @@ static int s_http_test_response_bad_version(struct aws_allocator *allocator, voi
     return AWS_OP_SUCCESS;
 }
 
-AWS_TEST_CASE(http_test_get_uri, s_http_test_get_uri);
-static int s_http_test_get_uri(struct aws_allocator *allocator, void *ctx) {
-    (void)ctx;
-
-    uint8_t buf[128];
-    struct aws_byte_cursor uri_data = aws_byte_cursor_from_array(buf, 128);
-
-    const char *msg = s_typical_request;
-    struct aws_http_decoder_params params;
-    s_common_test_setup(allocator, 1024, &params, s_request, &uri_data);
-    params.vtable.on_uri = s_on_uri;
-    struct aws_http_decoder *decoder = aws_http_decoder_new(&params);
-
-    size_t len = strlen(msg);
-    ASSERT_SUCCESS(aws_http_decode(decoder, msg, len, NULL));
-    ASSERT_TRUE(aws_byte_cursor_eq_c_str(&uri_data, "/"));
-
-    aws_http_decoder_destroy(decoder);
-    aws_http_library_clean_up();
-    return AWS_OP_SUCCESS;
-}
-
 AWS_TEST_CASE(http_test_get_status_code, s_http_test_get_status_code);
 static int s_http_test_get_status_code(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
@@ -210,7 +193,7 @@ static int s_http_test_get_status_code(struct aws_allocator *allocator, void *ct
     const char *msg = s_typical_response;
     struct aws_http_decoder_params params;
     s_common_test_setup(allocator, 1024, &params, s_response, &code);
-    params.vtable.on_code = s_on_code;
+    params.vtable.on_response = s_on_response;
     struct aws_http_decoder *decoder = aws_http_decoder_new(&params);
 
     size_t len = strlen(msg);