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 proxy-wasm#294 Signed-off-by: Konstantin Maksimov <konstantin.maksimov@ibm.com>
knm3000 · Sep 30, 2022 · d0a3249 · d0a3249
1 parent dec6b94
commit d0a3249
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 29 deletions.
diff --git 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<const uint32_t *>(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,19 +136,19 @@ Pairs PairsUtil::toPairs(std::string_view buffer) {
       return {};
     }
     s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
-                      contextOrEffectiveContext() == nullptr
-                          ? false
-                          : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder());
+                      contextOrEffectiveContext() != nullptr
+                          ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                          : false);
     pos += sizeof(uint32_t);
 
     // Read value length.
     if (pos + sizeof(uint32_t) > end) {
       return {};
     }
     s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(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
@@ -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
@@ -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<char> 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
@@ -0,0 +1,33 @@
+// 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");
+  auto size_str = std::to_string(data1.size());
+  pairs1.push_back({data1, size_str});
+  std::vector<char> 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