Skip to content

Commit

Permalink
Do not reverse bytes for nullvm
Browse files Browse the repository at this point in the history 
additional changes:
- moved the call to isWasmByteOrder() into htowasm/wasmtoh macros
- added surrounding brackets to htowasm/wasmtoh
- removed debug leftover

Fixes #294

Signed-off-by: Konstantin Maksimov <konstantin.maksimov@ibm.com>
  • Loading branch information
@knm3000
knm3000 committed Sep 9, 2022
1 parent d61f579 commit 6e273eb
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 37 deletions.
9 changes: 4 additions & 5 deletions include/proxy-wasm/pairs_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,19 @@ 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);
}

/**
* toPairs deserializes input buffer to Pairs.
* @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
5 changes: 2 additions & 3 deletions include/proxy-wasm/word.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 8 additions & 12 deletions src/exports.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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))) {
Expand All @@ -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<WasmHeaderMapType>(type.u64_),
PairsUtil::toPairs(data.value(), context->wasmVm()->isWasmByteOrder()));
return context->setHeaderMapPairs(static_cast<WasmHeaderMapType>(type.u64_),
PairsUtil::toPairs(data.value()));
}

Word get_header_map_size(Word type, Word result_ptr) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
37 changes: 27 additions & 10 deletions src/pairs_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#include <string_view>
#include <vector>

#include "include/proxy-wasm/exports.h"
#include "include/proxy-wasm/limits.h"
#include "include/proxy-wasm/word.h"

namespace proxy_wasm {

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -55,15 +57,21 @@ 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;
}
::memcpy(pos, &name_len, sizeof(uint32_t));
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;
}
Expand Down Expand Up @@ -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 {};
}
Expand All @@ -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<const uint32_t *>(pos), is_wasm_byte_order);
uint32_t num_pairs = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
contextOrEffectiveContext() == nullptr
? false
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
pos += sizeof(uint32_t);

// Check if we're not going to exceed the limit.
Expand All @@ -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<const uint32_t *>(pos), is_wasm_byte_order);
s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(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<const uint32_t *>(pos), is_wasm_byte_order);
s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
contextOrEffectiveContext() == nullptr
? false
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
pos += sizeof(uint32_t);
}

Expand Down
4 changes: 2 additions & 2 deletions test/fuzz/pairs_util_fuzzer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace {
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
auto input = std::string_view(reinterpret_cast<const char *>(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.
Expand All @@ -31,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
__builtin_trap();
}
std::vector<char> 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) {
Expand Down
7 changes: 2 additions & 5 deletions test/null_vm_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<char> 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
Expand Down

0 comments on commit 6e273eb

Please sign in to comment.