diff --git a/CMakeLists.txt b/CMakeLists.txt index e78164e2285..1492eb9b78e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -388,6 +388,7 @@ endif() add_dependencies(buildtests_c public_headers_must_be_c89) add_dependencies(buildtests_c badreq_bad_client_test) add_dependencies(buildtests_c connection_prefix_bad_client_test) +add_dependencies(buildtests_c duplicate_header_bad_client_test) add_dependencies(buildtests_c head_of_line_blocking_bad_client_test) add_dependencies(buildtests_c headers_bad_client_test) add_dependencies(buildtests_c initial_settings_frame_bad_client_test) @@ -12318,6 +12319,35 @@ target_link_libraries(connection_prefix_bad_client_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(duplicate_header_bad_client_test + test/core/bad_client/tests/duplicate_header.cc +) + + +target_include_directories(duplicate_header_bad_client_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${_gRPC_SSL_INCLUDE_DIR} + PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR} + PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR} + PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR} + PRIVATE ${_gRPC_CARES_INCLUDE_DIR} + PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR} +) + +target_link_libraries(duplicate_header_bad_client_test + ${_gRPC_SSL_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + bad_client_test + grpc_test_util_unsecure + grpc_unsecure + gpr_test_util + gpr +) + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(head_of_line_blocking_bad_client_test test/core/bad_client/tests/head_of_line_blocking.cc ) diff --git a/Makefile b/Makefile index ec9f60b8f47..d379ca56668 100644 --- a/Makefile +++ b/Makefile @@ -1244,6 +1244,7 @@ boringssl_span_test: $(BINDIR)/$(CONFIG)/boringssl_span_test boringssl_ssl_test: $(BINDIR)/$(CONFIG)/boringssl_ssl_test badreq_bad_client_test: $(BINDIR)/$(CONFIG)/badreq_bad_client_test connection_prefix_bad_client_test: $(BINDIR)/$(CONFIG)/connection_prefix_bad_client_test +duplicate_header_bad_client_test: $(BINDIR)/$(CONFIG)/duplicate_header_bad_client_test head_of_line_blocking_bad_client_test: $(BINDIR)/$(CONFIG)/head_of_line_blocking_bad_client_test headers_bad_client_test: $(BINDIR)/$(CONFIG)/headers_bad_client_test initial_settings_frame_bad_client_test: $(BINDIR)/$(CONFIG)/initial_settings_frame_bad_client_test @@ -1496,6 +1497,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/public_headers_must_be_c89 \ $(BINDIR)/$(CONFIG)/badreq_bad_client_test \ $(BINDIR)/$(CONFIG)/connection_prefix_bad_client_test \ + $(BINDIR)/$(CONFIG)/duplicate_header_bad_client_test \ $(BINDIR)/$(CONFIG)/head_of_line_blocking_bad_client_test \ $(BINDIR)/$(CONFIG)/headers_bad_client_test \ $(BINDIR)/$(CONFIG)/initial_settings_frame_bad_client_test \ @@ -2048,6 +2050,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/badreq_bad_client_test || ( echo test badreq_bad_client_test failed ; exit 1 ) $(E) "[RUN] Testing connection_prefix_bad_client_test" $(Q) $(BINDIR)/$(CONFIG)/connection_prefix_bad_client_test || ( echo test connection_prefix_bad_client_test failed ; exit 1 ) + $(E) "[RUN] Testing duplicate_header_bad_client_test" + $(Q) $(BINDIR)/$(CONFIG)/duplicate_header_bad_client_test || ( echo test duplicate_header_bad_client_test failed ; exit 1 ) $(E) "[RUN] Testing head_of_line_blocking_bad_client_test" $(Q) $(BINDIR)/$(CONFIG)/head_of_line_blocking_bad_client_test || ( echo test head_of_line_blocking_bad_client_test failed ; exit 1 ) $(E) "[RUN] Testing headers_bad_client_test" @@ -20101,6 +20105,26 @@ ifneq ($(NO_DEPS),true) endif +DUPLICATE_HEADER_BAD_CLIENT_TEST_SRC = \ + test/core/bad_client/tests/duplicate_header.cc \ + +DUPLICATE_HEADER_BAD_CLIENT_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(DUPLICATE_HEADER_BAD_CLIENT_TEST_SRC)))) + + +$(BINDIR)/$(CONFIG)/duplicate_header_bad_client_test: $(DUPLICATE_HEADER_BAD_CLIENT_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(DUPLICATE_HEADER_BAD_CLIENT_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) -o $(BINDIR)/$(CONFIG)/duplicate_header_bad_client_test + +$(OBJDIR)/$(CONFIG)/test/core/bad_client/tests/duplicate_header.o: $(LIBDIR)/$(CONFIG)/libbad_client_test.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util_unsecure.a $(LIBDIR)/$(CONFIG)/libgrpc_unsecure.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_duplicate_header_bad_client_test: $(DUPLICATE_HEADER_BAD_CLIENT_TEST_OBJS:.o=.dep) + +ifneq ($(NO_DEPS),true) +-include $(DUPLICATE_HEADER_BAD_CLIENT_TEST_OBJS:.o=.dep) +endif + + HEAD_OF_LINE_BLOCKING_BAD_CLIENT_TEST_SRC = \ test/core/bad_client/tests/head_of_line_blocking.cc \ diff --git a/build.yaml b/build.yaml index 829081fd108..f688f9ef824 100644 --- a/build.yaml +++ b/build.yaml @@ -13,7 +13,7 @@ settings: '#09': Per-language overrides are possible with (eg) ruby_version tag here '#10': See the expand_version.py for all the quirks here core_version: 6.0.0-dev - g_stands_for: glossy + g_stands_for: glamorous version: 1.10.0-dev filegroups: - name: census diff --git a/requirements.txt b/requirements.txt index c976cef4cb8..53768c6822f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ # GRPC Python setup requirements coverage>=4.0 -cython>=0.23 +cython>=0.27 enum34>=1.0.4 futures>=2.2.0 protobuf>=3.5.0.post1 diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 49522ef3e47..6b936444307 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1095,6 +1095,7 @@ static void pick_callback_done_locked(void* arg, grpc_error* error) { chand, calld); } async_pick_done_locked(elem, GRPC_ERROR_REF(error)); + GRPC_CALL_STACK_UNREF(calld->owning_call, "pick_callback"); } // Takes a ref to chand->lb_policy and calls grpc_lb_policy_pick_locked(). @@ -1134,6 +1135,7 @@ static bool pick_callback_start_locked(grpc_call_element* elem) { GRPC_CLOSURE_INIT(&calld->lb_pick_closure, pick_callback_done_locked, elem, grpc_combiner_scheduler(chand->combiner)); calld->pick.on_complete = &calld->lb_pick_closure; + GRPC_CALL_STACK_REF(calld->owning_call, "pick_callback"); const bool pick_done = grpc_lb_policy_pick_locked(chand->lb_policy, &calld->pick); if (pick_done) { @@ -1142,6 +1144,7 @@ static bool pick_callback_start_locked(grpc_call_element* elem) { gpr_log(GPR_DEBUG, "chand=%p calld=%p: pick completed synchronously", chand, calld); } + GRPC_CALL_STACK_UNREF(calld->owning_call, "pick_callback"); } else { GRPC_CALL_STACK_REF(calld->owning_call, "pick_callback_cancel"); grpc_call_combiner_set_notify_on_cancel( diff --git a/src/core/ext/filters/max_age/max_age_filter.cc b/src/core/ext/filters/max_age/max_age_filter.cc index 7b86e4cd6c1..a3f9780f3f6 100644 --- a/src/core/ext/filters/max_age/max_age_filter.cc +++ b/src/core/ext/filters/max_age/max_age_filter.cc @@ -37,6 +37,12 @@ #define MAX_CONNECTION_IDLE_INTEGER_OPTIONS \ { DEFAULT_MAX_CONNECTION_IDLE_MS, 1, INT_MAX } +/* States for idle_state in channel_data */ +#define MAX_IDLE_STATE_INIT ((gpr_atm)0) +#define MAX_IDLE_STATE_SEEN_EXIT_IDLE ((gpr_atm)1) +#define MAX_IDLE_STATE_SEEN_ENTER_IDLE ((gpr_atm)2) +#define MAX_IDLE_STATE_TIMER_SET ((gpr_atm)3) + namespace { struct channel_data { /* We take a reference to the channel stack for the timer callback */ @@ -64,7 +70,7 @@ struct channel_data { grpc_millis max_connection_age_grace; /* Closure to run when the channel's idle duration reaches max_connection_idle and should be closed gracefully */ - grpc_closure close_max_idle_channel; + grpc_closure max_idle_timer_cb; /* Closure to run when the channel reaches its max age and should be closed gracefully */ grpc_closure close_max_age_channel; @@ -85,26 +91,117 @@ struct channel_data { grpc_connectivity_state connectivity_state; /* Number of active calls */ gpr_atm call_count; + /* TODO(zyc): C++lize this state machine */ + /* 'idle_state' holds the states of max_idle_timer and channel idleness. + It can contain one of the following values: + +--------------------------------+----------------+---------+ + | idle_state | max_idle_timer | channel | + +--------------------------------+----------------+---------+ + | MAX_IDLE_STATE_INIT | unset | busy | + | MAX_IDLE_STATE_TIMER_SET | set, valid | idle | + | MAX_IDLE_STATE_SEEN_EXIT_IDLE | set, invalid | busy | + | MAX_IDLE_STATE_SEEN_ENTER_IDLE | set, invalid | idle | + +--------------------------------+----------------+---------+ + + MAX_IDLE_STATE_INIT: The initial and final state of 'idle_state'. The + channel has 1 or 1+ active calls, and the the timer is not set. Note that + we may put a virtual call to hold this state at channel initialization or + shutdown, so that the channel won't enter other states. + + MAX_IDLE_STATE_TIMER_SET: The state after the timer is set and no calls + have arrived after the timer is set. The channel must have 0 active call in + this state. If the timer is fired in this state, we will close the channel + due to idleness. + + MAX_IDLE_STATE_SEEN_EXIT_IDLE: The state after the timer is set and at + least one call has arrived after the timer is set. The channel must have 1 + or 1+ active calls in this state. If the timer is fired in this state, we + won't reschudle it. + + MAX_IDLE_STATE_SEEN_ENTER_IDLE: The state after the timer is set and the at + least one call has arrived after the timer is set, BUT the channel + currently has 1 or 1+ active calls. If the timer is fired in this state, we + will reschudle it. + + max_idle_timer will not be cancelled (unless the channel is shutting down). + If the timer callback is called when the max_idle_timer is valid (i.e. + idle_state is MAX_IDLE_STATE_TIMER_SET), the channel will be closed due to + idleness, otherwise the channel won't be changed. + + State transitions: + MAX_IDLE_STATE_INIT <-------3------ MAX_IDLE_STATE_SEEN_EXIT_IDLE + ^ | ^ ^ | + | | | | | + 1 2 +-----------4------------+ 6 7 + | | | | | + | v | | v + MAX_IDLE_STATE_TIMER_SET <----5------ MAX_IDLE_STATE_SEEN_ENTER_IDLE + + For 1, 3, 5 : See max_idle_timer_cb() function + For 2, 7 : See decrease_call_count() function + For 4, 6 : See increase_call_count() function */ + gpr_atm idle_state; + /* Time when the channel finished its last outstanding call, in grpc_millis */ + gpr_atm last_enter_idle_time_millis; }; } // namespace /* Increase the nubmer of active calls. Before the increasement, if there are no calls, the max_idle_timer should be cancelled. */ static void increase_call_count(channel_data* chand) { + /* Exit idle */ if (gpr_atm_full_fetch_add(&chand->call_count, 1) == 0) { - grpc_timer_cancel(&chand->max_idle_timer); + while (true) { + gpr_atm idle_state = gpr_atm_acq_load(&chand->idle_state); + switch (idle_state) { + case MAX_IDLE_STATE_TIMER_SET: + /* max_idle_timer_cb may have already set idle_state to + MAX_IDLE_STATE_INIT, in this case, we don't need to set it to + MAX_IDLE_STATE_SEEN_EXIT_IDLE */ + gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_TIMER_SET, + MAX_IDLE_STATE_SEEN_EXIT_IDLE); + return; + case MAX_IDLE_STATE_SEEN_ENTER_IDLE: + gpr_atm_rel_store(&chand->idle_state, MAX_IDLE_STATE_SEEN_EXIT_IDLE); + return; + default: + /* try again */ + break; + } + } } } /* Decrease the nubmer of active calls. After the decrement, if there are no calls, the max_idle_timer should be started. */ static void decrease_call_count(channel_data* chand) { + /* Enter idle */ if (gpr_atm_full_fetch_add(&chand->call_count, -1) == 1) { - GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_idle_timer"); - grpc_timer_init( - &chand->max_idle_timer, - grpc_core::ExecCtx::Get()->Now() + chand->max_connection_idle, - &chand->close_max_idle_channel); + gpr_atm_no_barrier_store(&chand->last_enter_idle_time_millis, + (gpr_atm)grpc_core::ExecCtx::Get()->Now()); + while (true) { + gpr_atm idle_state = gpr_atm_acq_load(&chand->idle_state); + switch (idle_state) { + case MAX_IDLE_STATE_INIT: + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age max_idle_timer"); + grpc_timer_init( + &chand->max_idle_timer, + grpc_core::ExecCtx::Get()->Now() + chand->max_connection_idle, + &chand->max_idle_timer_cb); + gpr_atm_rel_store(&chand->idle_state, MAX_IDLE_STATE_TIMER_SET); + return; + case MAX_IDLE_STATE_SEEN_EXIT_IDLE: + if (gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_EXIT_IDLE, + MAX_IDLE_STATE_SEEN_ENTER_IDLE)) { + return; + } + break; + default: + /* try again */ + break; + } + } } } @@ -152,20 +249,58 @@ static void start_max_age_grace_timer_after_goaway_op(void* arg, "max_age start_max_age_grace_timer_after_goaway_op"); } -static void close_max_idle_channel(void* arg, grpc_error* error) { +static void close_max_idle_channel(channel_data* chand) { + /* Prevent the max idle timer from being set again */ + gpr_atm_no_barrier_fetch_add(&chand->call_count, 1); + grpc_transport_op* op = grpc_make_transport_op(nullptr); + op->goaway_error = + grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_idle"), + GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); + grpc_channel_element* elem = + grpc_channel_stack_element(chand->channel_stack, 0); + elem->filter->start_transport_op(elem, op); +} + +static void max_idle_timer_cb(void* arg, grpc_error* error) { channel_data* chand = (channel_data*)arg; if (error == GRPC_ERROR_NONE) { - /* Prevent the max idle timer from being set again */ - gpr_atm_no_barrier_fetch_add(&chand->call_count, 1); - grpc_transport_op* op = grpc_make_transport_op(nullptr); - op->goaway_error = - grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_idle"), - GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); - grpc_channel_element* elem = - grpc_channel_stack_element(chand->channel_stack, 0); - elem->filter->start_transport_op(elem, op); - } else if (error != GRPC_ERROR_CANCELLED) { - GRPC_LOG_IF_ERROR("close_max_idle_channel", error); + bool try_again = true; + while (try_again) { + gpr_atm idle_state = gpr_atm_acq_load(&chand->idle_state); + switch (idle_state) { + case MAX_IDLE_STATE_TIMER_SET: + close_max_idle_channel(chand); + /* This MAX_IDLE_STATE_INIT is a final state, we don't have to check + * if idle_state has been changed */ + gpr_atm_rel_store(&chand->idle_state, MAX_IDLE_STATE_INIT); + try_again = false; + break; + case MAX_IDLE_STATE_SEEN_EXIT_IDLE: + if (gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_EXIT_IDLE, + MAX_IDLE_STATE_INIT)) { + try_again = false; + } + break; + case MAX_IDLE_STATE_SEEN_ENTER_IDLE: + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age max_idle_timer"); + grpc_timer_init(&chand->max_idle_timer, + (grpc_millis)gpr_atm_no_barrier_load( + &chand->last_enter_idle_time_millis) + + chand->max_connection_idle, + &chand->max_idle_timer_cb); + /* idle_state may have already been set to + MAX_IDLE_STATE_SEEN_EXIT_IDLE by increase_call_count(), in this + case, we don't need to set it to MAX_IDLE_STATE_TIMER_SET */ + gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_ENTER_IDLE, + MAX_IDLE_STATE_TIMER_SET); + try_again = false; + break; + default: + /* try again */ + break; + } + } } GRPC_CHANNEL_STACK_UNREF(chand->channel_stack, "max_age max_idle_timer"); } @@ -288,6 +423,9 @@ static grpc_error* init_channel_elem(grpc_channel_element* elem, chand->max_connection_idle = DEFAULT_MAX_CONNECTION_IDLE_MS == INT_MAX ? GRPC_MILLIS_INF_FUTURE : DEFAULT_MAX_CONNECTION_IDLE_MS; + chand->idle_state = MAX_IDLE_STATE_INIT; + gpr_atm_no_barrier_store(&chand->last_enter_idle_time_millis, + GRPC_MILLIS_INF_PAST); for (size_t i = 0; i < args->channel_args->num_args; ++i) { if (0 == strcmp(args->channel_args->args[i].key, GRPC_ARG_MAX_CONNECTION_AGE_MS)) { @@ -311,8 +449,8 @@ static grpc_error* init_channel_elem(grpc_channel_element* elem, value == INT_MAX ? GRPC_MILLIS_INF_FUTURE : value; } } - GRPC_CLOSURE_INIT(&chand->close_max_idle_channel, close_max_idle_channel, - chand, grpc_schedule_on_exec_ctx); + GRPC_CLOSURE_INIT(&chand->max_idle_timer_cb, max_idle_timer_cb, chand, + grpc_schedule_on_exec_ctx); GRPC_CLOSURE_INIT(&chand->close_max_age_channel, close_max_age_channel, chand, grpc_schedule_on_exec_ctx); GRPC_CLOSURE_INIT(&chand->force_close_max_age_channel, diff --git a/src/core/lib/security/transport/client_auth_filter.cc b/src/core/lib/security/transport/client_auth_filter.cc index 16814d25988..802503c8686 100644 --- a/src/core/lib/security/transport/client_auth_filter.cc +++ b/src/core/lib/security/transport/client_auth_filter.cc @@ -118,6 +118,7 @@ static void on_credentials_metadata(void* arg, grpc_error* input_error) { grpc_transport_stream_op_batch_finish_with_failure(batch, error, calld->call_combiner); } + GRPC_CALL_STACK_UNREF(calld->owning_call, "get_request_metadata"); } void grpc_auth_metadata_context_build( @@ -208,7 +209,7 @@ static void send_security_metadata(grpc_call_element* elem, chand->auth_context, &calld->auth_md_context); GPR_ASSERT(calld->pollent != nullptr); - + GRPC_CALL_STACK_REF(calld->owning_call, "get_request_metadata"); GRPC_CLOSURE_INIT(&calld->async_result_closure, on_credentials_metadata, batch, grpc_schedule_on_exec_ctx); grpc_error* error = GRPC_ERROR_NONE; @@ -250,6 +251,7 @@ static void on_host_checked(void* arg, grpc_error* error) { calld->call_combiner); gpr_free(error_msg); } + GRPC_CALL_STACK_UNREF(calld->owning_call, "check_call_host"); } static void cancel_check_call_host(void* arg, grpc_error* error) { @@ -312,6 +314,7 @@ static void auth_start_transport_stream_op_batch( } if (calld->have_host) { batch->handler_private.extra_arg = elem; + GRPC_CALL_STACK_REF(calld->owning_call, "check_call_host"); GRPC_CLOSURE_INIT(&calld->async_result_closure, on_host_checked, batch, grpc_schedule_on_exec_ctx); char* call_host = grpc_slice_to_c_string(calld->host); diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index f2096d89375..b538cc02120 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -1032,6 +1032,7 @@ static grpc_stream_compression_algorithm decode_stream_compression( static void publish_app_metadata(grpc_call* call, grpc_metadata_batch* b, int is_trailing) { if (b->list.count == 0) return; + if (is_trailing && call->buffered_metadata[1] == nullptr) return; GPR_TIMER_SCOPE("publish_app_metadata", 0); grpc_metadata_array* dest; grpc_metadata* mdusr; diff --git a/src/core/lib/surface/version.cc b/src/core/lib/surface/version.cc index 153b6e02977..51daad0368f 100644 --- a/src/core/lib/surface/version.cc +++ b/src/core/lib/surface/version.cc @@ -23,4 +23,4 @@ const char* grpc_version_string(void) { return "6.0.0-dev"; } -const char* grpc_g_stands_for(void) { return "glossy"; } +const char* grpc_g_stands_for(void) { return "glamorous"; } diff --git a/src/csharp/Grpc.Core/SourceLink.csproj.include b/src/csharp/Grpc.Core/SourceLink.csproj.include index 02ae79fb893..0ec273f57e6 100755 --- a/src/csharp/Grpc.Core/SourceLink.csproj.include +++ b/src/csharp/Grpc.Core/SourceLink.csproj.include @@ -13,7 +13,7 @@ - + diff --git a/test/core/bad_client/gen_build_yaml.py b/test/core/bad_client/gen_build_yaml.py index a8fd7772161..32afba5b1f5 100755 --- a/test/core/bad_client/gen_build_yaml.py +++ b/test/core/bad_client/gen_build_yaml.py @@ -27,6 +27,7 @@ default_test_options = TestOptions(False, 1.0) BAD_CLIENT_TESTS = { 'badreq': default_test_options, 'connection_prefix': default_test_options._replace(cpu_cost=0.2), + 'duplicate_header': default_test_options, 'headers': default_test_options._replace(cpu_cost=0.2), 'initial_settings_frame': default_test_options._replace(cpu_cost=0.2), 'head_of_line_blocking': default_test_options, diff --git a/test/core/bad_client/generate_tests.bzl b/test/core/bad_client/generate_tests.bzl index b595defb8c3..b846f97181b 100755 --- a/test/core/bad_client/generate_tests.bzl +++ b/test/core/bad_client/generate_tests.bzl @@ -25,6 +25,7 @@ def test_options(): BAD_CLIENT_TESTS = { 'badreq': test_options(), 'connection_prefix': test_options(), + 'duplicate_header': test_options(), 'headers': test_options(), 'initial_settings_frame': test_options(), 'head_of_line_blocking': test_options(), diff --git a/test/core/bad_client/tests/duplicate_header.cc b/test/core/bad_client/tests/duplicate_header.cc new file mode 100644 index 00000000000..0d689bbb7e2 --- /dev/null +++ b/test/core/bad_client/tests/duplicate_header.cc @@ -0,0 +1,134 @@ +/* + * + * Copyright 2018 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "test/core/bad_client/bad_client.h" + +#include + +#include + +#include "src/core/lib/surface/server.h" +#include "test/core/end2end/cq_verifier.h" + +#define PFX_STR \ + "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n" \ + "\x00\x00\x00\x04\x00\x00\x00\x00\x00" /* settings frame */ + +#define HEADER_STR \ + "\x00\x00\xc9\x01\x04\x00\x00\x00\x01" /* headers: generated from \ + simple_request.headers in this \ + directory */ \ + "\x10\x05:path\x08/foo/bar" \ + "\x10\x07:scheme\x04http" \ + "\x10\x07:method\x04POST" \ + "\x10\x0a:authority\x09localhost" \ + "\x10\x0c" \ + "content-type\x10" \ + "application/grpc" \ + "\x10\x14grpc-accept-encoding\x15" \ + "deflate,identity,gzip" \ + "\x10\x02te\x08trailers" \ + "\x10\x0auser-agent\"bad-client grpc-c/0.12.0.0 (linux)" + +#define PAYLOAD_STR \ + "\x00\x00\x20\x00\x00\x00\x00\x00\x01" \ + "\x00\x00\x00\x00" + +static void* tag(intptr_t t) { return (void*)t; } + +static void verifier(grpc_server* server, grpc_completion_queue* cq, + void* registered_method) { + grpc_call_error error; + grpc_call* s; + grpc_call_details call_details; + grpc_byte_buffer* request_payload_recv = nullptr; + grpc_op* op; + grpc_op ops[6]; + cq_verifier* cqv = cq_verifier_create(cq); + grpc_metadata_array request_metadata_recv; + int was_cancelled = 2; + + grpc_call_details_init(&call_details); + grpc_metadata_array_init(&request_metadata_recv); + + error = grpc_server_request_call(server, &s, &call_details, + &request_metadata_recv, cq, cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.host, "localhost")); + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo/bar")); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message.recv_message = &request_payload_recv; + op->flags = 0; + op->reserved = nullptr; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = nullptr; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_UNIMPLEMENTED; + grpc_slice status_details = grpc_slice_from_static_string("xyz"); + op->data.send_status_from_server.status_details = &status_details; + op->flags = 0; + op->reserved = nullptr; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(103), nullptr); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(103), 1); + + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_call_unref(s); + cq_verifier_destroy(cqv); +} + +int main(int argc, char** argv) { + grpc_test_init(argc, argv); + grpc_init(); + + /* Verify that sending multiple headers doesn't segfault */ + GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr, + PFX_STR HEADER_STR HEADER_STR PAYLOAD_STR, 0); + GRPC_RUN_BAD_CLIENT_TEST(verifier, nullptr, + PFX_STR HEADER_STR HEADER_STR HEADER_STR PAYLOAD_STR, + 0); + grpc_shutdown(); + return 0; +} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index e65ce0c13e4..910f3c1c132 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -5116,6 +5116,24 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "bad_client_test", + "gpr", + "gpr_test_util", + "grpc_test_util_unsecure", + "grpc_unsecure" + ], + "headers": [], + "is_filegroup": false, + "language": "c", + "name": "duplicate_header_bad_client_test", + "src": [ + "test/core/bad_client/tests/duplicate_header.cc" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "bad_client_test", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 87c0e260b3e..a1763920441 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4698,6 +4698,32 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "gtest": false, + "language": "c", + "name": "duplicate_header_bad_client_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [], "benchmark": false,