From c7a88c479fba3c7ee039e426ba6085b344a8759a Mon Sep 17 00:00:00 2001
From: karusher <mark.klein@mail.mcgill.ca>
Date: Wed, 14 Feb 2024 03:59:31 -0500
Subject: [PATCH] [API] Propagation: fix for hex conversion to binary for odd
 hex strings (#2533)

---
 .../trace/propagation/detail/hex.h            | 12 +++---
 api/test/trace/propagation/CMakeLists.txt     |  2 +
 api/test/trace/propagation/detail/BUILD       | 20 +++++++++
 .../trace/propagation/detail/CMakeLists.txt   | 12 ++++++
 api/test/trace/propagation/detail/hex_test.cc | 42 +++++++++++++++++++
 5 files changed, 83 insertions(+), 5 deletions(-)
 create mode 100644 api/test/trace/propagation/detail/BUILD
 create mode 100644 api/test/trace/propagation/detail/CMakeLists.txt
 create mode 100644 api/test/trace/propagation/detail/hex_test.cc

diff --git a/api/include/opentelemetry/trace/propagation/detail/hex.h b/api/include/opentelemetry/trace/propagation/detail/hex.h
index 157f5ec42a..3bf66e84bc 100644
--- a/api/include/opentelemetry/trace/propagation/detail/hex.h
+++ b/api/include/opentelemetry/trace/propagation/detail/hex.h
@@ -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;
diff --git a/api/test/trace/propagation/CMakeLists.txt b/api/test/trace/propagation/CMakeLists.txt
index e52957342a..a1cfcf83ee 100644
--- a/api/test/trace/propagation/CMakeLists.txt
+++ b/api/test/trace/propagation/CMakeLists.txt
@@ -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}
diff --git a/api/test/trace/propagation/detail/BUILD b/api/test/trace/propagation/detail/BUILD
new file mode 100644
index 0000000000..a082586650
--- /dev/null
+++ b/api/test/trace/propagation/detail/BUILD
@@ -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",
+    ],
+)
diff --git a/api/test/trace/propagation/detail/CMakeLists.txt b/api/test/trace/propagation/detail/CMakeLists.txt
new file mode 100644
index 0000000000..34e2f6ae40
--- /dev/null
+++ b/api/test/trace/propagation/detail/CMakeLists.txt
@@ -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()
diff --git a/api/test/trace/propagation/detail/hex_test.cc b/api/test/trace/propagation/detail/hex_test.cc
new file mode 100644
index 0000000000..f4bd765f48
--- /dev/null
+++ b/api/test/trace/propagation/detail/hex_test.cc
@@ -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]);
+  }
+}