address comment

istio · Aug 16, 2018 · fe4777b · fe4777b
1 parent 64d0d94
commit fe4777b
Show file tree

Hide file tree

Showing 16 changed files with 39 additions and 38 deletions.
diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc
@@ -31,12 +31,12 @@ using Envoy::Http::Istio::AuthN::FilterContext;
 using istio::authn::Payload;
 using istio::authn::Result;
 using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig;
-using testing::_;
 using testing::AtLeast;
 using testing::Invoke;
 using testing::NiceMock;
 using testing::ReturnRef;
 using testing::StrictMock;
+using testing::_;
 
 namespace iaapi = istio::authentication::v1alpha1;
 

diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc
@@ -27,13 +27,13 @@ namespace iaapi = istio::authentication::v1alpha1;
 
 using istio::authn::Payload;
 using istio::authn::Result;
-using testing::_;
 using testing::DoAll;
 using testing::MockFunction;
 using testing::NiceMock;
 using testing::Return;
 using testing::SetArgPointee;
 using testing::StrictMock;
+using testing::_;
 
 namespace Envoy {
 namespace Http {

diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc
@@ -26,13 +26,13 @@
 namespace iaapi = istio::authentication::v1alpha1;
 
 using istio::authn::Payload;
-using testing::_;
 using testing::DoAll;
 using testing::MockFunction;
 using testing::NiceMock;
 using testing::Return;
 using testing::SetArgPointee;
 using testing::StrictMock;
+using testing::_;
 
 namespace Envoy {
 namespace Http {

diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc
@@ -22,9 +22,9 @@
 
 using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1::
     JwtAuthentication;
-using ::testing::_;
 using ::testing::Invoke;
 using ::testing::NiceMock;
+using ::testing::_;
 
 namespace Envoy {
 namespace Http {

diff --git a/src/envoy/http/jwt_auth/token_extractor_test.cc b/src/envoy/http/jwt_auth/token_extractor_test.cc
@@ -19,9 +19,9 @@
 
 using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1::
     JwtAuthentication;
-using ::testing::_;
 using ::testing::Invoke;
 using ::testing::NiceMock;
+using ::testing::_;
 
 namespace Envoy {
 namespace Http {

diff --git a/src/istio/api_spec/http_api_spec_parser_test.cc b/src/istio/api_spec/http_api_spec_parser_test.cc
@@ -27,8 +27,8 @@ using ::istio::mixer::v1::Attributes;
 using ::istio::mixer::v1::config::client::HTTPAPISpec;
 using ::istio::utils::AttributesBuilder;
 
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace api_spec {

diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc
@@ -29,8 +29,8 @@ using ::google::protobuf::util::MessageDifferencer;
 using ::istio::mixer::v1::Attributes;
 using ::istio::mixer::v1::Attributes_StringMap;
 
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace control {

diff --git a/src/istio/control/http/request_handler_impl_test.cc b/src/istio/control/http/request_handler_impl_test.cc
@@ -35,8 +35,8 @@ using ::istio::mixerclient::MixerClient;
 using ::istio::mixerclient::TransportCheckFunc;
 using ::istio::quota_config::Requirement;
 
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace control {

diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc
@@ -27,9 +27,9 @@
 using ::google::protobuf::TextFormat;
 using ::google::protobuf::util::MessageDifferencer;
 
-using ::testing::_;
 using ::testing::Invoke;
 using ::testing::Return;
+using ::testing::_;
 
 namespace istio {
 namespace control {
@@ -385,15 +385,6 @@ TEST(AttributesBuilderTest, TestCheckAttributes) {
         }
         return true;
       }));
-  EXPECT_CALL(mock_data, GetSourceNamespace(_, _))
-      .WillRepeatedly(
-          Invoke([](const std::string& principal, std::string* ns) -> bool {
-            if (principal == "cluster.local/sa/test_user/ns/test_ns/") {
-              *ns = "test_ns";
-              return true;
-            }
-            return false;
-          }));
   EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5"));
   EXPECT_CALL(mock_data, GetRequestedServerName(_))
       .WillOnce(Invoke([](std::string* name) -> bool {

diff --git a/src/istio/control/tcp/mock_check_data.h b/src/istio/control/tcp/mock_check_data.h
@@ -28,8 +28,6 @@ class MockCheckData : public CheckData {
  public:
   MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string* ip, int* port));
   MOCK_CONST_METHOD2(GetPrincipal, bool(bool peer, std::string* user));
-  MOCK_CONST_METHOD2(GetSourceNamespace,
-                     bool(const std::string&, std::string* ns));
   MOCK_CONST_METHOD0(IsMutualTLS, bool());
   MOCK_CONST_METHOD1(GetRequestedServerName, bool(std::string* name));
   MOCK_CONST_METHOD0(GetConnectionId, std::string());

diff --git a/src/istio/control/tcp/request_handler_impl_test.cc b/src/istio/control/tcp/request_handler_impl_test.cc
@@ -31,8 +31,8 @@ using ::istio::mixerclient::MixerClient;
 using ::istio::mixerclient::TransportCheckFunc;
 using ::istio::quota_config::Requirement;
 
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace control {

diff --git a/src/istio/mixerclient/client_impl_test.cc b/src/istio/mixerclient/client_impl_test.cc
@@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest;
 using ::istio::mixer::v1::CheckResponse;
 using ::istio::mixerclient::CheckResponseInfo;
 using ::istio::quota_config::Requirement;
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace mixerclient {

diff --git a/src/istio/mixerclient/quota_cache_test.cc b/src/istio/mixerclient/quota_cache_test.cc
@@ -27,8 +27,8 @@ using ::istio::mixer::v1::CheckRequest;
 using ::istio::mixer::v1::CheckResponse;
 using ::istio::mixer::v1::ReferencedAttributes;
 using ::istio::quota_config::Requirement;
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace mixerclient {

diff --git a/src/istio/mixerclient/report_batch_test.cc b/src/istio/mixerclient/report_batch_test.cc
@@ -23,8 +23,8 @@ using ::google::protobuf::util::error::Code;
 using ::istio::mixer::v1::Attributes;
 using ::istio::mixer::v1::ReportRequest;
 using ::istio::mixer::v1::ReportResponse;
-using ::testing::_;
 using ::testing::Invoke;
+using ::testing::_;
 
 namespace istio {
 namespace mixerclient {

diff --git a/src/istio/utils/utils.cc b/src/istio/utils/utils.cc
@@ -36,22 +36,34 @@ bool GetSourceNamespace(const std::string& principal,
     // but is a little more flexible. It assumes that the principal begins with
     // a <DOMAIN> string followed by <key,value> pairs separated by '/'.
 
-    // Spilt the principal into tokens separated by kDelimiter.
-    std::stringstream ss(principal);
-    std::vector<std::string> tokens;
-    std::string token;
-    while (std::getline(ss, token, kDelimiter)) {
-      tokens.push_back(token);
+    // Skip the first <DOMAIN> string and check remaining <key, value> pairs.
+    size_t begin = principal.find(kDelimiter);
+    if (begin == std::string::npos) {
+      return false;
     }
+    begin += 1;
 
-    // Skip the first <DOMAIN> string and check remaining <key, value> pairs.
-    for (int i = 1; i < tokens.size(); i += 2) {
-      if (tokens[i] == kNamespaceKey) {
-        // Found the namespace key, treat the following token as namespace.
-        int j = i + 1;
-        *source_namespace = (j < tokens.size() ? tokens[j] : "");
+    while (1) {
+      size_t key_end = principal.find(kDelimiter, begin);
+      if (key_end == std::string::npos) {
+        return false;
+      }
+
+      size_t value_begin = key_end + 1;
+      size_t value_end = principal.find(kDelimiter, value_begin);
+
+      if (principal.compare(begin, key_end - begin, kNamespaceKey) == 0) {
+        size_t len = (value_end == std::string::npos ? value_end
+                                                     : value_end - value_begin);
+        *source_namespace = principal.substr(value_begin, len);
         return true;
       }
+
+      if (value_end == std::string::npos) {
+        return false;
+      }
+
+      begin = value_end + 1;
     }
   }
   return false;

diff --git a/src/istio/utils/utils_test.cc b/src/istio/utils/utils_test.cc
@@ -42,15 +42,15 @@ TEST_F(UtilsTest, GetSourceNamespace) {
   checkFalse("ns/abc/sa/user");
   checkFalse("cluster.local");
   checkFalse("cluster.local/");
+  checkFalse("cluster.local/ns");
   checkFalse("cluster.local/sa/user");
+  checkFalse("cluster.local/sa/user/ns");
   checkFalse("cluster.local/sa/user_ns/");
   checkFalse("cluster.local/sa/user_ns/abc/xyz");
   checkFalse("cluster.local/NS/abc");
 
-  checkTrue("cluster.local/ns", "");
   checkTrue("cluster.local/ns/", "");
   checkTrue("cluster.local/ns//", "");
-  checkTrue("cluster.local/sa/user/ns", "");
   checkTrue("cluster.local/sa/user/ns/", "");
   checkTrue("cluster.local/ns//sa/user", "");