Skip to content

Commit 6e273eb

Browse files
knm3000knm3000
committed
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 #294 Signed-off-by: Konstantin Maksimov <konstantin.maksimov@ibm.com>
1 parent d61f579 commit 6e273eb

File tree

6 files changed

+45
-37
lines changed

6 files changed

+45
-37
lines changed

‎include/proxy-wasm/pairs_util.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,19 @@ class PairsUtil {
4545
* @param size size of the output buffer.
4646
* @return indicates whether serialization succeeded or not.
4747
*/
48-
static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size, bool is_wasm_byte_order);
48+
static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size);
4949

50-
static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size,
51-
bool is_wasm_byte_order) {
50+
static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size) {
5251
Pairs views(stringpairs.begin(), stringpairs.end());
53-
return marshalPairs(views, buffer, size, is_wasm_byte_order);
52+
return marshalPairs(views, buffer, size);
5453
}
5554

5655
/**
5756
* toPairs deserializes input buffer to Pairs.
5857
* @param buffer serialized input buffer.
5958
* @return deserialized Pairs or an empty instance in case of deserialization failure.
6059
*/
61-
static Pairs toPairs(std::string_view buffer, bool is_wasm_byte_order);
60+
static Pairs toPairs(std::string_view buffer);
6261
};
6362

6463
} // namespace proxy_wasm

‎include/proxy-wasm/word.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ namespace proxy_wasm {
2424
// Use byteswap functions only when compiling for big-endian platforms.
2525
#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \
2626
__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
27-
#define htowasm1111(x, is_null) is_null ? (x) : __builtin_bswap32(x)
28-
#define htowasm(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x)
29-
#define wasmtoh(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x)
27+
#define htowasm(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x))
28+
#define wasmtoh(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x))
3029
#else
3130
#define htowasm(x, is_wasm_byte_order) (x)
3231
#define wasmtoh(x, is_wasm_byte_order) (x)

‎src/exports.cc

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,7 @@ Word send_local_response(Word response_code, Word response_code_details_ptr,
152152
if (!details || !body || !additional_response_header_pairs) {
153153
return WasmResult::InvalidMemoryAccess;
154154
}
155-
auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value(),
156-
context->wasmVm()->isWasmByteOrder());
155+
auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value());
157156
context->sendLocalResponse(response_code, body.value(), std::move(additional_headers),
158157
grpc_status, details.value());
159158
context->wasm()->stopNextIteration(true);
@@ -400,7 +399,7 @@ Word get_header_map_pairs(Word type, Word ptr_ptr, Word size_ptr) {
400399
if (buffer == nullptr) {
401400
return WasmResult::InvalidMemoryAccess;
402401
}
403-
if (!PairsUtil::marshalPairs(pairs, buffer, size, context->wasmVm()->isWasmByteOrder())) {
402+
if (!PairsUtil::marshalPairs(pairs, buffer, size)) {
404403
return WasmResult::InvalidMemoryAccess;
405404
}
406405
if (!context->wasmVm()->setWord(ptr_ptr, Word(ptr))) {
@@ -421,9 +420,8 @@ Word set_header_map_pairs(Word type, Word ptr, Word size) {
421420
if (!data) {
422421
return WasmResult::InvalidMemoryAccess;
423422
}
424-
return context->setHeaderMapPairs(
425-
static_cast<WasmHeaderMapType>(type.u64_),
426-
PairsUtil::toPairs(data.value(), context->wasmVm()->isWasmByteOrder()));
423+
return context->setHeaderMapPairs(static_cast<WasmHeaderMapType>(type.u64_),
424+
PairsUtil::toPairs(data.value()));
427425
}
428426

429427
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
521519
if (!uri || !body || !header_pairs || !trailer_pairs) {
522520
return WasmResult::InvalidMemoryAccess;
523521
}
524-
auto headers = PairsUtil::toPairs(header_pairs.value(), context->wasmVm()->isWasmByteOrder());
525-
auto trailers = PairsUtil::toPairs(trailer_pairs.value(), context->wasmVm()->isWasmByteOrder());
522+
auto headers = PairsUtil::toPairs(header_pairs.value());
523+
auto trailers = PairsUtil::toPairs(trailer_pairs.value());
526524
uint32_t token = 0;
527525
// NB: try to write the token to verify the memory before starting the async
528526
// operation.
@@ -591,8 +589,7 @@ Word grpc_call(Word service_ptr, Word service_size, Word service_name_ptr, Word
591589
return WasmResult::InvalidMemoryAccess;
592590
}
593591
uint32_t token = 0;
594-
auto initial_metadata =
595-
PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder());
592+
auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value());
596593
auto result = context->grpcCall(service.value(), service_name.value(), method_name.value(),
597594
initial_metadata, request.value(),
598595
std::chrono::milliseconds(timeout_milliseconds), &token);
@@ -618,8 +615,7 @@ Word grpc_stream(Word service_ptr, Word service_size, Word service_name_ptr, Wor
618615
return WasmResult::InvalidMemoryAccess;
619616
}
620617
uint32_t token = 0;
621-
auto initial_metadata =
622-
PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder());
618+
auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value());
623619
auto result = context->grpcStream(service.value(), service_name.value(), method_name.value(),
624620
initial_metadata, &token);
625621
if (result != WasmResult::Ok) {

‎src/pairs_util.cc

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
#include <string_view>
2020
#include <vector>
2121

22+
#include "include/proxy-wasm/exports.h"
2223
#include "include/proxy-wasm/limits.h"
23-
#include "include/proxy-wasm/word.h"
2424

2525
namespace proxy_wasm {
2626

@@ -36,8 +36,7 @@ size_t PairsUtil::pairsSize(const Pairs &pairs) {
3636
return size;
3737
}
3838

39-
bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size,
40-
[[maybe_unused]] bool is_wasm_byte_order) {
39+
bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) {
4140
if (buffer == nullptr) {
4241
return false;
4342
}
@@ -46,7 +45,10 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size,
4645
const char *end = buffer + size;
4746

4847
// Write number of pairs.
49-
uint32_t num_pairs = htowasm(pairs.size(), is_wasm_byte_order);
48+
uint32_t num_pairs =
49+
htowasm(pairs.size(), contextOrEffectiveContext() == nullptr
50+
? false
51+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
5052
if (pos + sizeof(uint32_t) > end) {
5153
return false;
5254
}
@@ -55,15 +57,21 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size,
5557

5658
for (const auto &p : pairs) {
5759
// Write name length.
58-
uint32_t name_len = htowasm(p.first.size(), is_wasm_byte_order);
60+
uint32_t name_len =
61+
htowasm(p.first.size(), contextOrEffectiveContext() == nullptr
62+
? false
63+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
5964
if (pos + sizeof(uint32_t) > end) {
6065
return false;
6166
}
6267
::memcpy(pos, &name_len, sizeof(uint32_t));
6368
pos += sizeof(uint32_t);
6469

6570
// Write value length.
66-
uint32_t value_len = htowasm(p.second.size(), is_wasm_byte_order);
71+
uint32_t value_len =
72+
htowasm(p.second.size(), contextOrEffectiveContext() == nullptr
73+
? false
74+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
6775
if (pos + sizeof(uint32_t) > end) {
6876
return false;
6977
}
@@ -92,7 +100,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size,
92100
return pos == end;
93101
}
94102

95-
Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_byte_order) {
103+
Pairs PairsUtil::toPairs(std::string_view buffer) {
96104
if (buffer.data() == nullptr || buffer.size() > PROXY_WASM_HOST_PAIRS_MAX_BYTES) {
97105
return {};
98106
}
@@ -104,7 +112,10 @@ Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_
104112
if (pos + sizeof(uint32_t) > end) {
105113
return {};
106114
}
107-
uint32_t num_pairs = wasmtoh(*reinterpret_cast<const uint32_t *>(pos), is_wasm_byte_order);
115+
uint32_t num_pairs = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
116+
contextOrEffectiveContext() == nullptr
117+
? false
118+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
108119
pos += sizeof(uint32_t);
109120

110121
// 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_
123134
if (pos + sizeof(uint32_t) > end) {
124135
return {};
125136
}
126-
s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(pos), is_wasm_byte_order);
137+
s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
138+
contextOrEffectiveContext() == nullptr
139+
? false
140+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
127141
pos += sizeof(uint32_t);
128142

129143
// Read value length.
130144
if (pos + sizeof(uint32_t) > end) {
131145
return {};
132146
}
133-
s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(pos), is_wasm_byte_order);
147+
s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
148+
contextOrEffectiveContext() == nullptr
149+
? false
150+
: contextOrEffectiveContext()->wasmVm()->isWasmByteOrder());
134151
pos += sizeof(uint32_t);
135152
}
136153

‎test/fuzz/pairs_util_fuzzer.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace {
2222
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
2323
auto input = std::string_view(reinterpret_cast<const char *>(data), size);
2424

25-
auto pairs = PairsUtil::toPairs(input, true);
25+
auto pairs = PairsUtil::toPairs(input);
2626

2727
if (!pairs.empty()) {
2828
// 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) {
3131
__builtin_trap();
3232
}
3333
std::vector<char> new_data(new_size);
34-
if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size(), true)) {
34+
if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size())) {
3535
__builtin_trap();
3636
}
3737
if (::memcmp(new_data.data(), data, size) != 0) {

‎test/null_vm_test.cc

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,8 @@ TEST_F(BaseVmTest, ByteOrder) {
8484
std::string data1("some_data");
8585
pairs1.push_back({data1.data(), std::to_string(data1.size())});
8686
std::vector<char> buffer(PairsUtil::pairsSize(pairs1));
87-
// encode using null_vm byte order
88-
EXPECT_TRUE(
89-
PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size(), wasm_vm->isWasmByteOrder()));
90-
// decode using host byte order
91-
auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()), false);
87+
EXPECT_TRUE(PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size()));
88+
auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()));
9289
EXPECT_EQ(pairs2.size(), pairs1.size());
9390
EXPECT_EQ(pairs2[0].second, pairs1[0].second);
9491
#endif

0 commit comments

Comments
 (0)