From 6e273eb741392b98f23a6e1ae29488c4ab1f74b9 Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Fri, 9 Sep 2022 18:36:07 +0200 Subject: [PATCH] Do not reverse bytes for nullvm additional changes: - moved the call to isWasmByteOrder() into htowasm/wasmtoh macros - added surrounding brackets to htowasm/wasmtoh - removed debug leftover Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- include/proxy-wasm/pairs_util.h | 9 ++++---- include/proxy-wasm/word.h | 5 ++--- src/exports.cc | 20 +++++++----------- src/pairs_util.cc | 37 ++++++++++++++++++++++++--------- test/fuzz/pairs_util_fuzzer.cc | 4 ++-- test/null_vm_test.cc | 7 ++----- 6 files changed, 45 insertions(+), 37 deletions(-) diff --git a/include/proxy-wasm/pairs_util.h b/include/proxy-wasm/pairs_util.h index de1854ff..54df2fd8 100644 --- a/include/proxy-wasm/pairs_util.h +++ b/include/proxy-wasm/pairs_util.h @@ -45,12 +45,11 @@ class PairsUtil { * @param size size of the output buffer. * @return indicates whether serialization succeeded or not. */ - static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size, bool is_wasm_byte_order); + static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size); - static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size, - bool is_wasm_byte_order) { + static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size) { Pairs views(stringpairs.begin(), stringpairs.end()); - return marshalPairs(views, buffer, size, is_wasm_byte_order); + return marshalPairs(views, buffer, size); } /** @@ -58,7 +57,7 @@ class PairsUtil { * @param buffer serialized input buffer. * @return deserialized Pairs or an empty instance in case of deserialization failure. */ - static Pairs toPairs(std::string_view buffer, bool is_wasm_byte_order); + static Pairs toPairs(std::string_view buffer); }; } // namespace proxy_wasm diff --git a/include/proxy-wasm/word.h b/include/proxy-wasm/word.h index 1af3e4ac..f3549d48 100644 --- a/include/proxy-wasm/word.h +++ b/include/proxy-wasm/word.h @@ -24,9 +24,8 @@ namespace proxy_wasm { // Use byteswap functions only when compiling for big-endian platforms. #if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ -#define htowasm1111(x, is_null) is_null ? (x) : __builtin_bswap32(x) -#define htowasm(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) -#define wasmtoh(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) +#define htowasm(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x)) +#define wasmtoh(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x)) #else #define htowasm(x, is_wasm_byte_order) (x) #define wasmtoh(x, is_wasm_byte_order) (x) diff --git a/src/exports.cc b/src/exports.cc index c8ed9e2c..fbf63305 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -152,8 +152,7 @@ Word send_local_response(Word response_code, Word response_code_details_ptr, if (!details || !body || !additional_response_header_pairs) { return WasmResult::InvalidMemoryAccess; } - auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value(), - context->wasmVm()->isWasmByteOrder()); + auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value()); context->sendLocalResponse(response_code, body.value(), std::move(additional_headers), grpc_status, details.value()); context->wasm()->stopNextIteration(true); @@ -400,7 +399,7 @@ Word get_header_map_pairs(Word type, Word ptr_ptr, Word size_ptr) { if (buffer == nullptr) { return WasmResult::InvalidMemoryAccess; } - if (!PairsUtil::marshalPairs(pairs, buffer, size, context->wasmVm()->isWasmByteOrder())) { + if (!PairsUtil::marshalPairs(pairs, buffer, size)) { return WasmResult::InvalidMemoryAccess; } if (!context->wasmVm()->setWord(ptr_ptr, Word(ptr))) { @@ -421,9 +420,8 @@ Word set_header_map_pairs(Word type, Word ptr, Word size) { if (!data) { return WasmResult::InvalidMemoryAccess; } - return context->setHeaderMapPairs( - static_cast(type.u64_), - PairsUtil::toPairs(data.value(), context->wasmVm()->isWasmByteOrder())); + return context->setHeaderMapPairs(static_cast(type.u64_), + PairsUtil::toPairs(data.value())); } Word get_header_map_size(Word type, Word result_ptr) { @@ -521,8 +519,8 @@ Word http_call(Word uri_ptr, Word uri_size, Word header_pairs_ptr, Word header_p if (!uri || !body || !header_pairs || !trailer_pairs) { return WasmResult::InvalidMemoryAccess; } - auto headers = PairsUtil::toPairs(header_pairs.value(), context->wasmVm()->isWasmByteOrder()); - auto trailers = PairsUtil::toPairs(trailer_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto headers = PairsUtil::toPairs(header_pairs.value()); + auto trailers = PairsUtil::toPairs(trailer_pairs.value()); uint32_t token = 0; // NB: try to write the token to verify the memory before starting the async // operation. @@ -591,8 +589,7 @@ Word grpc_call(Word service_ptr, Word service_size, Word service_name_ptr, Word return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = - PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); auto result = context->grpcCall(service.value(), service_name.value(), method_name.value(), initial_metadata, request.value(), std::chrono::milliseconds(timeout_milliseconds), &token); @@ -618,8 +615,7 @@ Word grpc_stream(Word service_ptr, Word service_size, Word service_name_ptr, Wor return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = - PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); auto result = context->grpcStream(service.value(), service_name.value(), method_name.value(), initial_metadata, &token); if (result != WasmResult::Ok) { diff --git a/src/pairs_util.cc b/src/pairs_util.cc index a6a75d04..3b711571 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -19,8 +19,8 @@ #include #include +#include "include/proxy-wasm/exports.h" #include "include/proxy-wasm/limits.h" -#include "include/proxy-wasm/word.h" namespace proxy_wasm { @@ -36,8 +36,7 @@ size_t PairsUtil::pairsSize(const Pairs &pairs) { return size; } -bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, - [[maybe_unused]] bool is_wasm_byte_order) { +bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { if (buffer == nullptr) { return false; } @@ -46,7 +45,10 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, const char *end = buffer + size; // Write number of pairs. - uint32_t num_pairs = htowasm(pairs.size(), is_wasm_byte_order); + uint32_t num_pairs = + htowasm(pairs.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -55,7 +57,10 @@ 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(), is_wasm_byte_order); + uint32_t name_len = + htowasm(p.first.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -63,7 +68,10 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, pos += sizeof(uint32_t); // Write value length. - uint32_t value_len = htowasm(p.second.size(), is_wasm_byte_order); + uint32_t value_len = + htowasm(p.second.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -92,7 +100,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, return pos == end; } -Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_byte_order) { +Pairs PairsUtil::toPairs(std::string_view buffer) { if (buffer.data() == nullptr || buffer.size() > PROXY_WASM_HOST_PAIRS_MAX_BYTES) { return {}; } @@ -104,7 +112,10 @@ Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_ if (pos + sizeof(uint32_t) > end) { return {}; } - uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -123,14 +134,20 @@ Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_ if (pos + sizeof(uint32_t) > end) { return {}; } - s.first = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + s.first = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); // Read value length. if (pos + sizeof(uint32_t) > end) { return {}; } - s.second = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + s.second = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); } diff --git a/test/fuzz/pairs_util_fuzzer.cc b/test/fuzz/pairs_util_fuzzer.cc index 9bda6236..aaef0d09 100644 --- a/test/fuzz/pairs_util_fuzzer.cc +++ b/test/fuzz/pairs_util_fuzzer.cc @@ -22,7 +22,7 @@ namespace { extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { auto input = std::string_view(reinterpret_cast(data), size); - auto pairs = PairsUtil::toPairs(input, true); + auto pairs = PairsUtil::toPairs(input); if (!pairs.empty()) { // Verify that non-empty Pairs serializes back to the same bytes. @@ -31,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { __builtin_trap(); } std::vector new_data(new_size); - if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size(), true)) { + if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size())) { __builtin_trap(); } if (::memcmp(new_data.data(), data, size) != 0) { diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index e2db0346..6aa20492 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -84,11 +84,8 @@ TEST_F(BaseVmTest, ByteOrder) { std::string data1("some_data"); pairs1.push_back({data1.data(), std::to_string(data1.size())}); std::vector buffer(PairsUtil::pairsSize(pairs1)); - // encode using null_vm byte order - EXPECT_TRUE( - PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size(), wasm_vm->isWasmByteOrder())); - // decode using host byte order - auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()), false); + 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