Skip to content

Commit

Permalink
router: fix matching when all domains have wildcards (#4326)
Browse files Browse the repository at this point in the history
When all domains of all virtual hosts had wildcard characters and there
was a default virtual host, RouteMatcher::findVirtualHost() used to
erroneously ignore the wildcard suffixes.

This patch fixes the issue and introduces a unit test that covers this
case.

Risk level: Medium. The fix is straightforward, but users who rely on
the erroneous behavior might be affected.

Testing: Introduced TestRoutesWithWildcardAndDefaultOnly

Signed-off-by: Tal Nordan tal.nordan@solo.io
  • Loading branch information
talnordan authored and dnoe committed Sep 5, 2018
1 parent aa06142 commit ae6a252
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ RouteConstSharedPtr VirtualHostImpl::getRouteFromEntries(const Http::HeaderMap&

const VirtualHostImpl* RouteMatcher::findVirtualHost(const Http::HeaderMap& headers) const {
// Fast path the case where we only have a default virtual host.
if (virtual_hosts_.empty() && default_virtual_host_) {
if (virtual_hosts_.empty() && wildcard_virtual_host_suffixes_.empty() && default_virtual_host_) {
return default_virtual_host_.get();
}

Expand Down
25 changes: 25 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,31 @@ TEST(RouteMatcherTest, TestRoutes) {
}
}

TEST(RouteMatcherTest, TestRoutesWithWildcardAndDefaultOnly) {
std::string yaml = R"EOF(
virtual_hosts:
- name: wildcard
domains: ["*.solo.io"]
routes:
- match: { prefix: "/" }
route: { cluster: "wildcard" }
- name: default
domains: ["*"]
routes:
- match: { prefix: "/" }
route: { cluster: "default" }
)EOF";

const auto proto_config = parseRouteConfigurationFromV2Yaml(yaml);
NiceMock<Server::Configuration::MockFactoryContext> factory_context;
TestConfigImpl config(proto_config, factory_context, true);

EXPECT_EQ("wildcard",
config.route(genHeaders("gloo.solo.io", "/", "GET"), 0)->routeEntry()->clusterName());
EXPECT_EQ("default",
config.route(genHeaders("example.com", "/", "GET"), 0)->routeEntry()->clusterName());
}

TEST(RouteMatcherTest, TestRoutesWithInvalidRegex) {
std::string invalid_route = R"EOF(
virtual_hosts:
Expand Down

0 comments on commit ae6a252

Please sign in to comment.