Skip to content

Commit

Permalink
Fix for hex conversion to binary
Browse files Browse the repository at this point in the history
The conversion of hex to binary did not handle odd-length inputs
correctly.

The output was zero-padded on the right rather than the left. On
little endian machines, the second to last entry received the 0.
  • Loading branch information
karusher committed Feb 14, 2024
1 parent 614b8db commit daa21f7
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 5 deletions.
12 changes: 7 additions & 5 deletions api/include/opentelemetry/trace/propagation/detail/hex.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,17 @@ inline bool HexToBinary(nostd::string_view hex, uint8_t *buffer, size_t buffer_s
int64_t buffer_pos = int64_t(buffer_size) - (hex_size + 1) / 2;
int64_t last_hex_pos = hex_size - 1;

int64_t i = 0;
for (; i < last_hex_pos; i += 2)
bool is_hex_size_odd = (hex_size % 2) == 1;
int64_t i = 0;

if (is_hex_size_odd)
{
buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]);
buffer[buffer_pos++] = HexToInt(hex[i++]);
}

if (i == last_hex_pos)
for (; i < last_hex_pos; i += 2)
{
buffer[buffer_pos] = HexToInt(hex[i]);
buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]);
}

return true;
Expand Down
2 changes: 2 additions & 0 deletions api/test/trace/propagation/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

add_subdirectory(detail)

foreach(testname http_text_format_test b3_propagation_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
Expand Down
20 changes: 20 additions & 0 deletions api/test/trace/propagation/detail/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")

cc_test(
name = "hex_test",
srcs = [
"hex_test.cc",
],
tags = [
"api",
"test",
"trace",
],
deps = [
"//api",
"@com_google_googletest//:gtest_main",
],
)
12 changes: 12 additions & 0 deletions api/test/trace/propagation/detail/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

foreach(testname hex_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_api)
gtest_add_tests(
TARGET ${testname}
TEST_PREFIX trace.
TEST_LIST ${testname})
endforeach()
42 changes: 42 additions & 0 deletions api/test/trace/propagation/detail/hex_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/trace/propagation/detail/hex.h"

#include <map>

#include <gtest/gtest.h>

using namespace opentelemetry;

TEST(HexTest, ConvertOddLength)
{
const int kLength = 16;
std::string trace_id_hex = "78cfcfec62ae9e9";
uint8_t trace_id[kLength];
trace::propagation::detail::HexToBinary(trace_id_hex, trace_id, sizeof(trace_id));

const uint8_t expected_trace_id[kLength] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x7, 0x8c, 0xfc, 0xfe, 0xc6, 0x2a, 0xe9, 0xe9};

for (int i = 0; i < kLength; ++i)
{
EXPECT_EQ(trace_id[i], expected_trace_id[i]);
}
}

TEST(HexTest, ConvertEvenLength)
{
const int kLength = 16;
std::string trace_id_hex = "078cfcfec62ae9e9";
uint8_t trace_id[kLength];
trace::propagation::detail::HexToBinary(trace_id_hex, trace_id, sizeof(trace_id));

const uint8_t expected_trace_id[kLength] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x7, 0x8c, 0xfc, 0xfe, 0xc6, 0x2a, 0xe9, 0xe9};

for (int i = 0; i < kLength; ++i)
{
EXPECT_EQ(trace_id[i], expected_trace_id[i]);
}
}

0 comments on commit daa21f7

Please sign in to comment.