Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce safe regex matcher based on re2 engine #7878

Merged
merged 40 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
ec6e30a
introduce safe regex matcher based on re2 engine
mattklein123 Jul 29, 2019
52fe33b
fix
mattklein123 Aug 8, 2019
e2ce809
fix
mattklein123 Aug 8, 2019
876c6e6
fix
mattklein123 Aug 8, 2019
2da7fc2
more fix
mattklein123 Aug 9, 2019
b5100cb
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 9, 2019
9a5df74
fix
mattklein123 Aug 13, 2019
a191481
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 13, 2019
7646e3e
Merge branch 'master' into safe_regex
mattklein123 Aug 14, 2019
0a7ad69
fix
mattklein123 Aug 14, 2019
befef2b
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 14, 2019
f2a25ea
more
mattklein123 Aug 14, 2019
85d6b06
format
mattklein123 Aug 14, 2019
c8f60b2
fix
mattklein123 Aug 14, 2019
be16355
fix
mattklein123 Aug 14, 2019
c7abb14
fix
mattklein123 Aug 14, 2019
25aa583
fix and merge
mattklein123 Aug 15, 2019
c6a4775
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 15, 2019
1107252
more
mattklein123 Aug 16, 2019
0956d5a
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 16, 2019
718c4f2
checkpoint
mattklein123 Aug 16, 2019
1cec838
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 16, 2019
9237720
more
mattklein123 Aug 16, 2019
c98c32f
more
mattklein123 Aug 16, 2019
f3543b5
more
mattklein123 Aug 16, 2019
8381588
comment
mattklein123 Aug 16, 2019
ed82996
fix
mattklein123 Aug 16, 2019
a038234
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 17, 2019
03544f7
Merge branch 'master' into safe_regex
mattklein123 Aug 20, 2019
462454e
comments
mattklein123 Aug 20, 2019
65102e4
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 21, 2019
17f9579
comments
mattklein123 Aug 22, 2019
24f0fcf
fix
mattklein123 Aug 22, 2019
866d1d5
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 22, 2019
a717ecf
fix
mattklein123 Aug 22, 2019
f2657e5
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 22, 2019
b825b9a
comment
mattklein123 Aug 22, 2019
138f596
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 22, 2019
b82ca63
fix
mattklein123 Aug 23, 2019
83f53ae
Merge remote-tracking branch 'origin/master' into safe_regex
mattklein123 Aug 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
more
Signed-off-by: Matt Klein <[email protected]>
  • Loading branch information
mattklein123 committed Aug 16, 2019
commit c98c32f51872c8d513e94bd43c45a3768b76a6c2
16 changes: 9 additions & 7 deletions api/envoy/api/v2/route/route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1103,13 +1103,14 @@ message VirtualCluster {
// * The regex */rides/\d+* does not match the path */rides/123/456*
//
// .. attention::
// This field has been deprecated in favor of `regex` as it is not safe for use with
// This field has been deprecated in favor of `headers` as it is not safe for use with
// untrusted input in all cases.
string pattern = 1 [(validate.rules).string.max_bytes = 1024, deprecated = true];

// Specifies a regex pattern to use for matching requests. The entire path of the request
// must match the regex.
type.matcher.RegexMatcher regex = 4;
// Specifies a list of header matchers to use for matching requests. Each specified header must
// match. The pseudo-headers `:path` and `:method` can be used to match the request path and
// method, respectively.
repeated HeaderMatcher headers = 4;

// Specifies the name of the virtual cluster. The virtual cluster name as well
// as the virtual host name are used when emitting statistics. The statistics are emitted by the
Expand All @@ -1118,9 +1119,10 @@ message VirtualCluster {

// Optionally specifies the HTTP method to match on. For example GET, PUT,
// etc.
// [#comment:TODO(htuch): add (validate.rules).enum.defined_only = true once
// https://github.com/lyft/protoc-gen-validate/issues/42 is resolved.]
core.RequestMethod method = 3;
//
// .. attention::
// This field has been deprecated in favor of `headers`.
core.RequestMethod method = 3 [deprecated = true];
}

// Global rate limiting :ref:`architecture overview <arch_overview_rate_limit>`.
Expand Down
4 changes: 2 additions & 2 deletions docs/root/intro/deprecated.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Version 1.12.0 (pending)
* The `allow_origin` and `allow_origin_regex` fields in :ref:`CorsPolicy
<envoy_api_msg_route.CorsPolicy>` have been deprecated in favor of the
`allow_origin_string_match` field.
* The `pattern` field in :ref:`VirtualCluster <envoy_api_msg_route.VirtualCluster>` has been
deprecated in favor of the `regex` field.
* The `pattern` and `method` fields in :ref:`VirtualCluster <envoy_api_msg_route.VirtualCluster>`
have been deprecated in favor of the `headers` field.
* The `regex_match` field in :ref:`HeaderMatcher <envoy_api_msg_route.HeaderMatcher>` has been
deprecated in favor of the `safe_regex_match` field.
* The `value` and `regex` fields in :ref:`QueryParameterMatcher
Expand Down
31 changes: 17 additions & 14 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1017,19 +1017,26 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::route::VirtualHost& virtu
VirtualHostImpl::VirtualClusterEntry::VirtualClusterEntry(
const envoy::api::v2::route::VirtualCluster& virtual_cluster, Stats::StatNamePool& pool)
: stat_name_(pool.add(virtual_cluster.name())) {
if (virtual_cluster.method() != envoy::api::v2::core::RequestMethod::METHOD_UNSPECIFIED) {
method_ = envoy::api::v2::core::RequestMethod_Name(virtual_cluster.method());
}

if (virtual_cluster.pattern().empty() == !virtual_cluster.has_regex()) {
throw EnvoyException("virtual clusters must define either 'pattern' or 'regex'");
if (virtual_cluster.pattern().empty() == virtual_cluster.headers().empty()) {
throw EnvoyException("virtual clusters must define either 'pattern' or 'headers'");
}

if (!virtual_cluster.pattern().empty()) {
regex_ = Regex::Utility::parseStdRegexAsCompiledMatcher(virtual_cluster.pattern());
envoy::api::v2::route::HeaderMatcher matcher_config;
matcher_config.set_name(Http::Headers::get().Path.get());
matcher_config.set_regex_match(virtual_cluster.pattern());
headers_.push_back(std::make_unique<Http::HeaderUtility::HeaderData>(matcher_config));
} else {
ASSERT(virtual_cluster.has_regex());
regex_ = Regex::Utility::parseRegex(virtual_cluster.regex());
ASSERT(!virtual_cluster.headers().empty());
headers_ = Http::HeaderUtility::buildHeaderDataVector(virtual_cluster.headers());
}

if (virtual_cluster.method() != envoy::api::v2::core::RequestMethod::METHOD_UNSPECIFIED) {
envoy::api::v2::route::HeaderMatcher matcher_config;
matcher_config.set_name(Http::Headers::get().Method.get());
matcher_config.set_exact_match(
envoy::api::v2::core::RequestMethod_Name(virtual_cluster.method()));
headers_.push_back(std::make_unique<Http::HeaderUtility::HeaderData>(matcher_config));
}
}

Expand Down Expand Up @@ -1174,11 +1181,7 @@ const std::shared_ptr<const SslRedirectRoute> VirtualHostImpl::SSL_REDIRECT_ROUT
const VirtualCluster*
VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const {
for (const VirtualClusterEntry& entry : virtual_clusters_) {
bool method_matches =
!entry.method_ || headers.Method()->value().getStringView() == entry.method_.value();

absl::string_view path_view = headers.Path()->value().getStringView();
if (method_matches && entry.regex_->match(path_view)) {
if (Http::HeaderUtility::matchHeaders(headers, entry.headers_)) {
return &entry;
}
}
Expand Down
3 changes: 1 addition & 2 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,8 @@ class VirtualHostImpl : public VirtualHost {
// Router::VirtualCluster
Stats::StatName statName() const override { return stat_name_; }

Regex::CompiledMatcherPtr regex_;
absl::optional<std::string> method_;
const Stats::StatName stat_name_;
std::vector<Http::HeaderUtility::HeaderDataPtr> headers_;
};

class CatchAllVirtualCluster : public VirtualCluster {
Expand Down
13 changes: 8 additions & 5 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,13 @@ TEST_F(RouteMatcherTest, TestRoutes) {
- pattern: "^/users/\\d+$"
method: PUT
name: update_user
- regex:
google_re2: {}
regex: "^/users/\\d+/location$"
method: POST
- headers:
- name: ":path"
safe_regex_match:
google_re2: {}
regex: "^/users/\\d+/location$"
- name: ":method"
exact_match: POST
name: ulu
)EOF";

Expand Down Expand Up @@ -664,7 +667,7 @@ TEST_F(RouteMatcherTest, TestRoutesWithInvalidVirtualCluster) {
EXPECT_THROW_WITH_REGEX(TestConfigImpl(parseRouteConfigurationFromV2Yaml(invalid_virtual_cluster),
factory_context_, true),
EnvoyException,
"virtual clusters must define either 'pattern' or 'regex'");
"virtual clusters must define either 'pattern' or 'headers'");
}

// Validates behavior of request_headers_to_add at router, vhost, and route levels.
Expand Down