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.

Sign up for GitHub

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

[redis_proxy] added a constraint for route.prefix().size() #12637

Merged
merged 14 commits into from
Aug 14, 2020
Merged

[redis_proxy] added a constraint for route.prefix().size() #12637

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
00e5deb
added a test case with long string for trie, and added destructor of the
jianwen612 Aug 4, 2020
dcd8885
set string to be const.
jianwen612 Aug 4, 2020
1065f8f
added a comment for destructor
jianwen612 Aug 5, 2020
28ff90b
added a helper function to makecode clearer.
jianwen612 Aug 5, 2020
99f9958
refined the comments
jianwen612 Aug 5, 2020
c7738a1
refined the comments
jianwen612 Aug 5, 2020
b5fcd4c
added regression test for fuzzer
jianwen612 Aug 5, 2020
15bc3ad
Merge remote-tracking branch 'upstream/master' into fix_issue_24558
jianwen612 Aug 6, 2020
4962561
fixed issue 24558 by constrain the length of route.prefix.length()
jianwen612 Aug 13, 2020
3d3022c
restore a empty line
jianwen612 Aug 13, 2020
f777601
removed an empty line
jianwen612 Aug 13, 2020
686548c
added a new line
jianwen612 Aug 13, 2020
3db2a65
Merge remote-tracking branch 'upstream/master' into another_fix_issue…
jianwen612 Aug 13, 2020
3429c3a
add PGV for route.length()
jianwen612 Aug 13, 2020
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
5 changes: 5 additions & 0 deletions source/extensions/filters/network/redis_proxy/router_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ PrefixRoutes::PrefixRoutes(
: nullptr) {

for (auto const& route : config.routes()) {
if(route.prefix().size() > 1000){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be a PGV annotation?

Copy link
Contributor Author

@jianwen612 jianwen612 Aug 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be a PGV annotation?

Oh I can make it part of PGV. Do you think 1000 is ok?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

// Avoid a stack overflow issue when prefix_loopup_table_'s depth is great and default
// destructor recursively collects unique_ptr.
throw EnvoyException(fmt::format("prefix size is too long: size = {}", route.prefix().size()));
}
std::string copy(route.prefix());

if (case_insensitive_) {
Expand Down
11 changes: 11 additions & 0 deletions ...ommon/fuzz/network_readfilter_corpus/oss_redis_proxy_stackoverflow_with_long_route_prefix
View file
Open in desktop

Large diffs are not rendered by default.