From c3b344ef5adb2a1b493d86fa4730e28aeb505edf Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 6 Dec 2022 11:43:37 -0500 Subject: [PATCH 1/8] build(deps): bump abseil-cpp to latest version This also allows us to remove the mobile override and its patch because the new version of abseil-cpp pulls in https://github.com/google/cctz/pull/237. Fixes https://github.com/envoyproxy/envoy-mobile/issues/136. Signed-off-by: JP Simard --- bazel/repository_locations.bzl | 6 +++--- mobile/bazel/abseil.patch | 11 ----------- mobile/bazel/envoy_mobile_repositories.bzl | 13 ------------- 3 files changed, 3 insertions(+), 27 deletions(-) delete mode 100644 mobile/bazel/abseil.patch diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index bd800d7855151..af02070b21be9 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -150,12 +150,12 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "Abseil", project_desc = "Open source collection of C++ libraries drawn from the most fundamental pieces of Google’s internal codebase", project_url = "https://abseil.io/", - version = "8317b9a01cbc32594ad4bf971709c97cb13ec921", - sha256 = "f772811a2d5dff79da54e24d6d4f5f5bc06d49e4d4ef4107f74a6c361edada99", + version = "6dab0bd99c5856e747fe35e928395ae80ca5e5f1", + sha256 = "356d194d166c82695ee74be6cd76d3367177bf400bd22a7904f24811c11729e0", strip_prefix = "abseil-cpp-{version}", urls = ["https://github.com/abseil/abseil-cpp/archive/{version}.tar.gz"], use_category = ["dataplane_core", "controlplane"], - release_date = "2022-10-06", + release_date = "2022-12-06", cpe = "N/A", license = "Apache-2.0", license_url = "https://github.com/abseil/abseil-cpp/blob/{version}/LICENSE", diff --git a/mobile/bazel/abseil.patch b/mobile/bazel/abseil.patch deleted file mode 100644 index 51bd90fa6e232..0000000000000 --- a/mobile/bazel/abseil.patch +++ /dev/null @@ -1,11 +0,0 @@ ---- absl/time/internal/cctz/BUILD.bazel -+++ absl/time/internal/cctz/BUILD.bazel -@@ -76,7 +76,7 @@ cc_library( - ], - linkopts = select({ - ":osx": [ -- "-framework Foundation", -+ # "-framework Foundation", - ], - ":ios": [ - "-framework Foundation", diff --git a/mobile/bazel/envoy_mobile_repositories.bzl b/mobile/bazel/envoy_mobile_repositories.bzl index f53eaaeb3d699..9f0223cb45e90 100644 --- a/mobile/bazel/envoy_mobile_repositories.bzl +++ b/mobile/bazel/envoy_mobile_repositories.bzl @@ -38,19 +38,6 @@ def upstream_envoy_overrides(): build_file_content = """filegroup(name = "all", srcs = glob(["**"]), visibility = ["//visibility:public"])""", ) - # Patch upstream Abseil to prevent Foundation dependency from leaking into Android builds. - # Workaround for https://github.com/abseil/abseil-cpp/issues/326. - # TODO: Should be removed in https://github.com/envoyproxy/envoy-mobile/issues/136 once rules_android - # supports platform toolchains. - http_archive( - name = "com_google_absl", - patches = ["@envoy_mobile//bazel:abseil.patch"], - sha256 = "3a0bb3d2e6f53352526a8d1a7e7b5749c68cd07f2401766a404fb00d2853fa49", - strip_prefix = "abseil-cpp-4bbdb026899fea9f882a95cbd7d6a4adaf49b2dd", - # 2022-07-05 - urls = ["https://github.com/abseil/abseil-cpp/archive/4bbdb026899fea9f882a95cbd7d6a4adaf49b2dd.tar.gz"], - ) - # This should be kept in sync with Envoy itself, we just need to apply this patch # Remove this once https://boringssl-review.googlesource.com/c/boringssl/+/37804 is in master-with-bazel http_archive( From 04e567261010b6c83f5ef6f9a2e878bde88453b0 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 6 Dec 2022 12:18:11 -0500 Subject: [PATCH 2/8] Fix `release_date` Signed-off-by: JP Simard --- bazel/repository_locations.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index af02070b21be9..b3129b61be0f3 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -155,7 +155,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( strip_prefix = "abseil-cpp-{version}", urls = ["https://github.com/abseil/abseil-cpp/archive/{version}.tar.gz"], use_category = ["dataplane_core", "controlplane"], - release_date = "2022-12-06", + release_date = "2022-12-05", cpe = "N/A", license = "Apache-2.0", license_url = "https://github.com/abseil/abseil-cpp/blob/{version}/LICENSE", From 70dad2a9c1ee176ce902e3663fc256b002a9d6d4 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Wed, 7 Dec 2022 22:21:17 -0500 Subject: [PATCH 3/8] Bump again I'm hoping that https://github.com/abseil/abseil-cpp/commit/5736d76ae60c1746cf6fe60466df655c91bf7a7b fixes the asan failure we're seeing. Signed-off-by: JP Simard --- bazel/repository_locations.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 9a2ee91f2c381..479cbf6f3926f 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -150,12 +150,12 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "Abseil", project_desc = "Open source collection of C++ libraries drawn from the most fundamental pieces of Google’s internal codebase", project_url = "https://abseil.io/", - version = "6dab0bd99c5856e747fe35e928395ae80ca5e5f1", - sha256 = "356d194d166c82695ee74be6cd76d3367177bf400bd22a7904f24811c11729e0", + version = "9bff2a9302a8dbf91712fc215eb2e2cf8ec234e7", + sha256 = "ae959138730b55b3fb968d3c357e740e7ffdeab4648dc3eb28843a1e9fa56b57", strip_prefix = "abseil-cpp-{version}", urls = ["https://github.com/abseil/abseil-cpp/archive/{version}.tar.gz"], use_category = ["dataplane_core", "controlplane"], - release_date = "2022-12-05", + release_date = "2022-12-07", cpe = "N/A", license = "Apache-2.0", license_url = "https://github.com/abseil/abseil-cpp/blob/{version}/LICENSE", From cad2d50e011b76794c898ca72796b56f2c425dd5 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Thu, 8 Dec 2022 10:21:15 -0500 Subject: [PATCH 4/8] Debugging: Use `memcpy(...)` Signed-off-by: JP Simard --- source/common/http/BUILD | 1 + source/common/http/status.cc | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index fb3beedd9febb..9f9acbb84378a 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -566,5 +566,6 @@ envoy_cc_library( deps = [ "//envoy/http:codes_interface", "//source/common/common:assert_lib", + "//source/common/common:safe_memcpy_lib", ], ) diff --git a/source/common/http/status.cc b/source/common/http/status.cc index 2ce7e3318253e..f11f38e3d0a82 100644 --- a/source/common/http/status.cc +++ b/source/common/http/status.cc @@ -1,6 +1,7 @@ #include "source/common/http/status.h" #include "source/common/common/assert.h" +#include "source/common/common/safe_memcpy.h" #include "absl/strings/str_cat.h" @@ -41,7 +42,10 @@ struct PrematureResponsePayload : public EnvoyStatusPayload { }; template void storePayload(absl::Status& status, const T& payload) { - absl::Cord cord(absl::string_view(reinterpret_cast(&payload), sizeof(payload))); + const size_t payload_size = sizeof(payload); + char* buffer = new char[payload_size]; + safeMemcpyUnsafeDst(buffer, &payload); + absl::Cord cord(absl::string_view(buffer, payload_size)); cord.Flatten(); // Flatten ahead of time for easier access later. status.SetPayload(EnvoyPayloadUrl, std::move(cord)); } @@ -57,7 +61,9 @@ template const T& getPayload(const absl::Statu auto data = cord.TryFlat(); ASSERT(data.has_value()); // EnvoyPayloadUrl cords are flattened ahead of time ASSERT(data.value().length() >= sizeof(T), "Invalid payload length"); - payload = reinterpret_cast(data.value().data()); + char* buffer = new char[sizeof(T)]; + memcpy(buffer, data.value().data(), sizeof(T)); // NOLINT(safe-memcpy) + payload = reinterpret_cast(buffer); } }); ASSERT(payload); From 869dbd211eedd2d3d2d01bb282b6b168a5fa19d1 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Thu, 8 Dec 2022 11:28:15 -0500 Subject: [PATCH 5/8] Fix buffer leak Signed-off-by: JP Simard --- source/common/http/status.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/http/status.cc b/source/common/http/status.cc index f11f38e3d0a82..385b5d15b0e25 100644 --- a/source/common/http/status.cc +++ b/source/common/http/status.cc @@ -48,6 +48,7 @@ template void storePayload(absl::Status& status, const T& payload) absl::Cord cord(absl::string_view(buffer, payload_size)); cord.Flatten(); // Flatten ahead of time for easier access later. status.SetPayload(EnvoyPayloadUrl, std::move(cord)); + free(buffer); } template const T& getPayload(const absl::Status& status) { From 68c52cfacc20ecff8f9e242c76d24ae616565256 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 12 Dec 2022 11:30:55 -0500 Subject: [PATCH 6/8] Apply suggestions from code review Signed-off-by: JP Simard --- source/common/http/status.cc | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/source/common/http/status.cc b/source/common/http/status.cc index 385b5d15b0e25..a9c0465d1b1f5 100644 --- a/source/common/http/status.cc +++ b/source/common/http/status.cc @@ -1,7 +1,6 @@ #include "source/common/http/status.h" #include "source/common/common/assert.h" -#include "source/common/common/safe_memcpy.h" #include "absl/strings/str_cat.h" @@ -42,13 +41,11 @@ struct PrematureResponsePayload : public EnvoyStatusPayload { }; template void storePayload(absl::Status& status, const T& payload) { - const size_t payload_size = sizeof(payload); - char* buffer = new char[payload_size]; - safeMemcpyUnsafeDst(buffer, &payload); - absl::Cord cord(absl::string_view(buffer, payload_size)); + const T* allocated = new T(payload); + const absl::string_view sv = absl::string_view(reinterpret_cast(allocated), sizeof(allocated)); + absl::Cord cord = absl::MakeCordFromExternal(sv, [allocated]() { delete allocated; }); cord.Flatten(); // Flatten ahead of time for easier access later. status.SetPayload(EnvoyPayloadUrl, std::move(cord)); - free(buffer); } template const T& getPayload(const absl::Status& status) { @@ -62,9 +59,7 @@ template const T& getPayload(const absl::Statu auto data = cord.TryFlat(); ASSERT(data.has_value()); // EnvoyPayloadUrl cords are flattened ahead of time ASSERT(data.value().length() >= sizeof(T), "Invalid payload length"); - char* buffer = new char[sizeof(T)]; - memcpy(buffer, data.value().data(), sizeof(T)); // NOLINT(safe-memcpy) - payload = reinterpret_cast(buffer); + payload = reinterpret_cast(data.value().data()); } }); ASSERT(payload); From 1bede3f817c5f56c6b57d45cb13713646b5d17a7 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 12 Dec 2022 11:50:11 -0500 Subject: [PATCH 7/8] fixup! Apply suggestions from code review Signed-off-by: JP Simard --- source/common/http/status.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/common/http/status.cc b/source/common/http/status.cc index a9c0465d1b1f5..c3500fcae9f9d 100644 --- a/source/common/http/status.cc +++ b/source/common/http/status.cc @@ -42,7 +42,8 @@ struct PrematureResponsePayload : public EnvoyStatusPayload { template void storePayload(absl::Status& status, const T& payload) { const T* allocated = new T(payload); - const absl::string_view sv = absl::string_view(reinterpret_cast(allocated), sizeof(allocated)); + const absl::string_view sv = + absl::string_view(reinterpret_cast(allocated), sizeof(allocated)); absl::Cord cord = absl::MakeCordFromExternal(sv, [allocated]() { delete allocated; }); cord.Flatten(); // Flatten ahead of time for easier access later. status.SetPayload(EnvoyPayloadUrl, std::move(cord)); From 2b5e093eabb9e345cffbf9a715bc0568e4d77244 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 12 Dec 2022 12:31:08 -0500 Subject: [PATCH 8/8] fixup! fixup! Apply suggestions from code review Signed-off-by: JP Simard --- source/common/http/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 9f9acbb84378a..fb3beedd9febb 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -566,6 +566,5 @@ envoy_cc_library( deps = [ "//envoy/http:codes_interface", "//source/common/common:assert_lib", - "//source/common/common:safe_memcpy_lib", ], )