Delay calling plugin_creds callback

reviewable/pr19139/r1
yang-g 6 years ago
parent b7e726a748
commit 384f15ab6e
  1. 48
      src/core/lib/security/credentials/plugin/plugin_credentials.cc
  2. 7
      src/core/lib/security/credentials/plugin/plugin_credentials.h
  3. 2
      src/core/lib/security/transport/client_auth_filter.cc
  4. 88
      test/cpp/end2end/end2end_test.cc

@ -54,15 +54,10 @@ void grpc_plugin_credentials::pending_request_remove_locked(
}
}
// Checks if the request has been cancelled.
// If not, removes it from the pending list, so that it cannot be
// cancelled out from under us.
// When this returns, r->cancelled indicates whether the request was
// cancelled before completion.
void grpc_plugin_credentials::pending_request_complete(pending_request* r) {
GPR_DEBUG_ASSERT(r->creds == this);
gpr_mu_lock(&mu_);
if (!r->cancelled) pending_request_remove_locked(r);
pending_request_remove_locked(r);
gpr_mu_unlock(&mu_);
// Ref to credentials not needed anymore.
Unref();
@ -76,7 +71,8 @@ static grpc_error* process_plugin_result(
char* msg;
gpr_asprintf(&msg, "Getting metadata from plugin failed with error: %s",
error_details);
error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
error = grpc_error_set_int(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg),
GRPC_ERROR_INT_GRPC_STATUS, status);
gpr_free(msg);
} else {
bool seen_illegal_header = false;
@ -95,7 +91,9 @@ static grpc_error* process_plugin_result(
}
}
if (seen_illegal_header) {
error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Illegal metadata");
error = grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Illegal metadata."),
GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_INTERNAL);
} else {
for (size_t i = 0; i < num_md; ++i) {
grpc_mdelem mdelem =
@ -125,19 +123,18 @@ static void plugin_md_request_metadata_ready(void* request,
"asynchronously",
r->creds, r);
}
// Remove request from pending list if not previously cancelled.
// Remove request from pending list
r->creds->pending_request_complete(r);
// If it has not been cancelled, process it.
if (!r->cancelled) {
grpc_error* error =
process_plugin_result(r, md, num_md, status, error_details);
GRPC_CLOSURE_SCHED(r->on_request_metadata, error);
if (r->error == GRPC_ERROR_NONE) {
r->error = process_plugin_result(r, md, num_md, status, error_details);
} else if (GRPC_TRACE_FLAG_ENABLED(grpc_plugin_credentials_trace)) {
gpr_log(GPR_INFO,
"plugin_credentials[%p]: request %p: plugin was previously "
"cancelled",
r->creds, r);
}
GRPC_CLOSURE_SCHED(r->on_request_metadata, r->error);
gpr_free(r);
}
@ -145,11 +142,11 @@ bool grpc_plugin_credentials::get_request_metadata(
grpc_polling_entity* pollent, grpc_auth_metadata_context context,
grpc_credentials_mdelem_array* md_array, grpc_closure* on_request_metadata,
grpc_error** error) {
bool retval = true; // Synchronous return.
if (plugin_.get_metadata != nullptr) {
// Create pending_request object.
pending_request* request =
static_cast<pending_request*>(gpr_zalloc(sizeof(*request)));
request->error = GRPC_ERROR_NONE;
request->creds = this;
request->md_array = md_array;
request->on_request_metadata = on_request_metadata;
@ -183,20 +180,8 @@ bool grpc_plugin_credentials::get_request_metadata(
return false; // Asynchronous return.
}
// Returned synchronously.
// Remove request from pending list if not previously cancelled.
// Remove request from pending list.
request->creds->pending_request_complete(request);
// If the request was cancelled, the error will have been returned
// asynchronously by plugin_cancel_get_request_metadata(), so return
// false. Otherwise, process the result.
if (request->cancelled) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_plugin_credentials_trace)) {
gpr_log(GPR_INFO,
"plugin_credentials[%p]: request %p was cancelled, error "
"will be returned asynchronously",
this, request);
}
retval = false;
} else {
if (GRPC_TRACE_FLAG_ENABLED(grpc_plugin_credentials_trace)) {
gpr_log(GPR_INFO,
"plugin_credentials[%p]: request %p: plugin returned "
@ -205,7 +190,6 @@ bool grpc_plugin_credentials::get_request_metadata(
}
*error = process_plugin_result(request, creds_md, num_creds_md, status,
error_details);
}
// Clean up.
for (size_t i = 0; i < num_creds_md; ++i) {
grpc_slice_unref_internal(creds_md[i].key);
@ -214,7 +198,7 @@ bool grpc_plugin_credentials::get_request_metadata(
gpr_free((void*)error_details);
gpr_free(request);
}
return retval;
return true; // Synchronous return.
}
void grpc_plugin_credentials::cancel_get_request_metadata(
@ -227,15 +211,11 @@ void grpc_plugin_credentials::cancel_get_request_metadata(
gpr_log(GPR_INFO, "plugin_credentials[%p]: cancelling request %p", this,
pending_request);
}
pending_request->cancelled = true;
GRPC_CLOSURE_SCHED(pending_request->on_request_metadata,
GRPC_ERROR_REF(error));
pending_request_remove_locked(pending_request);
pending_request->error = error;
break;
}
}
gpr_mu_unlock(&mu_);
GRPC_ERROR_UNREF(error);
}
grpc_plugin_credentials::grpc_plugin_credentials(

@ -31,7 +31,7 @@ extern grpc_core::TraceFlag grpc_plugin_credentials_trace;
struct grpc_plugin_credentials final : public grpc_call_credentials {
public:
struct pending_request {
bool cancelled;
grpc_error* error;
struct grpc_plugin_credentials* creds;
grpc_credentials_mdelem_array* md_array;
grpc_closure* on_request_metadata;
@ -51,11 +51,6 @@ struct grpc_plugin_credentials final : public grpc_call_credentials {
void cancel_get_request_metadata(grpc_credentials_mdelem_array* md_array,
grpc_error* error) override;
// Checks if the request has been cancelled.
// If not, removes it from the pending list, so that it cannot be
// cancelled out from under us.
// When this returns, r->cancelled indicates whether the request was
// cancelled before completion.
void pending_request_complete(pending_request* r);
private:

@ -160,8 +160,6 @@ static void on_credentials_metadata(void* arg, grpc_error* input_error) {
if (error == GRPC_ERROR_NONE) {
grpc_call_next_op(elem, batch);
} else {
error = grpc_error_set_int(error, GRPC_ERROR_INT_GRPC_STATUS,
GRPC_STATUS_UNAVAILABLE);
grpc_transport_stream_op_batch_finish_with_failure(batch, error,
calld->call_combiner);
}

@ -90,11 +90,13 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key,
const grpc::string_ref& metadata_value,
bool is_blocking, bool is_successful)
bool is_blocking, bool is_successful,
int delay_ms)
: metadata_key_(metadata_key.data(), metadata_key.length()),
metadata_value_(metadata_value.data(), metadata_value.length()),
is_blocking_(is_blocking),
is_successful_(is_successful) {}
is_successful_(is_successful),
delay_ms_(delay_ms) {}
bool IsBlocking() const override { return is_blocking_; }
@ -102,6 +104,11 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
grpc::string_ref service_url, grpc::string_ref method_name,
const grpc::AuthContext& channel_auth_context,
std::multimap<grpc::string, grpc::string>* metadata) override {
if (delay_ms_ != 0) {
gpr_sleep_until(
gpr_time_add(gpr_now(GPR_CLOCK_REALTIME),
gpr_time_from_millis(delay_ms_, GPR_TIMESPAN)));
}
EXPECT_GT(service_url.length(), 0UL);
EXPECT_GT(method_name.length(), 0UL);
EXPECT_TRUE(channel_auth_context.IsPeerAuthenticated());
@ -119,6 +126,7 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
grpc::string metadata_value_;
bool is_blocking_;
bool is_successful_;
int delay_ms_;
};
const char TestMetadataCredentialsPlugin::kBadMetadataKey[] =
@ -137,7 +145,7 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey, kGoodGuy,
is_blocking_, true)));
is_blocking_, true, 0)));
}
std::shared_ptr<CallCredentials> GetIncompatibleClientCreds() {
@ -145,7 +153,7 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey, "Mr Hyde",
is_blocking_, true)));
is_blocking_, true, 0)));
}
// Interface implementation
@ -1835,12 +1843,13 @@ TEST_P(SecureEnd2endTest, AuthMetadataPluginKeyFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kBadMetadataKey,
"Does not matter, will fail the key is invalid.", false, true))));
"Does not matter, will fail the key is invalid.", false, true,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.error_code(), StatusCode::UNAVAILABLE);
EXPECT_EQ(s.error_code(), StatusCode::INTERNAL);
}
TEST_P(SecureEnd2endTest, AuthMetadataPluginValueFailure) {
@ -1853,12 +1862,59 @@ TEST_P(SecureEnd2endTest, AuthMetadataPluginValueFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"With illegal \n value.", false, true))));
"With illegal \n value.", false, true, 0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.error_code(), StatusCode::UNAVAILABLE);
EXPECT_EQ(s.error_code(), StatusCode::INTERNAL);
}
TEST_P(SecureEnd2endTest, AuthMetadataPluginWithDeadline) {
MAYBE_SKIP_TEST;
ResetStub();
EchoRequest request;
EchoResponse response;
ClientContext context;
const int delay = 100;
std::chrono::system_clock::time_point deadline =
std::chrono::system_clock::now() + std::chrono::milliseconds(delay);
context.set_deadline(deadline);
context.set_credentials(grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin("meta_key", "Does not matter", true,
true, delay))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
if (!s.ok()) {
EXPECT_EQ(StatusCode::DEADLINE_EXCEEDED, s.error_code());
}
}
TEST_P(SecureEnd2endTest, AuthMetadataPluginWithCancel) {
MAYBE_SKIP_TEST;
ResetStub();
EchoRequest request;
EchoResponse response;
ClientContext context;
const int delay = 100;
context.set_credentials(grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin("meta_key", "Does not matter", true,
true, delay))));
request.set_message("Hello");
std::thread cancel_thread([&context] {
gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME),
gpr_time_from_millis(delay, GPR_TIMESPAN)));
context.TryCancel();
});
Status s = stub_->Echo(&context, request, &response);
if (!s.ok()) {
EXPECT_EQ(StatusCode::CANCELLED, s.error_code());
}
cancel_thread.join();
}
TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
@ -1871,13 +1927,13 @@ TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", false,
false))));
"Does not matter, will fail anyway (see 3rd param)", false, false,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.error_code(), StatusCode::UNAVAILABLE);
EXPECT_EQ(s.error_code(), StatusCode::NOT_FOUND);
EXPECT_EQ(s.error_message(),
grpc::string("Getting metadata from plugin failed with error: ") +
kTestCredsPluginErrorMsg);
@ -1935,13 +1991,13 @@ TEST_P(SecureEnd2endTest, BlockingAuthMetadataPluginFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", true,
false))));
"Does not matter, will fail anyway (see 3rd param)", true, false,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.error_code(), StatusCode::UNAVAILABLE);
EXPECT_EQ(s.error_code(), StatusCode::NOT_FOUND);
EXPECT_EQ(s.error_message(),
grpc::string("Getting metadata from plugin failed with error: ") +
kTestCredsPluginErrorMsg);
@ -1962,11 +2018,11 @@ TEST_P(SecureEnd2endTest, CompositeCallCreds) {
grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(kMetadataKey1, kMetadataVal1,
true, true))),
true, true, 0))),
grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(kMetadataKey2, kMetadataVal2,
true, true)))));
true, true, 0)))));
request.set_message("Hello");
request.mutable_param()->set_echo_metadata(true);

Loading…
Cancel
Save