From f145d71dcce35cab31164b7cc6bbdc1065a4980e Mon Sep 17 00:00:00 2001 From: Qian Sun Date: Tue, 21 Feb 2017 18:07:33 -0800 Subject: [PATCH 1/4] Fail request if api_key is not valid. --- .../src/api_manager/service_control/aggregated.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/contrib/endpoints/src/api_manager/service_control/aggregated.cc b/contrib/endpoints/src/api_manager/service_control/aggregated.cc index d2754d69dbd..7bdd85ea1e0 100644 --- a/contrib/endpoints/src/api_manager/service_control/aggregated.cc +++ b/contrib/endpoints/src/api_manager/service_control/aggregated.cc @@ -193,10 +193,10 @@ Status Aggregated::Init() { options.periodic_timer = [this](int interval_ms, std::function callback) -> std::unique_ptr<::google::service_control_client::PeriodicTimer> { - return std::unique_ptr<::google::service_control_client::PeriodicTimer>( - new ApiManagerPeriodicTimer(env_->StartPeriodicTimer( - std::chrono::milliseconds(interval_ms), callback))); - }; + return std::unique_ptr<::google::service_control_client::PeriodicTimer>( + new ApiManagerPeriodicTimer(env_->StartPeriodicTimer( + std::chrono::milliseconds(interval_ms), callback))); + }; client_ = ::google::service_control_client::CreateServiceControlClient( service_->name(), service_->id(), options); return Status::OK; @@ -289,7 +289,10 @@ void Aggregated::Check( *response, service_control_proto_.service_name(), &response_info); // If allow_unregistered_calls is true, it is always OK to proceed. if (allow_unregistered_calls) { - on_done(Status::OK, response_info); + if (response_info.is_api_key_valid && + response_info.service_is_activated) { + on_done(Status::OK, response_info); + } } else { on_done(status, response_info); } From 49492a1890215ff54477d771bc322c4305bf9f72 Mon Sep 17 00:00:00 2001 From: Qian Sun Date: Wed, 22 Feb 2017 10:25:50 -0800 Subject: [PATCH 2/4] Format code. --- .../api_manager/service_control/aggregated.cc | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/contrib/endpoints/src/api_manager/service_control/aggregated.cc b/contrib/endpoints/src/api_manager/service_control/aggregated.cc index 7bdd85ea1e0..bb055fa88f3 100644 --- a/contrib/endpoints/src/api_manager/service_control/aggregated.cc +++ b/contrib/endpoints/src/api_manager/service_control/aggregated.cc @@ -193,10 +193,10 @@ Status Aggregated::Init() { options.periodic_timer = [this](int interval_ms, std::function callback) -> std::unique_ptr<::google::service_control_client::PeriodicTimer> { - return std::unique_ptr<::google::service_control_client::PeriodicTimer>( - new ApiManagerPeriodicTimer(env_->StartPeriodicTimer( - std::chrono::milliseconds(interval_ms), callback))); - }; + return std::unique_ptr<::google::service_control_client::PeriodicTimer>( + new ApiManagerPeriodicTimer(env_->StartPeriodicTimer( + std::chrono::milliseconds(interval_ms), callback))); + }; client_ = ::google::service_control_client::CreateServiceControlClient( service_->name(), service_->id(), options); return Status::OK; @@ -287,12 +287,11 @@ void Aggregated::Check( if (status.ok()) { Status status = Proto::ConvertCheckResponse( *response, service_control_proto_.service_name(), &response_info); - // If allow_unregistered_calls is true, it is always OK to proceed. - if (allow_unregistered_calls) { - if (response_info.is_api_key_valid && - response_info.service_is_activated) { - on_done(Status::OK, response_info); - } + // When allow_unregistered_calls is true, it is always OK to proceed if + // the api_key is valid and service is activated. + if (allow_unregistered_calls && response_info.is_api_key_valid && + response_info.service_is_activated) { + on_done(Status::OK, response_info); } else { on_done(status, response_info); } From 46ab1a6dcea8463bbb74c160e8c8b211ba488908 Mon Sep 17 00:00:00 2001 From: Qian Sun Date: Wed, 22 Feb 2017 10:45:10 -0800 Subject: [PATCH 3/4] Update comments. --- .../src/api_manager/service_control/aggregated.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/service_control/aggregated.cc b/contrib/endpoints/src/api_manager/service_control/aggregated.cc index bb055fa88f3..eb0b40fa1b9 100644 --- a/contrib/endpoints/src/api_manager/service_control/aggregated.cc +++ b/contrib/endpoints/src/api_manager/service_control/aggregated.cc @@ -287,8 +287,12 @@ void Aggregated::Check( if (status.ok()) { Status status = Proto::ConvertCheckResponse( *response, service_control_proto_.service_name(), &response_info); - // When allow_unregistered_calls is true, it is always OK to proceed if - // the api_key is valid and service is activated. + // If server replied with either invalid api_key or not actived service, + // the request is failed even allow_unregistered_calls is true. Most + // likely, users provide a wrong api key. By failing the request, the + // users will be notified with the error and have chance to correct it. + // Otherwise, the Report call will fail. It is very hard to notice and + // debug the Report failure. if (allow_unregistered_calls && response_info.is_api_key_valid && response_info.service_is_activated) { on_done(Status::OK, response_info); From fe1c48ed29de4b1974684932f7b6bbd6096b6558 Mon Sep 17 00:00:00 2001 From: Qian Sun Date: Wed, 22 Feb 2017 11:14:55 -0800 Subject: [PATCH 4/4] Address comment. --- .../endpoints/src/api_manager/service_control/aggregated.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/service_control/aggregated.cc b/contrib/endpoints/src/api_manager/service_control/aggregated.cc index eb0b40fa1b9..e59a69dc8cc 100644 --- a/contrib/endpoints/src/api_manager/service_control/aggregated.cc +++ b/contrib/endpoints/src/api_manager/service_control/aggregated.cc @@ -287,8 +287,8 @@ void Aggregated::Check( if (status.ok()) { Status status = Proto::ConvertCheckResponse( *response, service_control_proto_.service_name(), &response_info); - // If server replied with either invalid api_key or not actived service, - // the request is failed even allow_unregistered_calls is true. Most + // If server replied with either invalid api_key or not activated service, + // the request is rejected even allow_unregistered_calls is true. Most // likely, users provide a wrong api key. By failing the request, the // users will be notified with the error and have chance to correct it. // Otherwise, the Report call will fail. It is very hard to notice and