Skip to content

deps: update cel-cpp and abseil#10243

Merged
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
kyessenov:update_cel_cpp_03_03
Mar 5, 2020
Merged

deps: update cel-cpp and abseil#10243
mattklein123 merged 5 commits intoenvoyproxy:masterfrom
kyessenov:update_cel_cpp_03_03

Conversation

@kyessenov
Copy link
Contributor

Signed-off-by: Kuat Yessenov kuat@google.com

Description: updates google/cel-cpp and abseil
Risk Level: low
Testing: backwards compatible
Docs Changes: none
Release Notes: none

Signed-off-by: Kuat Yessenov <kuat@google.com>
@asraa
Copy link
Contributor

asraa commented Mar 4, 2020

Thanks! Looks like there's a legitimate failure, due to how absl now throws when calling front() on an empty string_view here:

if (potential_ip_address.front() == '[' && potential_ip_address.back() == ']') {

But also we only passed in an empty string because there are 15 patterns but only 14 specified.

std::array<std::tuple<bool, std::string, std::string, absl::optional<uint32_t>>, 15> patterns{
We probably should guard against an empty host in parseAuthority or at least document that the host should be nonempty anyway.

@asraa asraa self-requested a review March 4, 2020 15:03
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
Copy link
Contributor Author

Thanks @asraa , nice find. I added a defensive check since changing the pre-condition would require changing all the clients (and some might live outside of this repo). Let me know what you think.

Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
Copy link
Contributor Author

Coverage failed in some QUIC test. I don't think this change could possibly cause that.

@asraa
Copy link
Contributor

asraa commented Mar 4, 2020

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #10243 (comment) was created by @asraa.

see: more, trace.

@kyessenov
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #10243 (comment) was created by @kyessenov.

see: more, trace.

@kyessenov
Copy link
Contributor Author

@lizan Coverage is again acting up. Let me know if you have any ideas how to resolve it this time.

@mattklein123
Copy link
Member

Please merge master and it will probably be (mostly) fixed.

@mattklein123 mattklein123 merged commit 458ae91 into envoyproxy:master Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants