From e9deffc76cb28c6c07231dc4378867741d6dce23 Mon Sep 17 00:00:00 2001 From: Justin Boswell Date: Tue, 10 Mar 2020 17:21:14 -0700 Subject: [PATCH 01/11] test -> test_steps --- builder.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder.json b/builder.json index 8033334ad..73d0fcbb2 100644 --- a/builder.json +++ b/builder.json @@ -13,7 +13,7 @@ "downstream": [ { "name": "aws-c-auth" } ], - "test": [ + "test_steps": [ ["ctest", "{build_dir}", "-v", "--output-on-failure"], ["{python}", "{project_dir}/integration-testing/http_client_test.py", "{install_dir}/bin/elasticurl{exe}"] ], From 9fa971d5441bc141af84664707c85bdce95729f9 Mon Sep 17 00:00:00 2001 From: Justin Boswell Date: Tue, 10 Mar 2020 17:52:33 -0700 Subject: [PATCH 02/11] use fix-test-runs channel of builder --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 382a3ebe1..6c5f85842 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: - '!master' env: - BUILDER_VERSION: v0.5.3 + BUILDER_VERSION: fix-test-runs BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-http LINUX_BASE_IMAGE: ubuntu-16-x64 From b4a300c9548f616d31ab64f46c7913eeac830566 Mon Sep 17 00:00:00 2001 From: Justin Boswell Date: Tue, 10 Mar 2020 17:55:49 -0700 Subject: [PATCH 03/11] Use default tests first --- builder.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder.json b/builder.json index 73d0fcbb2..3aba08edb 100644 --- a/builder.json +++ b/builder.json @@ -14,7 +14,7 @@ { "name": "aws-c-auth" } ], "test_steps": [ - ["ctest", "{build_dir}", "-v", "--output-on-failure"], + "test", ["{python}", "{project_dir}/integration-testing/http_client_test.py", "{install_dir}/bin/elasticurl{exe}"] ], "cmake_args": [ From a07ff90840b9c4d2680567173cf3b76a89ab462f Mon Sep 17 00:00:00 2001 From: Justin Boswell Date: Tue, 10 Mar 2020 19:27:40 -0700 Subject: [PATCH 04/11] Updated to v0.5.4 of builder, removed PQ ASM cmake flag --- .github/workflows/ci.yml | 2 +- builder.json | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c5f85842..5ad78e0eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: - '!master' env: - BUILDER_VERSION: fix-test-runs + BUILDER_VERSION: v0.5.4 BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-http LINUX_BASE_IMAGE: ubuntu-16-x64 diff --git a/builder.json b/builder.json index 3aba08edb..d72e590e2 100644 --- a/builder.json +++ b/builder.json @@ -16,8 +16,5 @@ "test_steps": [ "test", ["{python}", "{project_dir}/integration-testing/http_client_test.py", "{install_dir}/bin/elasticurl{exe}"] - ], - "cmake_args": [ - "-DS2N_NO_PQ_ASM=ON" ] } From e6ac01ef219f9d9780dc2370c3494d3478ddf879 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Wed, 11 Mar 2020 14:45:28 -0700 Subject: [PATCH 05/11] Ignore connection: close from proxies, Fix tests to play nice with backpressure changes. --- include/aws/http/private/h1_encoder.h | 1 + source/h1_connection.c | 18 +++++++++++++++++- source/h1_encoder.c | 4 ++++ source/h1_stream.c | 1 + tests/proxy_test_helper.c | 8 ++++---- tests/test_h1_client.c | 4 ++-- tests/test_proxy.c | 4 ++-- tests/test_websocket_handler.c | 20 +++++++++++++------- 8 files changed, 44 insertions(+), 16 deletions(-) diff --git a/include/aws/http/private/h1_encoder.h b/include/aws/http/private/h1_encoder.h index 27090a760..e8df09ac4 100644 --- a/include/aws/http/private/h1_encoder.h +++ b/include/aws/http/private/h1_encoder.h @@ -28,6 +28,7 @@ struct aws_h1_encoder_message { struct aws_input_stream *body; uint64_t content_length; bool has_connection_close_header; + bool ignore_response_connection_close_header; }; enum aws_h1_encoder_state { diff --git a/source/h1_connection.c b/source/h1_connection.c index 045e6cd09..868645703 100644 --- a/source/h1_connection.c +++ b/source/h1_connection.c @@ -182,6 +182,10 @@ struct h1_connection { /* If non-zero, then window_update_task is scheduled */ size_t window_update_size; + + /* for bug for bug compatibility with L7 proxies. They can send connection:close on successful connect + * responses. This should be ignored. */ + bool ignore_response_connection_close_header; } synced_data; }; @@ -442,6 +446,9 @@ struct aws_http_stream *s_make_request( { /* BEGIN CRITICAL SECTION */ s_h1_connection_lock_synced_data(connection); + connection->synced_data.ignore_response_connection_close_header = + stream->encoder_message.ignore_response_connection_close_header; + if (connection->synced_data.new_stream_error_code) { new_stream_error_code = connection->synced_data.new_stream_error_code; } else { @@ -985,7 +992,16 @@ 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")) { + bool ignore_connection_close = false; + { + /*BEGIN CRITICAL SECTION*/ + aws_mutex_lock(&connection->synced_data.lock); + ignore_connection_close = connection->synced_data.ignore_response_connection_close_header; + connection->synced_data.ignore_response_connection_close_header = false; + aws_mutex_unlock(&connection->synced_data.lock); + /*END CRITICAL SECTION*/ + } + 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.", diff --git a/source/h1_encoder.c b/source/h1_encoder.c index a76a8ee9e..a7699e45a 100644 --- a/source/h1_encoder.c +++ b/source/h1_encoder.c @@ -131,6 +131,10 @@ int aws_h1_encoder_message_init_from_request( goto error; } + if (aws_byte_cursor_eq_c_str_ignore_case(&method, "connect")) { + message->ignore_response_connection_close_header = true; + } + struct aws_byte_cursor uri; err = aws_http_message_get_request_path(request, &uri); if (err) { diff --git a/source/h1_stream.c b/source/h1_stream.c index 222ed2ccf..1f030c94d 100644 --- a/source/h1_stream.c +++ b/source/h1_stream.c @@ -15,6 +15,7 @@ #include #include + #include static void s_stream_destroy(struct aws_http_stream *stream_base) { diff --git a/tests/proxy_test_helper.c b/tests/proxy_test_helper.c index e84cc4a89..b119aab1f 100644 --- a/tests/proxy_test_helper.c +++ b/tests/proxy_test_helper.c @@ -254,7 +254,7 @@ 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); + 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; @@ -265,10 +265,10 @@ int proxy_tester_create_testing_channel_connection(struct proxy_tester *tester) ASSERT_NOT_NULL(slot); 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); + connection->vtable->on_channel_handler_installed(&connection->channel_handler, slot); tester->client_connection = connection; - testing_channel_drain_queued_tasks(tester->testing_channel); return AWS_OP_SUCCESS; } @@ -309,7 +309,7 @@ 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"; + response_string = "HTTP/1.0 200 Connection established\r\nconnection: close\r\n"; } /* send response */ diff --git a/tests/test_h1_client.c b/tests/test_h1_client.c index 5f038665e..ec7141faf 100644 --- a/tests/test_h1_client.c +++ b/tests/test_h1_client.c @@ -71,7 +71,7 @@ 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); + 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 +1501,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)); diff --git a/tests/test_proxy.c b/tests/test_proxy.c index 92d257cf7..ca1c6e877 100644 --- a/tests/test_proxy.c +++ b/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_UINT_EQUALS(AWS_ERROR_SUCCESS, tester.wait_result); ASSERT_SUCCESS(proxy_tester_clean_up(&tester)); diff --git a/tests/test_websocket_handler.c b/tests/test_websocket_handler.c index b5a7c377d..9fd732a8e 100644 --- a/tests/test_websocket_handler.c +++ b/tests/test_websocket_handler.c @@ -24,6 +24,8 @@ # pragma warning(disable : 4204) /* non-constant aggregate initializer */ #endif +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 +489,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, @@ -495,6 +497,7 @@ static int s_tester_init(struct tester *tester, struct aws_allocator *alloc) { .manual_window_update = s_tester_options.manual_window_update, }; tester->websocket = aws_websocket_handler_new(&ws_options); + testing_channel_drain_queued_tasks(&tester->testing_channel); ASSERT_NOT_NULL(tester->websocket); aws_websocket_decoder_init(&tester->written_frame_decoder, s_on_written_frame, s_on_written_frame_payload, tester); @@ -530,6 +533,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 +1698,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 +1728,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 +1737,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 +1755,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 +1780,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 +1807,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 +1828,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[] = { From 5c11e639cf78a893180640e46175d3dc75ad10a5 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Wed, 11 Mar 2020 20:13:24 -0700 Subject: [PATCH 06/11] addressed PR feedback. --- include/aws/http/private/h1_encoder.h | 1 - include/aws/http/private/http_impl.h | 2 ++ source/h1_connection.c | 23 +++++++---------------- source/h1_encoder.c | 4 ---- source/http.c | 1 + tests/proxy_test_helper.c | 4 +++- tests/test_h1_client.c | 2 ++ tests/test_websocket_handler.c | 4 +++- 8 files changed, 18 insertions(+), 23 deletions(-) diff --git a/include/aws/http/private/h1_encoder.h b/include/aws/http/private/h1_encoder.h index e8df09ac4..27090a760 100644 --- a/include/aws/http/private/h1_encoder.h +++ b/include/aws/http/private/h1_encoder.h @@ -28,7 +28,6 @@ struct aws_h1_encoder_message { struct aws_input_stream *body; uint64_t content_length; bool has_connection_close_header; - bool ignore_response_connection_close_header; }; enum aws_h1_encoder_state { diff --git a/include/aws/http/private/http_impl.h b/include/aws/http/private/http_impl.h index 03cd5800d..0cfff6a0b 100644 --- a/include/aws/http/private/http_impl.h +++ b/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, }; diff --git a/source/h1_connection.c b/source/h1_connection.c index 868645703..731224139 100644 --- a/source/h1_connection.c +++ b/source/h1_connection.c @@ -182,10 +182,6 @@ struct h1_connection { /* If non-zero, then window_update_task is scheduled */ size_t window_update_size; - - /* for bug for bug compatibility with L7 proxies. They can send connection:close on successful connect - * responses. This should be ignored. */ - bool ignore_response_connection_close_header; } synced_data; }; @@ -446,9 +442,6 @@ struct aws_http_stream *s_make_request( { /* BEGIN CRITICAL SECTION */ s_h1_connection_lock_synced_data(connection); - connection->synced_data.ignore_response_connection_close_header = - stream->encoder_message.ignore_response_connection_close_header; - if (connection->synced_data.new_stream_error_code) { new_stream_error_code = connection->synced_data.new_stream_error_code; } else { @@ -992,15 +985,13 @@ 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) { - bool ignore_connection_close = false; - { - /*BEGIN CRITICAL SECTION*/ - aws_mutex_lock(&connection->synced_data.lock); - ignore_connection_close = connection->synced_data.ignore_response_connection_close_header; - connection->synced_data.ignore_response_connection_close_header = false; - aws_mutex_unlock(&connection->synced_data.lock); - /*END CRITICAL SECTION*/ - } + /* 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, diff --git a/source/h1_encoder.c b/source/h1_encoder.c index a7699e45a..a76a8ee9e 100644 --- a/source/h1_encoder.c +++ b/source/h1_encoder.c @@ -131,10 +131,6 @@ int aws_h1_encoder_message_init_from_request( goto error; } - if (aws_byte_cursor_eq_c_str_ignore_case(&method, "connect")) { - message->ignore_response_connection_close_header = true; - } - struct aws_byte_cursor uri; err = aws_http_message_get_request_path(request, &uri); if (err) { diff --git a/source/http.c b/source/http.c index 7174778a1..50f18b75e 100644 --- a/source/http.c +++ b/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, diff --git a/tests/proxy_test_helper.c b/tests/proxy_test_helper.c index b119aab1f..676c9fece 100644 --- a/tests/proxy_test_helper.c +++ b/tests/proxy_test_helper.c @@ -265,9 +265,9 @@ int proxy_tester_create_testing_channel_connection(struct proxy_tester *tester) ASSERT_NOT_NULL(slot); 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); - connection->vtable->on_channel_handler_installed(&connection->channel_handler, slot); tester->client_connection = connection; return AWS_OP_SUCCESS; @@ -309,6 +309,8 @@ 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 { + /* 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"; } diff --git a/tests/test_h1_client.c b/tests/test_h1_client.c index ec7141faf..7c7c9b4af 100644 --- a/tests/test_h1_client.c +++ b/tests/test_h1_client.c @@ -71,6 +71,8 @@ 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)); + /* 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); diff --git a/tests/test_websocket_handler.c b/tests/test_websocket_handler.c index 9fd732a8e..5232642fa 100644 --- a/tests/test_websocket_handler.c +++ b/tests/test_websocket_handler.c @@ -24,6 +24,8 @@ # 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) \ @@ -497,8 +499,8 @@ static int s_tester_init(struct tester *tester, struct aws_allocator *alloc) { .manual_window_update = s_tester_options.manual_window_update, }; tester->websocket = aws_websocket_handler_new(&ws_options); - testing_channel_drain_queued_tasks(&tester->testing_channel); 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); From 7bd7f88df754473808b83b0aebf6073b5ac8a8f7 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Wed, 11 Mar 2020 20:16:42 -0700 Subject: [PATCH 07/11] Added another comment to the test for small initial window sizes. --- tests/proxy_test_helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/proxy_test_helper.c b/tests/proxy_test_helper.c index 676c9fece..f1ca5088e 100644 --- a/tests/proxy_test_helper.c +++ b/tests/proxy_test_helper.c @@ -254,6 +254,8 @@ 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; + /* 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); From a372cf28577d1337f364d4561f4300f12d1409ea Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 12 Mar 2020 16:29:51 -0700 Subject: [PATCH 08/11] Unbreak tls failure proxy test --- tests/proxy_test_helper.c | 2 +- tests/test_proxy.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/proxy_test_helper.c b/tests/proxy_test_helper.c index f1ca5088e..d71d30111 100644 --- a/tests/proxy_test_helper.c +++ b/tests/proxy_test_helper.c @@ -313,7 +313,7 @@ int proxy_tester_send_connect_response(struct proxy_tester *tester) { } else { /* 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"; + response_string = "HTTP/1.0 200 Connection established\r\nconnection: close\r\n\r\n"; } /* send response */ diff --git a/tests/test_proxy.c b/tests/test_proxy.c index ca1c6e877..f1b7e0a1e 100644 --- a/tests/test_proxy.c +++ b/tests/test_proxy.c @@ -327,7 +327,7 @@ 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_NULL(tester.client_connection); - ASSERT_UINT_EQUALS(AWS_ERROR_SUCCESS, tester.wait_result); + ASSERT_TRUE(AWS_ERROR_SUCCESS != tester.wait_result); ASSERT_SUCCESS(proxy_tester_clean_up(&tester)); From e672b1ca2a45f469abbc2e712c0743107040e1e2 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 12 Mar 2020 16:33:03 -0700 Subject: [PATCH 09/11] Use good CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5ad78e0eb..32dc9afb9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: - '!master' env: - BUILDER_VERSION: v0.5.4 + BUILDER_VERSION: v0.5.7 BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-http LINUX_BASE_IMAGE: ubuntu-16-x64 From 6c8e1b3d34b13ec1ac4ad1fa41bbdeff0e7635ae Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Thu, 12 Mar 2020 17:48:43 -0700 Subject: [PATCH 10/11] Fix race condition in test. --- tests/test_connection.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_connection.c b/tests/test_connection.c index 32f31cdb8..3626a60de 100644 --- a/tests/test_connection.c +++ b/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); } From 3c0525ed51fb15bcbb9c7fdf143c81e87c7477f9 Mon Sep 17 00:00:00 2001 From: "Jonathan M. Henson" Date: Fri, 13 Mar 2020 10:37:20 -0700 Subject: [PATCH 11/11] Update builder version. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32dc9afb9..aeb055c7d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: - '!master' env: - BUILDER_VERSION: v0.5.7 + BUILDER_VERSION: v0.5.8 BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-http LINUX_BASE_IMAGE: ubuntu-16-x64