diff --git a/src/ruby/end2end/call_credentials_returning_bad_metadata_doesnt_kill_background_thread_driver.rb b/src/ruby/end2end/call_credentials_returning_bad_metadata_doesnt_kill_background_thread_driver.rb new file mode 100755 index 00000000000..414e97522ca --- /dev/null +++ b/src/ruby/end2end/call_credentials_returning_bad_metadata_doesnt_kill_background_thread_driver.rb @@ -0,0 +1,160 @@ +#!/usr/bin/env ruby +# +# Copyright 2020 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. + +this_dir = File.expand_path(File.dirname(__FILE__)) +protos_lib_dir = File.join(this_dir, 'lib') +grpc_lib_dir = File.join(File.dirname(this_dir), 'lib') +$LOAD_PATH.unshift(grpc_lib_dir) unless $LOAD_PATH.include?(grpc_lib_dir) +$LOAD_PATH.unshift(protos_lib_dir) unless $LOAD_PATH.include?(protos_lib_dir) +$LOAD_PATH.unshift(this_dir) unless $LOAD_PATH.include?(this_dir) + +require 'grpc' +require 'end2end_common' + +def create_channel_creds + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + files = ['ca.pem', 'client.key', 'client.pem'] + creds = files.map { |f| File.open(File.join(test_root, f)).read } + GRPC::Core::ChannelCredentials.new(creds[0], creds[1], creds[2]) +end + +def client_cert + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + cert = File.open(File.join(test_root, 'client.pem')).read + fail unless cert.is_a?(String) + cert +end + +def create_server_creds + test_root = File.join(File.dirname(__FILE__), '..', 'spec', 'testdata') + GRPC.logger.info("test root: #{test_root}") + files = ['ca.pem', 'server1.key', 'server1.pem'] + creds = files.map { |f| File.open(File.join(test_root, f)).read } + GRPC::Core::ServerCredentials.new( + creds[0], + [{ private_key: creds[1], cert_chain: creds[2] }], + true) # force client auth +end + +# Useful to update a value within a do block +class MutableValue + attr_accessor :value + + def initialize(value) + @value = value + end +end + +def run_rpc_expect_unavailable(stub) + exception = nil + begin + stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + rescue GRPC::BadStatus => e + exception = e + end + fail "Expected call to fail UNAVAILABLE. Got failure:|#{exception}|" unless exception.is_a?(GRPC::Unavailable) +end + +# rubocop:disable Metrics/AbcSize +# rubocop:disable Metrics/MethodLength +def main + server_runner = ServerRunner.new(EchoServerImpl) + server_runner.server_creds = create_server_creds + server_port = server_runner.run + channel_args = { + GRPC::Core::Channel::SSL_TARGET => 'foo.test.google.fr' + } + call_creds_invocation_count = MutableValue.new(0) + call_creds_invocation_count_mu = Mutex.new + empty_header_proc = proc do + call_creds_invocation_count_mu.synchronize do + call_creds_invocation_count.value += 1 + end + # Empty header values are invalid, so returning one here should + # raise cause grpc-ruby to fail the RPC. + { '' => '123' } + end + bad_type_proc = proc do + call_creds_invocation_count_mu.synchronize do + call_creds_invocation_count.value += 1 + end + # Returning bad type here (not nil or Hash) should cause the RPC to fail + 1 + end + nil_proc = proc do + call_creds_invocation_count_mu.synchronize do + call_creds_invocation_count.value += 1 + end + # The RPC should succeed in this case, but just not add any headers + nil + end + raising_proc = proc do + call_creds_invocation_count_mu.synchronize do + call_creds_invocation_count.value += 1 + end + # The RPC should fail in this case + raise 'exception thrown by raising_proc call creds' + end + good_header_proc = proc do + call_creds_invocation_count_mu.synchronize do + call_creds_invocation_count.value += 1 + end + { 'authorization' => 'fake_val' } + end + empty_header_stub = Echo::EchoServer::Stub.new( + "localhost:#{server_port}", + create_channel_creds.compose(GRPC::Core::CallCredentials.new(empty_header_proc)), + channel_args: channel_args) + bad_type_stub = Echo::EchoServer::Stub.new( + "localhost:#{server_port}", + create_channel_creds.compose(GRPC::Core::CallCredentials.new(bad_type_proc)), + channel_args: channel_args) + nil_stub = Echo::EchoServer::Stub.new( + "localhost:#{server_port}", + create_channel_creds.compose(GRPC::Core::CallCredentials.new(nil_proc)), + channel_args: channel_args) + raising_stub = Echo::EchoServer::Stub.new( + "localhost:#{server_port}", + create_channel_creds.compose(GRPC::Core::CallCredentials.new(raising_proc)), + channel_args: channel_args) + good_stub = Echo::EchoServer::Stub.new( + "localhost:#{server_port}", + create_channel_creds.compose(GRPC::Core::CallCredentials.new(good_header_proc)), + channel_args: channel_args) + STDERR.puts 'perform an RPC using call creds that return valid headers and expect OK...' + good_stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + STDERR.puts 'perform an RPC using call creds that return an empty header and expect it to fail...' + run_rpc_expect_unavailable(empty_header_stub) + STDERR.puts 'perform an RPC using call creds that return a bad type and expect it to fail...' + run_rpc_expect_unavailable(bad_type_stub) + STDERR.puts 'perform an RPC using call creds that return nil and expect OK...' + nil_stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + STDERR.puts 'perform an RPC using call creds that raise an error and expect it to fail...' + run_rpc_expect_unavailable(raising_stub) + STDERR.puts 'perform an RPC using call creds that return valid headers and expect OK...' + # Note that the purpose of this RPC is to test that the bad call creds used by the previous + # RPCs didn't get the gRPC-ruby library into a wedged state (specifically by killing + # the call credentials user callback invocation thread). + good_stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 10) + server_runner.stop + call_creds_invocation_count_mu.synchronize do + unless call_creds_invocation_count.value == 6 + fail 'test did not actually use the call credentials' + end + end +end + +main diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index fe6158183d8..117ab63e9af 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -48,7 +48,7 @@ static VALUE grpc_rb_sBatchResult; /* grpc_rb_cMdAry is the MetadataArray class whose instances proxy * grpc_metadata_array. */ -static VALUE grpc_rb_cMdAry; +VALUE grpc_rb_cMdAry; /* id_credentials is the name of the hidden ivar that preserves the value * of the credentials added to the call */ @@ -103,7 +103,7 @@ static void grpc_rb_call_destroy(void* p) { xfree(p); } -static const rb_data_type_t grpc_rb_md_ary_data_type = { +const rb_data_type_t grpc_rb_md_ary_data_type = { "grpc_metadata_array", {GRPC_RB_GC_NOT_MARKED, GRPC_RB_GC_DONT_FREE, @@ -489,6 +489,7 @@ static int grpc_rb_md_ary_capacity_hash_cb(VALUE key, VALUE val, /* grpc_rb_md_ary_convert converts a ruby metadata hash into a grpc_metadata_array. + Note that this function may throw exceptions. */ void grpc_rb_md_ary_convert(VALUE md_ary_hash, grpc_metadata_array* md_ary) { VALUE md_ary_obj = Qnil; diff --git a/src/ruby/ext/grpc/rb_call.h b/src/ruby/ext/grpc/rb_call.h index a2202eb8d31..790422169bc 100644 --- a/src/ruby/ext/grpc/rb_call.h +++ b/src/ruby/ext/grpc/rb_call.h @@ -23,6 +23,10 @@ #include +extern const rb_data_type_t grpc_rb_md_ary_data_type; + +extern VALUE grpc_rb_cMdAry; + /* Gets the wrapped call from a VALUE. */ grpc_call* grpc_rb_get_wrapped_call(VALUE v); diff --git a/src/ruby/ext/grpc/rb_call_credentials.c b/src/ruby/ext/grpc/rb_call_credentials.c index 6ca9c060898..76e5159cb6e 100644 --- a/src/ruby/ext/grpc/rb_call_credentials.c +++ b/src/ruby/ext/grpc/rb_call_credentials.c @@ -54,32 +54,41 @@ typedef struct callback_params { grpc_credentials_plugin_metadata_cb callback; } callback_params; -static VALUE grpc_rb_call_credentials_callback(VALUE callback_args) { +static VALUE grpc_rb_call_credentials_callback(VALUE args) { VALUE result = rb_hash_new(); + VALUE callback_func = rb_ary_entry(args, 0); + VALUE callback_args = rb_ary_entry(args, 1); + VALUE md_ary_obj = rb_ary_entry(args, 2); if (gpr_should_log(GPR_LOG_SEVERITY_DEBUG)) { - VALUE callback_args_as_str = - rb_funcall(callback_args, rb_intern("to_s"), 0); - VALUE callback_source_info = rb_funcall(rb_ary_entry(callback_args, 0), - rb_intern("source_location"), 0); + VALUE callback_func_str = rb_funcall(callback_func, rb_intern("to_s"), 0); + VALUE callback_args_str = rb_funcall(callback_args, rb_intern("to_s"), 0); + VALUE callback_source_info = + rb_funcall(callback_func, rb_intern("source_location"), 0); if (callback_source_info != Qnil) { VALUE source_filename = rb_ary_entry(callback_source_info, 0); VALUE source_line_number = rb_funcall( rb_ary_entry(callback_source_info, 1), rb_intern("to_s"), 0); gpr_log(GPR_DEBUG, - "GRPC_RUBY: grpc_rb_call_credentials invoking user callback " - "(source_filename:%s line_number:%s) with arguments:%s", + "GRPC_RUBY: grpc_rb_call_credentials invoking user callback:|%s| " + "source_filename:%s line_number:%s with arguments:|%s|", + StringValueCStr(callback_func_str), StringValueCStr(source_filename), StringValueCStr(source_line_number), - StringValueCStr(callback_args_as_str)); + StringValueCStr(callback_args_str)); } else { gpr_log(GPR_DEBUG, - "GRPC_RUBY: grpc_rb_call_credentials invoking user callback " - "(failed to get source filename ane line) with arguments:%s", - StringValueCStr(callback_args_as_str)); + "GRPC_RUBY: grpc_rb_call_credentials invoking user callback:|%s| " + "(failed to get source filename and line) with arguments:|%s|", + StringValueCStr(callback_func_str), + StringValueCStr(callback_args_str)); } } - VALUE metadata = rb_funcall(rb_ary_entry(callback_args, 0), rb_intern("call"), - 1, rb_ary_entry(callback_args, 1)); + VALUE metadata = + rb_funcall(callback_func, rb_intern("call"), 1, callback_args); + grpc_metadata_array* md_ary = NULL; + TypedData_Get_Struct(md_ary_obj, grpc_metadata_array, + &grpc_rb_md_ary_data_type, md_ary); + grpc_rb_md_ary_convert(metadata, md_ary); rb_hash_aset(result, rb_str_new2("metadata"), metadata); rb_hash_aset(result, rb_str_new2("status"), INT2NUM(GRPC_STATUS_OK)); rb_hash_aset(result, rb_str_new2("details"), rb_str_new2("")); @@ -89,14 +98,23 @@ static VALUE grpc_rb_call_credentials_callback(VALUE callback_args) { static VALUE grpc_rb_call_credentials_callback_rescue(VALUE args, VALUE exception_object) { VALUE result = rb_hash_new(); - VALUE backtrace = - rb_funcall(rb_funcall(exception_object, rb_intern("backtrace"), 0), - rb_intern("join"), 1, rb_str_new2("\n\tfrom ")); + VALUE backtrace = rb_funcall(exception_object, rb_intern("backtrace"), 0); + VALUE backtrace_str; + if (backtrace != Qnil) { + backtrace_str = + rb_funcall(backtrace, rb_intern("join"), 1, rb_str_new2("\n\tfrom ")); + } else { + backtrace_str = rb_str_new2( + "failed to get backtrace, this exception was likely thrown from native " + "code"); + } VALUE rb_exception_info = rb_funcall(exception_object, rb_intern("inspect"), 0); (void)args; - gpr_log(GPR_INFO, "Call credentials callback failed: %s\n%s", - StringValueCStr(rb_exception_info), StringValueCStr(backtrace)); + gpr_log(GPR_INFO, + "GRPC_RUBY call credentials callback failed, exception inspect:|%s| " + "backtrace:|%s|", + StringValueCStr(rb_exception_info), StringValueCStr(backtrace_str)); rb_hash_aset(result, rb_str_new2("metadata"), Qnil); rb_hash_aset(result, rb_str_new2("status"), INT2NUM(GRPC_STATUS_UNAUTHENTICATED)); @@ -120,11 +138,15 @@ static void grpc_rb_call_credentials_callback_with_gil(void* param) { rb_hash_aset(args, ID2SYM(rb_intern("jwt_aud_uri")), auth_uri); rb_ary_push(callback_args, params->get_metadata); rb_ary_push(callback_args, args); + // Wrap up the grpc_metadata_array into a ruby object and do the conversion + // from hash to grpc_metadata_array within the rescue block, because the + // conversion can throw exceptions. + rb_ary_push(callback_args, + TypedData_Wrap_Struct(grpc_rb_cMdAry, &grpc_rb_md_ary_data_type, + &md_ary)); result = rb_rescue(grpc_rb_call_credentials_callback, callback_args, grpc_rb_call_credentials_callback_rescue, Qnil); // Both callbacks return a hash, so result should be a hash - grpc_rb_md_ary_convert(rb_hash_aref(result, rb_str_new2("metadata")), - &md_ary); status = NUM2INT(rb_hash_aref(result, rb_str_new2("status"))); details = rb_hash_aref(result, rb_str_new2("details")); error_details = StringValueCStr(details); diff --git a/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh b/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh index 304814c2553..fe552ce318e 100755 --- a/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh +++ b/tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh @@ -36,4 +36,5 @@ time ruby src/ruby/end2end/errors_load_before_grpc_lib.rb || EXIT_CODE=1 time ruby src/ruby/end2end/logger_load_before_grpc_lib.rb || EXIT_CODE=1 time ruby src/ruby/end2end/status_codes_load_before_grpc_lib.rb || EXIT_CODE=1 time ruby src/ruby/end2end/call_credentials_timeout_driver.rb || EXIT_CODE=1 +time ruby src/ruby/end2end/call_credentials_returning_bad_metadata_doesnt_kill_background_thread_driver.rb || EXIT_CODE=1 exit $EXIT_CODE