Merge pull request #23333 from apolcyn/bad_md_drops_call_creds_thread

Make sure call creds user callbacks can't kill the ruby call credentials thread
pull/23391/head
apolcyn 4 years ago committed by GitHub
commit 4cacc60651
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 160
      src/ruby/end2end/call_credentials_returning_bad_metadata_doesnt_kill_background_thread_driver.rb
  2. 5
      src/ruby/ext/grpc/rb_call.c
  3. 4
      src/ruby/ext/grpc/rb_call.h
  4. 62
      src/ruby/ext/grpc/rb_call_credentials.c
  5. 1
      tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh

@ -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

@ -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;

@ -23,6 +23,10 @@
#include <grpc/grpc.h>
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);

@ -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);

@ -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

Loading…
Cancel
Save