Skip to content

Commit

Permalink
Make ABSL use STL types. (envoyproxy#25108)
Browse files Browse the repository at this point in the history
Signed-off-by: Yan Avlasov <[email protected]>
  • Loading branch information
yanavlasov authored Feb 17, 2023
1 parent 7107a83 commit 525ff42
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 55 deletions.
42 changes: 0 additions & 42 deletions bazel/abseil.patch
Original file line number Diff line number Diff line change
@@ -1,45 +1,3 @@
# Force internal versions of std classes per
# https://abseil.io/docs/cpp/guides/options
diff --git a/absl/base/options.h b/absl/base/options.h
index 230bf1e..6e1b9e5 100644
--- a/absl/base/options.h
+++ b/absl/base/options.h
@@ -100,7 +100,7 @@
// User code should not inspect this macro. To check in the preprocessor if
// absl::any is a typedef of std::any, use the feature macro ABSL_USES_STD_ANY.

-#define ABSL_OPTION_USE_STD_ANY 2
+#define ABSL_OPTION_USE_STD_ANY 0


// ABSL_OPTION_USE_STD_OPTIONAL
@@ -127,7 +127,7 @@
// absl::optional is a typedef of std::optional, use the feature macro
// ABSL_USES_STD_OPTIONAL.

-#define ABSL_OPTION_USE_STD_OPTIONAL 2
+#define ABSL_OPTION_USE_STD_OPTIONAL 0


// ABSL_OPTION_USE_STD_STRING_VIEW
@@ -154,7 +154,7 @@
// absl::string_view is a typedef of std::string_view, use the feature macro
// ABSL_USES_STD_STRING_VIEW.

-#define ABSL_OPTION_USE_STD_STRING_VIEW 2
+#define ABSL_OPTION_USE_STD_STRING_VIEW 0

// ABSL_OPTION_USE_STD_VARIANT
//
@@ -180,7 +180,7 @@
// absl::variant is a typedef of std::variant, use the feature macro
// ABSL_USES_STD_VARIANT.

-#define ABSL_OPTION_USE_STD_VARIANT 2
+#define ABSL_OPTION_USE_STD_VARIANT 0


// ABSL_OPTION_USE_INLINE_NAMESPACE
diff --git a/absl/flags/commandlineflag.h b/absl/flags/commandlineflag.h
index f2fa08977f..8e97fdb0ca 100644
--- a/absl/flags/commandlineflag.h
Expand Down
15 changes: 15 additions & 0 deletions bazel/cel-cpp.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#
# Temporary patch to cel-cpp to accomodate switch from ABSL types to STL types.
#
# Patch will be removed once build issues in new cel-cpp are addressed.
#
--- a/eval/public/ast_traverse.cc 2023-01-23 17:38:48.730240356 +0000
+++ b/eval/public/ast_traverse.cc 2023-01-23 17:39:27.182207871 +0000
@@ -17,6 +17,7 @@
#include <stack>

#include "google/api/expr/v1alpha1/syntax.pb.h"
+#include "absl/base/attributes.h"
#include "absl/types/variant.h"
#include "eval/public/ast_visitor.h"
#include "eval/public/source_position.h"
6 changes: 5 additions & 1 deletion bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,11 @@ def _com_github_facebook_zstd():
)

def _com_google_cel_cpp():
external_http_archive("com_google_cel_cpp")
external_http_archive(
"com_google_cel_cpp",
patches = ["@envoy//bazel:cel-cpp.patch"],
patch_args = ["-p1"],
)

def _com_github_google_perfetto():
external_http_archive(
Expand Down
2 changes: 2 additions & 0 deletions envoy/common/hashable.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

#include <cstdint>

#include "envoy/common/pure.h"

#include "absl/types/optional.h"
Expand Down
2 changes: 1 addition & 1 deletion mobile/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -262,5 +262,5 @@ common:remote-ci-debug --action_env=VERBOSE_COVERAGE=true
common:remote-ci-debug --test_env=VERBOSE_COVERAGE=true
common:remote-ci-debug --test_env=DISPLAY_LCOV_CMD=true
#############################################################################
# Experimental EngFlow Remote Execution Configs
# Experimental EngFlow Remote Execution Configs.
#############################################################################
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ void Utility::extractCommonAccessLogProperties(
*stream_info.downstreamAddressProvider().localAddress(),
*common_access_log.mutable_downstream_local_address());
}
if (stream_info.downstreamAddressProvider().requestedServerName() != nullptr) {
if (!stream_info.downstreamAddressProvider().requestedServerName().empty()) {
common_access_log.mutable_tls_properties()->set_tls_sni_hostname(
std::string(stream_info.downstreamAddressProvider().requestedServerName()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ namespace Http {
namespace HeaderValidators {
namespace EnvoyDefault {

namespace {

template <typename IntType>
std::from_chars_result fromChars(const absl::string_view string_value, IntType& value) {
return std::from_chars(string_value.data(), string_value.data() + string_value.size(), value);
}

} // namespace

using ::envoy::extensions::http::header_validators::envoy_default::v3::HeaderValidatorConfig;
using ::Envoy::Http::HeaderString;
using ::Envoy::Http::Protocol;
Expand Down Expand Up @@ -140,8 +149,9 @@ HeaderValidator::validateStatusHeader(const HeaderString& value) {

// Convert the status to an integer.
std::uint32_t status_value{};
auto result = std::from_chars(value_string_view.begin(), value_string_view.end(), status_value);
if (result.ec != std::errc() || result.ptr != value_string_view.end()) {
auto result = fromChars(value_string_view, status_value);
if (result.ec != std::errc() ||
result.ptr != (value_string_view.data() + value_string_view.size())) {
return {HeaderValueValidationResult::Action::Reject,
UhvResponseCodeDetail::get().InvalidStatus};
}
Expand Down Expand Up @@ -246,16 +256,17 @@ HeaderValidator::validateContentLengthHeader(const HeaderString& value) {
//
// Content-Length = 1*DIGIT
// TODO(#23315) - Validate multiple Content-Length values
const auto& value_string_view = value.getStringView();
const auto value_string_view = value.getStringView();

if (value_string_view.empty()) {
return {HeaderValueValidationResult::Action::Reject,
UhvResponseCodeDetail::get().InvalidContentLength};
}

std::uint64_t int_value{};
auto result = std::from_chars(value_string_view.begin(), value_string_view.end(), int_value);
if (result.ec != std::errc() || result.ptr != value_string_view.end()) {
auto result = fromChars(value_string_view, int_value);
if (result.ec != std::errc() ||
result.ptr != (value_string_view.data() + value_string_view.size())) {
return {HeaderValueValidationResult::Action::Reject,
UhvResponseCodeDetail::get().InvalidContentLength};
}
Expand Down Expand Up @@ -306,9 +317,9 @@ HeaderValidator::validateHostHeader(const HeaderString& value) {
} else {
// parse the port number
std::uint16_t port_int{};
auto result = std::from_chars(std::next(port_string.begin()), port_string.end(), port_int);

if (result.ec != std::errc() || result.ptr != port_string.end() || port_int == 0) {
auto result = fromChars(port_string.substr(1), port_int);
if (result.ec != std::errc() || result.ptr != (port_string.data() + port_string.size()) ||
port_int == 0) {
is_valid = false;
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/common/api/os_sys_calls_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
os_fd_t fd = open_result.return_value_;
#ifdef WIN32
// `pwrite` and `pread` are not supported. Just write some bytes so we can still test stat.
EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.begin(), file_contents.size()));
EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.data(), file_contents.size()));
#else
// Test `pwrite`
Api::SysCallSizeResult write_result =
os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
os_syscalls.pwrite(fd, file_contents.data(), file_contents.size(), 0);
EXPECT_EQ(write_result.return_value_, file_contents.size());
EXPECT_EQ(write_result.errno_, 0);
// Test `pread`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ TEST_P(KillRequestFilterIntegrationTestAllProtocols, KillRequestCrashEnvoy) {

// Disabled for coverage per #18569
#if !defined(ENVOY_CONFIG_COVERAGE)
// KillRequestCrashEnvoyOnResponse is flaky on Windows
#ifndef WIN32
// Request crash Envoy controlled via response.
TEST_P(KillRequestFilterIntegrationTestAllProtocols, KillRequestCrashEnvoyOnResponse) {
const std::string filter_config_response =
Expand Down Expand Up @@ -75,6 +77,7 @@ TEST_P(KillRequestFilterIntegrationTestAllProtocols, KillRequestCrashEnvoyOnResp
EXPECT_DEATH(sendRequestAndWaitForResponse(request_headers, 0, kill_response_headers, 1024),
"KillRequestFilter is crashing Envoy!!!");
}
#endif

TEST_P(KillRequestFilterIntegrationTestAllProtocols, KillRequestCrashEnvoyWithCustomKillHeader) {
const std::string filter_config_with_custom_kill_header =
Expand Down

0 comments on commit 525ff42

Please sign in to comment.