From e2713da8dc92d3915e3f64f821276bdb5dc3e427 Mon Sep 17 00:00:00 2001 From: Constance Caramanolis Date: Thu, 1 Sep 2016 09:43:54 -0700 Subject: [PATCH 1/3] Check for duplicate domains --- source/common/router/config_impl.cc | 5 ++++ test/common/router/config_impl_test.cc | 38 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 4cf8f8b79aa6..593f52a17b48 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -244,6 +244,11 @@ RouteMatcher::RouteMatcher(const Json::Object& config, Runtime::Loader& runtime, } default_virtual_host_ = virtual_host; } else { + if (virtual_hosts_.find(domain) != virtual_hosts_.end()) { + throw EnvoyException(fmt::format( + "Only unique values for domains are permitted. Duplicate entry of domain {}", + domain)); + } virtual_hosts_.emplace(domain, virtual_host); } } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index e68cc49a0471..37428a0db687 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -641,6 +641,44 @@ TEST(RouteMatcherTest, TestBadDefaultConfig) { EXPECT_THROW(ConfigImpl config(loader, runtime, cm), EnvoyException); } +TEST(RouteMatcherTest, TestDuplicateDomainConfig) { +std::string json = R"EOF( +{ + "virtual_hosts": [ + { + "name": "www2", + "domains": ["www.lyft.com"], + "routes": [ + { + "prefix": "/", + "cluster": "www2" + } + ] + }, + { + "name": "www2_staging", + "domains": ["www.lyft.com"], + "routes": [ + { + "prefix": "/", + "cluster": "www2_staging" + } + ] + } + ], + + "internal_only_headers": [ + "x-lyft-user-id" + ] +} + )EOF"; + +Json::StringLoader loader(json); +NiceMock runtime; +NiceMock cm; +EXPECT_THROW(ConfigImpl config(loader, runtime, cm), EnvoyException); +} + static Http::HeaderMapImpl genRedirectHeaders(const std::string& host, const std::string& path, bool ssl, bool internal) { Http::HeaderMapImpl headers{ From 1603b1951d65a174ee86f2699665508185858d24 Mon Sep 17 00:00:00 2001 From: Constance Caramanolis Date: Thu, 1 Sep 2016 09:55:03 -0700 Subject: [PATCH 2/3] address prr comments --- test/common/router/config_impl_test.cc | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 37428a0db687..c7d1a2f53e70 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -642,7 +642,7 @@ TEST(RouteMatcherTest, TestBadDefaultConfig) { } TEST(RouteMatcherTest, TestDuplicateDomainConfig) { -std::string json = R"EOF( + std::string json = R"EOF( { "virtual_hosts": [ { @@ -665,18 +665,14 @@ std::string json = R"EOF( } ] } - ], - - "internal_only_headers": [ - "x-lyft-user-id" ] } )EOF"; -Json::StringLoader loader(json); -NiceMock runtime; -NiceMock cm; -EXPECT_THROW(ConfigImpl config(loader, runtime, cm), EnvoyException); + Json::StringLoader loader(json); + NiceMock runtime; + NiceMock cm; + EXPECT_THROW(ConfigImpl config(loader, runtime, cm), EnvoyException); } static Http::HeaderMapImpl genRedirectHeaders(const std::string& host, const std::string& path, From af9215560faf3b75e19ce6ce1a3ff917269f0a28 Mon Sep 17 00:00:00 2001 From: Constance Caramanolis Date: Thu, 1 Sep 2016 10:51:54 -0700 Subject: [PATCH 3/3] Add docs --- docs/configuration/http_conn_man/route_config/vhost.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/configuration/http_conn_man/route_config/vhost.rst b/docs/configuration/http_conn_man/route_config/vhost.rst index 84ff8cd6efdc..121851448431 100644 --- a/docs/configuration/http_conn_man/route_config/vhost.rst +++ b/docs/configuration/http_conn_man/route_config/vhost.rst @@ -27,7 +27,8 @@ domains *(required, array)* A list of domains (host/authority header) that will be matched to this virtual host. Currently, wildcard matching of the form "\*.foo.com" is not supported, however a special entry "\*" is allowed which will match any host/authority header. Only a single virtual - host in the entire route configuration can match on "\*". + host in the entire route configuration can match on "\*". A domain must be unique across all + virtual hosts or the config will fail to load. :ref:`routes ` *(required, array)* The list of routes that will be matched, in order, for incoming requests.