utility: remove absl::strings_internal::memcasecmp usage (#2700) #2702
Conversation
…2700) Signed-off-by: Akhil Thampy <akhilthampy@yahoo.com>
|
Some tests are failing with the change. At a quick glance the change looks correct; is there a subtle behavior difference here? |
source/common/common/utility.cc
Outdated
| @@ -134,7 +134,7 @@ bool StringUtil::caseCompare(absl::string_view lhs, absl::string_view rhs) { | |||
| if (rhs.size() != lhs.size()) { | |||
| return false; | |||
| } | |||
| return absl::strings_internal::memcasecmp(rhs.data(), lhs.data(), rhs.size()) == 0; | |||
| return absl::StartsWithIgnoreCase(rhs.data(), lhs.data()); | |||
There was a problem hiding this comment.
@jmarantz gzip_filter_test passed after removing .data() calls
https://gist.github.com/athampy/545550bbd2b1bcb461cb9d677ef802f2
There was a problem hiding this comment.
Right! @athampy absl::StartsWithIgnoreCase expects absl::string_view while var.data() returns a char *
https://github.com/abseil/abseil-cpp/blob/master/absl/strings/match.cc#L30
There was a problem hiding this comment.
Yes, exactly!
Here's what I think is happening:
absl::string_view.size() doesn't necessarily represent the size of the underlying char*
When we pass in rhs.data(), for example, it gets implicitly converted to an absl::string_view where the size() == size of the char*
This becomes a problem when we have:
lhs = <absl::string_view> (_ptr="my;test;string", size=3)
// lhs.data() is "my;test;string" which gets converted to absl::string_view with size=14
rhs = <absl::string_view> (_ptr="my;", size=3)
// rhs.data() is "my;" which gets converted to absl::string_view with size=3
absl::StartsWithIgnoreCase(rhs.data(), lhs.data()) -> false // rhs.size() is not >= lhs.size()…ing_view.data() (envoyproxy#2700) Signed-off-by: Akhil Thampy <akhilthampy@yahoo.com>
…oyproxy#2700) (envoyproxy#2702)" This reverts commit faadda1.
This fixes up issues exposed in #24151 where the PlatformBridgeCertValidatorFactory was not associated with the PlatformBridgeCertValidator proto. Risk Level: low Testing: #24151 Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: JP Simard <jp@jpsim.com>
This fixes up issues exposed in #24151 where the PlatformBridgeCertValidatorFactory was not associated with the PlatformBridgeCertValidator proto. Risk Level: low Testing: #24151 Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: JP Simard <jp@jpsim.com>
title: Remove absl::strings_internal usage in utility
Description:
Replace use of absl::strings_internal::memcasecmp() in utility with absl::StartsWithIgnoreCase. This removes dependency on Abseil's internals and is inline with the Abseil Compatibility Guidelines..
Risk Level: Low
Testing: Ran unit tests
bazel test //test/common/common:utility_testDocs Changes:N/A
Release Notes: N/A
Fixes #2700