From 65c8c8ffca96c3b4805e061ca3ff08327829014a Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Fri, 30 Sep 2022 18:15:26 +0200 Subject: [PATCH] Do not reverse bytes for nullvm - moved pairs test into a new file test/pairs_util_test.cc - run pairs test on all platforms - made contextOrEffectiveContext() null check a bit more readable Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- src/pairs_util.cc | 36 ++++++++++++++++++------------------ test/BUILD | 11 +++++++++++ test/null_vm_test.cc | 11 ----------- test/pairs_util_test.cc | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 29 deletions(-) create mode 100644 test/pairs_util_test.cc diff --git a/src/pairs_util.cc b/src/pairs_util.cc index 7ef2acb76..d21135788 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -47,9 +47,9 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { // Write number of pairs. uint32_t num_pairs = - htowasm(pairs.size(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(pairs.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -59,9 +59,9 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { for (const auto &p : pairs) { // Write name length. uint32_t name_len = - htowasm(p.first.size(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(p.first.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -70,9 +70,9 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { // Write value length. uint32_t value_len = - htowasm(p.second.size(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(p.second.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -114,9 +114,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -136,9 +136,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } s.first = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); // Read value length. @@ -146,9 +146,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } s.second = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); } diff --git a/test/BUILD b/test/BUILD index 42748b92d..71b30296b 100644 --- a/test/BUILD +++ b/test/BUILD @@ -165,6 +165,17 @@ cc_test( ], ) +cc_test( + name = "pairs_util_test", + srcs = ["pairs_util_test.cc"], + linkstatic = 1, + deps = [ + "//:lib", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "utility_lib", testonly = True, diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index d7ecb3720..8bf7159f2 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -78,17 +78,6 @@ TEST_F(BaseVmTest, ByteOrder) { auto wasm_vm = createNullVm(); EXPECT_TRUE(wasm_vm->load("test_null_vm_plugin", {}, {})); EXPECT_FALSE(wasm_vm->usesWasmByteOrder()); -#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ - __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ - proxy_wasm::Pairs pairs1; - std::string data1("some_data"); - pairs1.push_back({data1.data(), std::to_string(data1.size())}); - std::vector buffer(PairsUtil::pairsSize(pairs1)); - EXPECT_TRUE(PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size())); - auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size())); - EXPECT_EQ(pairs2.size(), pairs1.size()); - EXPECT_EQ(pairs2[0].second, pairs1[0].second); -#endif } } // namespace proxy_wasm diff --git a/test/pairs_util_test.cc b/test/pairs_util_test.cc new file mode 100644 index 000000000..c136e6f56 --- /dev/null +++ b/test/pairs_util_test.cc @@ -0,0 +1,32 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "include/proxy-wasm/pairs_util.h" +#include "gtest/gtest.h" + +namespace proxy_wasm { + +TEST(PairsUtilTest, EncodeDecode) { + proxy_wasm::Pairs pairs1; + std::string data1("some_data"); + pairs1.push_back({data1.data(), std::to_string(data1.size())}); + std::vector buffer(PairsUtil::pairsSize(pairs1)); + EXPECT_TRUE(PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size())); + auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size())); + EXPECT_EQ(pairs2.size(), pairs1.size()); + EXPECT_EQ(pairs2[0].first, pairs1[0].first); + EXPECT_EQ(pairs2[0].second, pairs1[0].second); +} + +} // namespace proxy_wasm