From befd3cd53659619f152b96add720641143693a5f Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Fri, 31 Mar 2017 16:47:43 -0700 Subject: [PATCH 1/6] schema: enforce cluster name length and invalid chars --- docs/configuration/cluster_manager/cluster.rst | 1 + source/common/json/config_schemas.cc | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/configuration/cluster_manager/cluster.rst b/docs/configuration/cluster_manager/cluster.rst index cb78a199b917a..cc87ae27a1f7c 100644 --- a/docs/configuration/cluster_manager/cluster.rst +++ b/docs/configuration/cluster_manager/cluster.rst @@ -28,6 +28,7 @@ Cluster name *(required, string)* Supplies the name of the cluster which must be unique across all clusters. The cluster name is used when emitting :ref:`statistics `. + The cluster name can be at most 60 characters long, and must **not** contain ``:``. type *(required, string)* The :ref:`service discovery type ` to diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index d7d6c693f326d..f3fb93b6530f8 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1032,7 +1032,12 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( }, "type" : "object", "properties" : { - "name" : {"type" : "string"}, + "name" : { + "type" : "string", + "pattern" : "^[^:]+$", + "minLength" : 1, + "maxLength" : 60 + }, "type" : { "type" : "string", "enum" : ["static", "strict_dns", "logical_dns", "sds"] From 1af0ddad80ae7f41c965dd378070191ea55ff36c Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Mon, 3 Apr 2017 12:57:00 -0700 Subject: [PATCH 2/6] tests --- .../upstream/cluster_manager_impl_test.cc | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 584e15aa48a93..daafb00993f5f 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -260,6 +260,46 @@ TEST_F(ClusterManagerImplTest, UnknownHcType) { EXPECT_THROW(create(*loader), EnvoyException); } +TEST_F(ClusterManagerImplTest, MaxClusterName) { + std::string json = R"EOF( + { + "clusters": [ + { + "name": "clusterwithareallyreallylongnamemorethanmaxcharsallowedbyschema" + }] + } + )EOF"; + + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + try { + create(*loader); + } catch (Json::Exception& e) { + EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " + "Invalid keyword: maxLength.\n Invalid document key: #/name", + std::string(e.what())); + } +} + +TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { + std::string json = R"EOF( + { + "clusters": [ + { + "name": "cluster:" + }] + } + )EOF"; + + Json::ObjectPtr loader = Json::Factory::LoadFromString(json); + try { + create(*loader); + } catch (Json::Exception& e) { + EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " + "Invalid keyword: pattern.\n Invalid document key: #/name", + std::string(e.what())); + } +} + TEST_F(ClusterManagerImplTest, TcpHealthChecker) { std::string json = R"EOF( { From 427adb0afedd7a12e55cce4e22e8a7301994eb62 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Mon, 3 Apr 2017 17:05:30 -0700 Subject: [PATCH 3/6] update --- test/common/upstream/cluster_manager_impl_test.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index daafb00993f5f..04df31705a29f 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -273,11 +273,13 @@ TEST_F(ClusterManagerImplTest, MaxClusterName) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); try { create(*loader); + ADD_FAILURE() << "Json::Exception should take place. It did not."; } catch (Json::Exception& e) { EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " "Invalid keyword: maxLength.\n Invalid document key: #/name", std::string(e.what())); } + factory_.tls_.shutdownThread(); } TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { @@ -293,11 +295,13 @@ TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { Json::ObjectPtr loader = Json::Factory::LoadFromString(json); try { create(*loader); + ADD_FAILURE() << "Json::Exception should take place. It did not."; } catch (Json::Exception& e) { EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " "Invalid keyword: pattern.\n Invalid document key: #/name", std::string(e.what())); } + factory_.tls_.shutdownThread(); } TEST_F(ClusterManagerImplTest, TcpHealthChecker) { From 0865218745f0a7c6fcbf4e4718eb89803dcab3e6 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Tue, 4 Apr 2017 16:02:58 -0700 Subject: [PATCH 4/6] macro --- .../upstream/cluster_manager_impl_test.cc | 24 +++++++------------ test/test_common/utility.h | 10 ++++++++ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 04df31705a29f..dd187dcb9cd62 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -271,14 +271,10 @@ TEST_F(ClusterManagerImplTest, MaxClusterName) { )EOF"; Json::ObjectPtr loader = Json::Factory::LoadFromString(json); - try { - create(*loader); - ADD_FAILURE() << "Json::Exception should take place. It did not."; - } catch (Json::Exception& e) { - EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " - "Invalid keyword: maxLength.\n Invalid document key: #/name", - std::string(e.what())); - } + EXPECT_THROW_WITH_MESSAGE(create(*loader), Json::Exception, + "JSON object doesn't conform to schema.\n Invalid schema: " + "#/properties/name.\n Invalid keyword: maxLength.\n Invalid document " + "key: #/name"); factory_.tls_.shutdownThread(); } @@ -293,14 +289,10 @@ TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { )EOF"; Json::ObjectPtr loader = Json::Factory::LoadFromString(json); - try { - create(*loader); - ADD_FAILURE() << "Json::Exception should take place. It did not."; - } catch (Json::Exception& e) { - EXPECT_EQ("JSON object doesn't conform to schema.\n Invalid schema: #/properties/name.\n " - "Invalid keyword: pattern.\n Invalid document key: #/name", - std::string(e.what())); - } + EXPECT_THROW_WITH_MESSAGE(create(*loader), Json::Exception, + "JSON object doesn't conform to schema.\n Invalid schema: " + "#/properties/name.\n Invalid keyword: pattern.\n Invalid document " + "key: #/name"); factory_.tls_.shutdownThread(); } diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 2e5bd9e3d16af..5e4e18e8192a1 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -5,6 +5,16 @@ #include "common/http/header_map_impl.h" +using testing::_; + +#define EXPECT_THROW_WITH_MESSAGE(statement, expected_exception, message) \ + try { \ + statement; \ + ADD_FAILURE() << "Exception should take place. It did not."; \ + } catch (expected_exception & e) { \ + EXPECT_EQ(message, std::string(e.what())); \ + } + class TestUtility { public: /** From 2afffa3461036e76768f045709b5508606ddd458 Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Tue, 4 Apr 2017 16:22:01 -0700 Subject: [PATCH 5/6] update --- test/common/upstream/cluster_manager_impl_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index dd187dcb9cd62..8a9787d58d429 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -275,7 +275,6 @@ TEST_F(ClusterManagerImplTest, MaxClusterName) { "JSON object doesn't conform to schema.\n Invalid schema: " "#/properties/name.\n Invalid keyword: maxLength.\n Invalid document " "key: #/name"); - factory_.tls_.shutdownThread(); } TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { @@ -293,7 +292,6 @@ TEST_F(ClusterManagerImplTest, InvalidClusterNameChars) { "JSON object doesn't conform to schema.\n Invalid schema: " "#/properties/name.\n Invalid keyword: pattern.\n Invalid document " "key: #/name"); - factory_.tls_.shutdownThread(); } TEST_F(ClusterManagerImplTest, TcpHealthChecker) { From dfbd4f2e05644f05acf529fefcdbfc297a5babad Mon Sep 17 00:00:00 2001 From: Jose Nino Date: Wed, 5 Apr 2017 10:08:15 -0700 Subject: [PATCH 6/6] update --- test/test_common/utility.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 5e4e18e8192a1..2376cfe8a6e31 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -5,8 +5,6 @@ #include "common/http/header_map_impl.h" -using testing::_; - #define EXPECT_THROW_WITH_MESSAGE(statement, expected_exception, message) \ try { \ statement; \