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.

Author: JonathanHenson

.github/workflows/ci.yml

@@ -7,7 +7,7 @@ on:
       - '!master'
 
 env:
-  BUILDER_VERSION: v0.5.3
+  BUILDER_VERSION: v0.5.8
   BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
   PACKAGE_NAME: aws-c-http
   LINUX_BASE_IMAGE: ubuntu-16-x64

builder.json

@@ -13,11 +13,8 @@
     "downstream": [
         { "name": "aws-c-auth" }
     ],
-    "test": [
-        ["ctest", "{build_dir}", "-v", "--output-on-failure"],
+    "test_steps": [
+        "test",
         ["{python}", "{project_dir}/integration-testing/http_client_test.py", "{install_dir}/bin/elasticurl{exe}"]
-    ],
-    "cmake_args": [
-        "-DS2N_NO_PQ_ASM=ON"
     ]
 }

include/aws/http/private/http_impl.h

@@ -26,6 +26,7 @@ enum aws_http_method {
     AWS_HTTP_METHOD_UNKNOWN, /* Unrecognized value. */
     AWS_HTTP_METHOD_GET,
     AWS_HTTP_METHOD_HEAD,
+    AWS_HTTP_METHOD_CONNECT,
     AWS_HTTP_METHOD_COUNT, /* Number of enums */
 };
 
@@ -50,6 +51,7 @@ enum aws_http_status {
     AWS_HTTP_STATUS_UNKNOWN = -1, /* Invalid status code. Not using 0 because it's technically a legal value */
     AWS_HTTP_STATUS_100_CONTINUE = 100,
     AWS_HTTP_STATUS_101_SWITCHING_PROTOCOLS = 101,
+    AWS_HTTP_STATUS_200_OK = 200,
     AWS_HTTP_STATUS_204_NO_CONTENT = 204,
     AWS_HTTP_STATUS_304_NOT_MODIFIED = 304,
 };

source/h1_connection.c

@@ -985,7 +985,14 @@ static int s_decoder_on_header(const struct aws_h1_decoded_header *header, void
     /* RFC-7230 section 6.1.
      * "Connection: close" header signals that a connection will not persist after the current request/response */
     if (header->name == AWS_HTTP_HEADER_CONNECTION) {
-        if (aws_byte_cursor_eq_c_str(&header->value_data, "close")) {
+        /* Certain L7 proxies send a connection close header on a 200/OK response to a CONNECT request. This is nutty
+         * behavior, but the obviously desired behavior on a 200 CONNECT response is to leave the connection open
+         * for the tunneling. */
+        bool ignore_connection_close = incoming_stream->base.request_method == AWS_HTTP_METHOD_CONNECT &&
+                                       incoming_stream->base.client_data &&
+                                       incoming_stream->base.client_data->response_status == AWS_HTTP_STATUS_200_OK;
+
+        if (!ignore_connection_close && aws_byte_cursor_eq_c_str_ignore_case(&header->value_data, "close")) {
             AWS_LOGF_TRACE(
                 AWS_LS_HTTP_STREAM,
                 "id=%p: Received 'Connection: close' header. This will be the final stream on this connection.",

source/h1_stream.c

@@ -15,6 +15,7 @@
 #include <aws/http/private/h1_stream.h>
 
 #include <aws/http/private/connection_impl.h>
+
 #include <aws/io/logging.h>
 
 static void s_stream_destroy(struct aws_http_stream *stream_base) {

source/http.c

@@ -202,6 +202,7 @@ static struct aws_byte_cursor s_method_enum_to_str[AWS_HTTP_METHOD_COUNT]; /* fo
 static void s_methods_init(struct aws_allocator *alloc) {
     s_method_enum_to_str[AWS_HTTP_METHOD_GET] = aws_http_method_get;
     s_method_enum_to_str[AWS_HTTP_METHOD_HEAD] = aws_http_method_head;
+    s_method_enum_to_str[AWS_HTTP_METHOD_CONNECT] = aws_http_method_connect;
 
     s_init_str_to_enum_hash_table(
         &s_method_str_to_enum,

tests/proxy_test_helper.c

@@ -254,7 +254,9 @@ int proxy_tester_create_testing_channel_connection(struct proxy_tester *tester)
     tester->testing_channel->channel_shutdown = s_testing_channel_shutdown_callback;
     tester->testing_channel->channel_shutdown_user_data = tester;
 
-    struct aws_http_connection *connection = aws_http_connection_new_http1_1_client(tester->alloc, SIZE_MAX);
+    /* Use small window so that we can observe it opening in tests.
+     * Channel may wait until the window is small before issuing the increment command. */
+    struct aws_http_connection *connection = aws_http_connection_new_http1_1_client(tester->alloc, 256);
     ASSERT_NOT_NULL(connection);
 
     connection->user_data = tester->http_bootstrap->user_data;
@@ -266,9 +268,9 @@ int proxy_tester_create_testing_channel_connection(struct proxy_tester *tester)
     ASSERT_SUCCESS(aws_channel_slot_insert_end(tester->testing_channel->channel, slot));
     ASSERT_SUCCESS(aws_channel_slot_set_handler(slot, &connection->channel_handler));
     connection->vtable->on_channel_handler_installed(&connection->channel_handler, slot);
+    testing_channel_drain_queued_tasks(tester->testing_channel);
 
     tester->client_connection = connection;
-    testing_channel_drain_queued_tasks(tester->testing_channel);
 
     return AWS_OP_SUCCESS;
 }
@@ -309,7 +311,9 @@ int proxy_tester_send_connect_response(struct proxy_tester *tester) {
     if (tester->failure_type == PTFT_CONNECT_REQUEST) {
         response_string = "HTTP/1.0 401 Unauthorized\r\n\r\n";
     } else {
-        response_string = "HTTP/1.0 200 Connection established\r\n\r\n";
+        /* adding close here because it's an edge case we need to exercise. The desired behavior is that it has
+         * absolutely no effect. */
+        response_string = "HTTP/1.0 200 Connection established\r\nconnection: close\r\n\r\n";
     }
 
     /* send response */

tests/test_connection.c

@@ -530,7 +530,7 @@ static bool s_tester_new_client_shutdown_pred(void *user_data) {
     return tester->new_client_shut_down;
 }
 
-/* when we shutdown the server, no more new connection will be accept */
+/* when we shutdown the server, no more new connection will be accepted */
 static int s_test_connection_server_shutting_down_new_connection_setup_fail(
     struct aws_allocator *allocator,
     void *ctx) {
@@ -591,9 +591,13 @@ static int s_test_connection_server_shutting_down_new_connection_setup_fail(
     ASSERT_FAILS(s_tester_wait(&tester, s_tester_connection_setup_pred));
     /* wait for the client side connection */
     s_tester_wait(&tester, s_tester_new_client_setup_pred);
-    if (tester.new_client_connection) {
+
+    if (tester.new_client_connection && !tester.client_connection_is_shutdown) {
         /* wait for it to shut down, we do not need to call shut down, the socket will know */
         ASSERT_SUCCESS(s_tester_wait(&tester, s_tester_new_client_shutdown_pred));
+    }
+
+    if (tester.new_client_connection) {
         aws_http_connection_release(tester.new_client_connection);
     }
 

tests/test_h1_client.c

@@ -71,7 +71,9 @@ static int s_tester_init(struct tester *tester, struct aws_allocator *alloc) {
     struct aws_testing_channel_options test_channel_options = {.clock_fn = aws_high_res_clock_get_ticks};
     ASSERT_SUCCESS(testing_channel_init(&tester->testing_channel, alloc, &test_channel_options));
 
-    tester->connection = aws_http_connection_new_http1_1_client(alloc, SIZE_MAX);
+    /* Use small window so that we can observe it opening in tests.
+     * Channel may wait until the window is small before issuing the increment command. */
+    tester->connection = aws_http_connection_new_http1_1_client(alloc, 256);
     ASSERT_NOT_NULL(tester->connection);
 
     struct aws_channel_slot *slot = aws_channel_slot_new(tester->testing_channel.channel);
@@ -1501,7 +1503,7 @@ H1_CLIENT_TEST_CASE(h1_client_window_shrinks_if_user_says_so) {
     /* check result */
     size_t window_update = testing_channel_last_window_update(&tester.testing_channel);
     size_t message_sans_body = strlen(response_str) - 9;
-    ASSERT_TRUE(window_update == message_sans_body);
+    ASSERT_UINT_EQUALS(message_sans_body, window_update);
 
     /* clean up */
     ASSERT_SUCCESS(s_response_tester_clean_up(&response));

tests/test_proxy.c

@@ -326,8 +326,8 @@ static int s_test_https_proxy_connection_failure_tls(struct aws_allocator *alloc
 
     ASSERT_SUCCESS(proxy_tester_verify_connection_attempt_was_to_proxy(
         &tester, aws_byte_cursor_from_c_str(s_proxy_host_name), s_proxy_port));
-    ASSERT_TRUE(tester.client_connection == NULL);
-    ASSERT_TRUE(tester.wait_result != AWS_ERROR_SUCCESS);
+    ASSERT_NULL(tester.client_connection);
+    ASSERT_TRUE(AWS_ERROR_SUCCESS != tester.wait_result);
 
     ASSERT_SUCCESS(proxy_tester_clean_up(&tester));
 

tests/test_websocket_handler.c

@@ -24,6 +24,10 @@
 #    pragma warning(disable : 4204) /* non-constant aggregate initializer */
 #endif
 
+/* Use small window so that we can observe it opening in tests.
+ * Channel may wait until the window is small before issuing the increment command. */
+static const size_t s_default_initial_window_size = 256;
+
 #define TEST_CASE(NAME)                                                                                                \
     AWS_TEST_CASE(NAME, s_test_##NAME);                                                                                \
     static int s_test_##NAME(struct aws_allocator *allocator, void *ctx)
@@ -487,7 +491,7 @@ static int s_tester_init(struct tester *tester, struct aws_allocator *alloc) {
     struct aws_websocket_handler_options ws_options = {
         .allocator = alloc,
         .channel = tester->testing_channel.channel,
-        .initial_window_size = SIZE_MAX,
+        .initial_window_size = s_default_initial_window_size,
         .user_data = tester,
         .on_incoming_frame_begin = s_on_incoming_frame_begin,
         .on_incoming_frame_payload = s_on_incoming_frame_payload,
@@ -496,6 +500,7 @@ static int s_tester_init(struct tester *tester, struct aws_allocator *alloc) {
     };
     tester->websocket = aws_websocket_handler_new(&ws_options);
     ASSERT_NOT_NULL(tester->websocket);
+    testing_channel_drain_queued_tasks(&tester->testing_channel);
 
     aws_websocket_decoder_init(&tester->written_frame_decoder, s_on_written_frame, s_on_written_frame_payload, tester);
     aws_websocket_encoder_init(&tester->readpush_encoder, s_stream_readpush_payload, tester);
@@ -530,6 +535,8 @@ static int s_install_downstream_handler(struct tester *tester, size_t initial_wi
     tester->is_midchannel_handler = true;
 
     ASSERT_SUCCESS(testing_channel_install_downstream_handler(&tester->testing_channel, initial_window));
+    testing_channel_drain_queued_tasks(&tester->testing_channel);
+
     return AWS_OP_SUCCESS;
 }
 
@@ -1693,6 +1700,7 @@ static int s_window_manual_increment_common(struct aws_allocator *allocator, boo
 
     /* Assert that window did not fully re-open*/
     uint64_t frame_minus_payload_size = aws_websocket_frame_encoded_size(&pushing.def) - pushing.def.payload_length;
+
     ASSERT_UINT_EQUALS(frame_minus_payload_size, testing_channel_last_window_update(&tester.testing_channel));
 
     /* Manually increment window */
@@ -1722,7 +1730,7 @@ TEST_CASE(websocket_midchannel_sanity_check) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
     ASSERT_SUCCESS(s_tester_clean_up(&tester));
     return AWS_OP_SUCCESS;
 }
@@ -1731,7 +1739,7 @@ TEST_CASE(websocket_midchannel_write_message) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
 
     /* Write data */
     struct aws_byte_cursor writing = aws_byte_cursor_from_c_str("My hat it has three corners");
@@ -1749,7 +1757,7 @@ TEST_CASE(websocket_midchannel_write_multiple_messages) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
 
     struct aws_byte_cursor writing[] = {
         aws_byte_cursor_from_c_str("My hat it has three corners."),
@@ -1774,7 +1782,7 @@ TEST_CASE(websocket_midchannel_write_huge_message) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
 
     /* Fill big buffer with random data */
     struct aws_byte_buf writing;
@@ -1801,7 +1809,7 @@ TEST_CASE(websocket_midchannel_read_message) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
 
     struct readpush_frame pushing = {
         .payload = aws_byte_cursor_from_c_str("Hello hello can you hear me Joe?"),
@@ -1822,7 +1830,7 @@ TEST_CASE(websocket_midchannel_read_multiple_messages) {
     (void)ctx;
     struct tester tester;
     ASSERT_SUCCESS(s_tester_init(&tester, allocator));
-    ASSERT_SUCCESS(s_install_downstream_handler(&tester, SIZE_MAX));
+    ASSERT_SUCCESS(s_install_downstream_handler(&tester, s_default_initial_window_size));
 
     /* Read a mix of different frame types, most of which shouldn't get passed along to next handler. */
     struct readpush_frame pushing[] = {