Use safe_malloc instead of new when creating new_envoy_map_entry (#2632)

Use safe_malloc instead of new when creating new_envoy_map_entry
since release_envoy_map uses free instead of delete. Found when
building this code internally with some sort of strict checking enabled

Signed-off-by: Ryan Hamilton rch@google.com

Risk Level: Low
Testing: Existing tests
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: JP Simard <jp@jpsim.com>

mobile/library/common/bridge/utility.h

@@ -18,7 +18,8 @@ envoy_error_code_t errorCodeFromLocalStatus(Http::Code status);
 template <class T> envoy_map makeEnvoyMap(const T& map) {
   envoy_map new_map;
   new_map.length = std::size(map);
-  new_map.entries = new envoy_map_entry[std::size(map)];
+  new_map.entries =
+      static_cast<envoy_map_entry*>(safe_malloc(sizeof(envoy_map_entry) * std::size(map)));
 
   uint64_t i = 0;
   for (const auto& e : map) {

mobile/test/common/http/BUILD

@@ -29,6 +29,7 @@ envoy_cc_test(
     srcs = ["header_utility_test.cc"],
     repository = "@envoy",
     deps = [
+        "//library/common/bridge:utility_lib",
         "//library/common/data:utility_lib",
         "//library/common/http:header_utility_lib",
         "//library/common/types:c_types_lib",

mobile/test/common/http/header_utility_test.cc

@@ -1,6 +1,7 @@
 #include "source/common/http/header_map_impl.h"
 
 #include "gtest/gtest.h"
+#include "library/common/bridge/utility.h"
 #include "library/common/data/utility.h"
 #include "library/common/http/header_utility.h"
 #include "library/common/types/c_types.h"
@@ -18,17 +19,17 @@ envoy_data envoyTestString(std::string& s, uint32_t* sentinel) {
 }
 
 TEST(RequestHeaderDataConstructorTest, FromCToCppEmpty) {
-  envoy_map_entry* header_array = new envoy_map_entry[0];
-  envoy_headers empty_headers = {0, header_array};
+  std::map<std::string, std::string> empty_map;
+  envoy_headers empty_headers = Bridge::Utility::makeEnvoyMap(empty_map);
 
   RequestHeaderMapPtr cpp_headers = Utility::toRequestHeaders(empty_headers);
 
   ASSERT_TRUE(cpp_headers->empty());
 }
 
 TEST(RequestTrailerDataConstructorTest, FromCToCppEmpty) {
-  envoy_map_entry* header_array = new envoy_map_entry[0];
-  envoy_headers empty_trailers = {0, header_array};
+  std::map<std::string, std::string> empty_map;
+  envoy_headers empty_trailers = Bridge::Utility::makeEnvoyMap(empty_map);
 
   RequestTrailerMapPtr cpp_trailers = Utility::toRequestTrailers(empty_trailers);
 
@@ -40,7 +41,8 @@ TEST(RequestHeaderDataConstructorTest, FromCToCpp) {
   std::vector<std::pair<std::string, std::string>> headers = {
       {":method", "GET"}, {":scheme", "https"}, {":authority", "api.lyft.com"}, {":path", "/ping"}};
 
-  envoy_map_entry* header_array = new envoy_map_entry[headers.size()];
+  envoy_map_entry* header_array =
+      static_cast<envoy_map_entry*>(safe_malloc(sizeof(envoy_map_entry) * headers.size()));
 
   uint32_t* sentinel = new uint32_t;
   *sentinel = 0;
@@ -81,7 +83,8 @@ TEST(RequestTrailerDataConstructorTest, FromCToCpp) {
   std::vector<std::pair<std::string, std::string>> trailers = {
       {"processing-duration-ms", "25"}, {"response-compression-ratio", "0.61"}};
 
-  envoy_map_entry* header_array = new envoy_map_entry[trailers.size()];
+  envoy_map_entry* header_array =
+      static_cast<envoy_map_entry*>(safe_malloc(sizeof(envoy_map_entry) * trailers.size()));
 
   uint32_t* sentinel = new uint32_t;
   *sentinel = 0;
@@ -105,9 +108,8 @@ TEST(RequestTrailerDataConstructorTest, FromCToCpp) {
   ASSERT_EQ(cpp_trailers->size(), c_trailers_copy.length);
 
   for (envoy_map_size_t i = 0; i < c_trailers_copy.length; i++) {
-    auto expected_key =
-        LowerCaseString(Data::Utility::copyToString(c_trailers_copy.entries[i].key));
-    auto expected_value = Data::Utility::copyToString(c_trailers_copy.entries[i].value);
+    LowerCaseString expected_key(Data::Utility::copyToString(c_trailers_copy.entries[i].key));
+    std::string expected_value = Data::Utility::copyToString(c_trailers_copy.entries[i].value);
 
     // Key is present.
     EXPECT_FALSE(cpp_trailers->get(expected_key).empty());
@@ -137,8 +139,8 @@ TEST(HeaderDataConstructorTest, FromCppToC) {
   ASSERT_EQ(c_headers.length, static_cast<envoy_map_size_t>(cpp_headers->size()));
 
   for (envoy_map_size_t i = 0; i < c_headers.length; i++) {
-    auto actual_key = LowerCaseString(Data::Utility::copyToString(c_headers.entries[i].key));
-    auto actual_value = Data::Utility::copyToString(c_headers.entries[i].value);
+    LowerCaseString actual_key(Data::Utility::copyToString(c_headers.entries[i].key));
+    std::string actual_value = Data::Utility::copyToString(c_headers.entries[i].value);
 
     // Key is present.
     EXPECT_FALSE(cpp_headers->get(actual_key).empty());
@@ -162,8 +164,8 @@ TEST(HeaderDataConstructorTest, FromCppToCWithAlpn) {
   ASSERT_EQ(c_headers.length, static_cast<envoy_map_size_t>(cpp_headers->size()));
 
   for (envoy_map_size_t i = 0; i < c_headers.length; i++) {
-    auto actual_key = LowerCaseString(Data::Utility::copyToString(c_headers.entries[i].key));
-    auto actual_value = Data::Utility::copyToString(c_headers.entries[i].value);
+    LowerCaseString actual_key(Data::Utility::copyToString(c_headers.entries[i].key));
+    std::string actual_value = Data::Utility::copyToString(c_headers.entries[i].value);
 
     // Key is present.
     EXPECT_FALSE(cpp_headers->get(actual_key).empty());