From f823190cad1740b87f695a098a68af33df26a8c3 Mon Sep 17 00:00:00 2001 From: Lukasz Towarek Date: Sun, 18 Oct 2026 21:09:00 +0200 Subject: [PATCH 1/5] [API] Add locale-independent ASCII helpers and use them across the codebase Character classes in specifications and protocols are defined over ASCII, yet callers used , whose results depend on the global locale and which is undefined for a negative char, and several of them passed bytes from headers, configuration files and environment variables straight through. Others repeated the same range checks and hexadecimal decoding and encoding by hand. common/detail/ascii.h provides constexpr IsAsciiDigit, IsAsciiLower, IsAsciiUpper, IsAsciiAlpha, IsAsciiAlnum, IsAsciiHexDigit, IsAsciiControl, IsAsciiSpace, ToAsciiLower, ToAsciiUpper, HexDigitValue, LowerHexDigit and UpperHexDigit, plus EncodeLowerHex and EncodeUpperHex for byte sequences. TraceState, Baggage, StringUtil::Trim, EnvironmentCarrier, env_variables, DocumentNode, PercentDecode, InstrumentDescriptor comparison and hashing, the OpenTelemetry trace state sampler, the environment entity detector, the OTLP and HTTP client header maps, the curl HTTP time parser and debug log, UrlDecoder, the HTTP test server, the ETW provider GUID, the Prometheus sanitizers and the W3C test server use them. The digit table in trace/propagation/detail/hex.h becomes HexDigitValue's lookup table: propagators decode trace and span IDs on every request, and the lookup is faster there than range checks. The encoders replace the hexadecimal encoding tables and loops in TraceId, SpanId, TraceFlags, Baggage, the service detector's UUID formatter and the OTLP HTTP and file clients, and the digit helpers replace them in the trace state sampler and the ETW UUID. Each keeps its case. ascii_test checks each classifier against in the "C" locale for all 256 byte values, and each encoder against printf. --- api/include/opentelemetry/baggage/baggage.h | 33 ++-- .../opentelemetry/common/detail/ascii.h | 127 +++++++++++++ .../opentelemetry/common/string_util.h | 7 +- .../context/propagation/environment_carrier.h | 11 +- .../trace/propagation/detail/hex.h | 27 +-- api/include/opentelemetry/trace/span_id.h | 8 +- api/include/opentelemetry/trace/trace_flags.h | 5 +- api/include/opentelemetry/trace/trace_id.h | 8 +- api/include/opentelemetry/trace/trace_state.h | 4 +- api/test/common/BUILD | 15 ++ api/test/common/CMakeLists.txt | 2 +- api/test/common/ascii_test.cc | 169 ++++++++++++++++++ .../opentelemetry/exporters/etw/utils.h | 3 +- .../opentelemetry/exporters/etw/uuid.h | 68 ++++--- .../exporters/otlp/otlp_environment.h | 10 +- exporters/otlp/src/otlp_file_client.cc | 35 +--- exporters/otlp/src/otlp_http_client.cc | 36 +--- exporters/prometheus/src/exporter_utils.cc | 13 +- .../ext/http/client/http_client.h | 9 +- .../ext/http/common/url_parser.h | 15 +- .../ext/http/server/http_server.h | 7 +- .../http/client/curl/http_operation_curl.cc | 4 +- ext/src/http/client/curl/http_time_util.cc | 4 +- .../w3c_tracecontext_http_test_server/main.cc | 5 +- resource_detectors/src/env_entity_detector.cc | 29 ++- .../src/service_detector_utils.cc | 20 +-- .../opentelemetry/sdk/metrics/instruments.h | 9 +- sdk/src/common/env_variables.cc | 6 +- sdk/src/configuration/document_node.cc | 6 +- sdk/src/resource/detail/percent_decode.cc | 24 +-- sdk/src/trace/samplers/ot_trace_state.cc | 23 +-- 31 files changed, 462 insertions(+), 280 deletions(-) create mode 100644 api/include/opentelemetry/common/detail/ascii.h create mode 100644 api/test/common/ascii_test.cc diff --git a/api/include/opentelemetry/baggage/baggage.h b/api/include/opentelemetry/baggage/baggage.h index ca7f70ac47..a5a133d88d 100644 --- a/api/include/opentelemetry/baggage/baggage.h +++ b/api/include/opentelemetry/baggage/baggage.h @@ -3,8 +3,9 @@ #pragma once -#include +#include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/kv_properties.h" #include "opentelemetry/common/macros.h" #include "opentelemetry/nostd/shared_ptr.h" @@ -215,16 +216,11 @@ class OPENTELEMETRY_EXPORT Baggage // Implementation inspired from : https://golang.org/src/net/url/url.go?s=7851:7884#L264 static std::string UrlEncode(nostd::string_view str) { - auto to_hex = [](char c) -> char { - static const char *hex = "0123456789ABCDEF"; - return hex[c & 15]; - }; - std::string ret; for (auto c : str) { - if (std::isalnum(c) || c == '-' || c == '_' || c == '.' || c == '~') + if (common::detail::IsAsciiAlnum(c) || c == '-' || c == '_' || c == '.' || c == '~') { ret.push_back(c); } @@ -234,9 +230,10 @@ class OPENTELEMETRY_EXPORT Baggage } else { - ret.push_back('%'); - ret.push_back(to_hex(static_cast(c >> 4))); - ret.push_back(to_hex(static_cast(c & 15))); + const auto byte = static_cast(c); + char escaped[] = {'%', '\0', '\0'}; + common::detail::EncodeUpperHex(&byte, 1, &escaped[1]); + ret.append(escaped, sizeof(escaped)); } } @@ -246,16 +243,6 @@ class OPENTELEMETRY_EXPORT Baggage // Uri decode key value pairs after extracting from header static std::string UrlDecode(nostd::string_view str, bool &err) { - auto IsHex = [](char c) { - return std::isdigit(c) || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f'); - }; - - auto from_hex = [](char c) -> char { - // c - '0' produces integer type which could trigger error/warning when casting to char, - // but the cast is safe here. - return static_cast(std::isdigit(c) ? c - '0' : std::toupper(c) - 'A' + 10); - }; - std::string ret; ret.reserve(str.size()); @@ -263,12 +250,14 @@ class OPENTELEMETRY_EXPORT Baggage { if (str[i] == '%') { - if (i + 2 >= str.size() || !IsHex(str[i + 1]) || !IsHex(str[i + 2])) + if (i + 2 >= str.size() || !common::detail::IsAsciiHexDigit(str[i + 1]) || + !common::detail::IsAsciiHexDigit(str[i + 2])) { err = true; return ""; } - ret.push_back(static_cast(from_hex(str[i + 1]) << 4 | from_hex(str[i + 2]))); + ret.push_back(static_cast(common::detail::HexDigitValue(str[i + 1]) << 4 | + common::detail::HexDigitValue(str[i + 2]))); i += 2; } else if (str[i] == '+') diff --git a/api/include/opentelemetry/common/detail/ascii.h b/api/include/opentelemetry/common/detail/ascii.h new file mode 100644 index 0000000000..3a74c42d58 --- /dev/null +++ b/api/include/opentelemetry/common/detail/ascii.h @@ -0,0 +1,127 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include +#include + +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace common +{ +namespace detail +{ + +// Character classes for protocol and specification text, which define them over +// ASCII. The functions depend on the global locale, and passing them a +// negative char is undefined behavior. + +constexpr bool IsAsciiDigit(char c) noexcept +{ + return c >= '0' && c <= '9'; +} + +constexpr bool IsAsciiLower(char c) noexcept +{ + return c >= 'a' && c <= 'z'; +} + +constexpr bool IsAsciiUpper(char c) noexcept +{ + return c >= 'A' && c <= 'Z'; +} + +constexpr bool IsAsciiAlpha(char c) noexcept +{ + return IsAsciiLower(c) || IsAsciiUpper(c); +} + +constexpr bool IsAsciiAlnum(char c) noexcept +{ + return IsAsciiAlpha(c) || IsAsciiDigit(c); +} + +// Indexed by byte value: hexadecimal digits are decoded on the propagation hot +// path, where a lookup is faster than range comparisons. +constexpr int8_t kHexDigitValues[256] = { + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, -1, -1, -1, -1, -1, -1, -1, 10, 11, 12, 13, 14, 15, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, 10, 11, 12, 13, 14, 15, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1}; + +// Returns the value of a hexadecimal digit of either case, or -1 if c is not one. +constexpr int HexDigitValue(char c) noexcept +{ + return kHexDigitValues[static_cast(c)]; +} + +constexpr bool IsAsciiHexDigit(char c) noexcept +{ + return HexDigitValue(c) >= 0; +} + +// Returns the hexadecimal digit for the low four bits of value. +constexpr char LowerHexDigit(int value) noexcept +{ + return "0123456789abcdef"[value & 0xF]; +} + +constexpr char UpperHexDigit(int value) noexcept +{ + return "0123456789ABCDEF"[value & 0xF]; +} + +// Writes two hexadecimal digits per byte, high nibble first, to out, which must +// have room for 2 * size characters. +inline void EncodeLowerHex(const uint8_t *bytes, std::size_t size, char *out) noexcept +{ + for (std::size_t i = 0; i < size; ++i) + { + out[2 * i] = LowerHexDigit(bytes[i] >> 4); + out[2 * i + 1] = LowerHexDigit(bytes[i]); + } +} + +inline void EncodeUpperHex(const uint8_t *bytes, std::size_t size, char *out) noexcept +{ + for (std::size_t i = 0; i < size; ++i) + { + out[2 * i] = UpperHexDigit(bytes[i] >> 4); + out[2 * i + 1] = UpperHexDigit(bytes[i]); + } +} + +// 0x00 to 0x1F and 0x7F: what std::iscntrl accepts in the "C" locale. +constexpr bool IsAsciiControl(char c) noexcept +{ + return static_cast(c) < 0x20 || c == '\x7f'; +} + +// Space, \t, \n, \v, \f and \r: what std::isspace accepts in the "C" locale. +constexpr bool IsAsciiSpace(char c) noexcept +{ + return c == ' ' || (c >= '\t' && c <= '\r'); +} + +constexpr char ToAsciiLower(char c) noexcept +{ + return IsAsciiUpper(c) ? static_cast(c - 'A' + 'a') : c; +} + +constexpr char ToAsciiUpper(char c) noexcept +{ + return IsAsciiLower(c) ? static_cast(c - 'a' + 'A') : c; +} + +} // namespace detail +} // namespace common +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/common/string_util.h b/api/include/opentelemetry/common/string_util.h index 9ae8de0952..15891dbd4d 100644 --- a/api/include/opentelemetry/common/string_util.h +++ b/api/include/opentelemetry/common/string_util.h @@ -3,8 +3,7 @@ #pragma once -#include - +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/macros.h" #if OPENTELEMETRY_HAVE_EXCEPTIONS @@ -29,11 +28,11 @@ class StringUtil { return nostd::string_view(); } - while (left <= right && std::isspace(str[left])) + while (left <= right && detail::IsAsciiSpace(str[left])) { left++; } - while (left <= right && std::isspace(str[right])) + while (left <= right && detail::IsAsciiSpace(str[right])) { right--; } diff --git a/api/include/opentelemetry/context/propagation/environment_carrier.h b/api/include/opentelemetry/context/propagation/environment_carrier.h index b949ad9185..07c1579aa0 100644 --- a/api/include/opentelemetry/context/propagation/environment_carrier.h +++ b/api/include/opentelemetry/context/propagation/environment_carrier.h @@ -8,6 +8,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/context/propagation/text_map_propagator.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" @@ -135,17 +136,13 @@ class EnvironmentCarrier : public TextMapCarrier std::string result(key); for (auto &c : result) { - unsigned char uc = static_cast(c); - if (uc >= 'a' && uc <= 'z') - { - c = static_cast(uc - ('a' - 'A')); - } - else if (!((uc >= 'A' && uc <= 'Z') || (uc >= '0' && uc <= '9') || uc == '_')) + c = common::detail::ToAsciiUpper(c); + if (!common::detail::IsAsciiUpper(c) && !common::detail::IsAsciiDigit(c) && c != '_') { c = '_'; } } - if (result[0] >= '0' && result[0] <= '9') + if (common::detail::IsAsciiDigit(result[0])) { result.insert(result.begin(), '_'); } diff --git a/api/include/opentelemetry/trace/propagation/detail/hex.h b/api/include/opentelemetry/trace/propagation/detail/hex.h index 6aa7c46de9..f3d68c536b 100644 --- a/api/include/opentelemetry/trace/propagation/detail/hex.h +++ b/api/include/opentelemetry/trace/propagation/detail/hex.h @@ -7,6 +7,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" @@ -20,28 +21,9 @@ namespace propagation namespace detail { -constexpr int8_t kHexDigits[256] = { - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, -1, -1, -1, -1, -1, -1, -1, 10, 11, 12, 13, 14, 15, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, 10, 11, 12, 13, 14, 15, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -}; - -inline int8_t HexToInt(char c) -{ - return kHexDigits[static_cast(c)]; -} - inline bool IsValidHex(nostd::string_view s) { - return std::all_of(s.begin(), s.end(), [](char c) { return HexToInt(c) != -1; }); + return std::all_of(s.begin(), s.end(), [](char c) { return common::detail::IsAsciiHexDigit(c); }); } /** @@ -66,12 +48,13 @@ inline bool HexToBinary(nostd::string_view hex, uint8_t *buffer, size_t buffer_s if (is_hex_size_odd) { - buffer[buffer_pos++] = HexToInt(hex[i++]); + buffer[buffer_pos++] = static_cast(common::detail::HexDigitValue(hex[i++])); } for (; i < last_hex_pos; i += 2) { - buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]); + buffer[buffer_pos++] = static_cast((common::detail::HexDigitValue(hex[i]) << 4) | + common::detail::HexDigitValue(hex[i + 1])); } return true; diff --git a/api/include/opentelemetry/trace/span_id.h b/api/include/opentelemetry/trace/span_id.h index 22ae82de35..503962ed4a 100644 --- a/api/include/opentelemetry/trace/span_id.h +++ b/api/include/opentelemetry/trace/span_id.h @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/version.h" @@ -31,12 +32,7 @@ class SpanId final // Populates the buffer with the lowercase base16 representation of the ID. void ToLowerBase16(nostd::span buffer) const noexcept { - constexpr char kHex[] = "0123456789abcdef"; - for (size_t i = 0; i < kSize; ++i) - { - buffer[i * 2 + 0] = kHex[(rep_[i] >> 4) & 0xF]; - buffer[i * 2 + 1] = kHex[(rep_[i] >> 0) & 0xF]; - } + common::detail::EncodeLowerHex(rep_, kSize, buffer.data()); } // Returns a nostd::span of the ID. diff --git a/api/include/opentelemetry/trace/trace_flags.h b/api/include/opentelemetry/trace/trace_flags.h index f4c7bc6a44..f0fc63a136 100644 --- a/api/include/opentelemetry/trace/trace_flags.h +++ b/api/include/opentelemetry/trace/trace_flags.h @@ -5,6 +5,7 @@ #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/version.h" @@ -44,9 +45,7 @@ class TraceFlags final // Populates the buffer with the lowercase base16 representation of the flags. void ToLowerBase16(nostd::span buffer) const noexcept { - constexpr char kHex[] = "0123456789ABCDEF"; - buffer[0] = kHex[(rep_ >> 4) & 0xF]; - buffer[1] = kHex[(rep_ >> 0) & 0xF]; + common::detail::EncodeUpperHex(&rep_, 1, buffer.data()); } uint8_t flags() const noexcept { return rep_; } diff --git a/api/include/opentelemetry/trace/trace_id.h b/api/include/opentelemetry/trace/trace_id.h index d134889eea..5b04ff7fb5 100644 --- a/api/include/opentelemetry/trace/trace_id.h +++ b/api/include/opentelemetry/trace/trace_id.h @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/version.h" @@ -34,12 +35,7 @@ class TraceId final // Populates the buffer with the lowercase base16 representation of the ID. void ToLowerBase16(nostd::span buffer) const noexcept { - constexpr char kHex[] = "0123456789abcdef"; - for (size_t i = 0; i < kSize; ++i) - { - buffer[i * 2 + 0] = kHex[(rep_[i] >> 4) & 0xF]; - buffer[i * 2 + 1] = kHex[(rep_[i] >> 0) & 0xF]; - } + common::detail::EncodeLowerHex(rep_, kSize, buffer.data()); } // Returns a nostd::span of the ID. diff --git a/api/include/opentelemetry/trace/trace_state.h b/api/include/opentelemetry/trace/trace_state.h index a21aad41e1..6a4ab8de33 100644 --- a/api/include/opentelemetry/trace/trace_state.h +++ b/api/include/opentelemetry/trace/trace_state.h @@ -3,11 +3,11 @@ #pragma once -#include #include #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/kv_properties.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/shared_ptr.h" @@ -298,7 +298,7 @@ class OPENTELEMETRY_EXPORT TraceState static bool IsLowerCaseAlphaOrDigit(char c) noexcept { - return std::isdigit(c) || std::islower(c); + return common::detail::IsAsciiDigit(c) || common::detail::IsAsciiLower(c); } private: diff --git a/api/test/common/BUILD b/api/test/common/BUILD index 3af58b5300..19ca8cfc29 100644 --- a/api/test/common/BUILD +++ b/api/test/common/BUILD @@ -15,6 +15,21 @@ otel_cc_benchmark( deps = ["//api"], ) +cc_test( + name = "ascii_test", + srcs = [ + "ascii_test.cc", + ], + tags = [ + "api", + "test", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "kv_properties_test", srcs = [ diff --git a/api/test/common/CMakeLists.txt b/api/test/common/CMakeLists.txt index 58cf328218..44935f6c76 100644 --- a/api/test/common/CMakeLists.txt +++ b/api/test/common/CMakeLists.txt @@ -3,7 +3,7 @@ include(GoogleTest) -foreach(testname kv_properties_test string_util_test) +foreach(testname ascii_test kv_properties_test string_util_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) diff --git a/api/test/common/ascii_test.cc b/api/test/common/ascii_test.cc new file mode 100644 index 0000000000..7d467ac31a --- /dev/null +++ b/api/test/common/ascii_test.cc @@ -0,0 +1,169 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include + +#include "opentelemetry/common/detail/ascii.h" + +namespace ascii = opentelemetry::common::detail; + +namespace +{ + +// The helpers must agree with in the "C" locale, which every program +// starts in, for every byte value. +template +std::string Mismatches(Helper helper, Reference reference) +{ + std::string mismatches; + for (int byte = 0; byte < 256; ++byte) + { + const char c = static_cast(byte); + if (helper(c) != reference(byte)) + { + mismatches += std::to_string(byte) + " "; + } + } + return mismatches; +} + +int ReferenceHexDigitValue(int byte) +{ + if (std::isxdigit(byte) == 0) + { + return -1; + } + return std::isdigit(byte) != 0 ? byte - '0' : std::tolower(byte) - 'a' + 10; +} + +// The digit for the low four bits of every byte value, as printf formats it. +template +std::string EncodingMismatches(Encoder encoder, const char *format) +{ + std::string mismatches; + for (int value = 0; value < 256; ++value) + { + char expected[2]; + std::snprintf(expected, sizeof(expected), format, value & 0xF); + if (encoder(value) != expected[0]) + { + mismatches += std::to_string(value) + " "; + } + } + return mismatches; +} + +template +std::string EncodeEveryByte(Encoder encoder) +{ + uint8_t bytes[256]; + for (int byte = 0; byte < 256; ++byte) + { + bytes[byte] = static_cast(byte); + } + std::string encoded(2 * sizeof(bytes), '\0'); + encoder(bytes, sizeof(bytes), &encoded[0]); + return encoded; +} + +// Every byte value in order, each formatted by printf with format. +std::string ReferenceEncoding(const char *format) +{ + std::string encoded; + for (int byte = 0; byte < 256; ++byte) + { + char digits[3]; + std::snprintf(digits, sizeof(digits), format, byte); + encoded += digits; + } + return encoded; +} + +} // namespace + +static_assert(ascii::IsAsciiAlnum('z'), "helpers are usable in constant expressions"); +static_assert(ascii::HexDigitValue('F') == 15, "helpers are usable in constant expressions"); +static_assert(ascii::LowerHexDigit(0xab) == 'b', "helpers are usable in constant expressions"); + +TEST(AsciiTest, IsAsciiDigit) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiDigit, [](int b) { return std::isdigit(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiLower) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiLower, [](int b) { return std::islower(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiUpper) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiUpper, [](int b) { return std::isupper(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiAlpha) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiAlpha, [](int b) { return std::isalpha(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiAlnum) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiAlnum, [](int b) { return std::isalnum(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiHexDigit) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiHexDigit, [](int b) { return std::isxdigit(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiControl) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiControl, [](int b) { return std::iscntrl(b) != 0; }), ""); +} + +TEST(AsciiTest, IsAsciiSpace) +{ + EXPECT_EQ(Mismatches(ascii::IsAsciiSpace, [](int b) { return std::isspace(b) != 0; }), ""); +} + +TEST(AsciiTest, ToAsciiLower) +{ + EXPECT_EQ( + Mismatches(ascii::ToAsciiLower, [](int b) { return static_cast(std::tolower(b)); }), + ""); +} + +TEST(AsciiTest, ToAsciiUpper) +{ + EXPECT_EQ( + Mismatches(ascii::ToAsciiUpper, [](int b) { return static_cast(std::toupper(b)); }), + ""); +} + +TEST(AsciiTest, HexDigitValue) +{ + EXPECT_EQ(Mismatches(ascii::HexDigitValue, ReferenceHexDigitValue), ""); +} + +TEST(AsciiTest, LowerHexDigit) +{ + EXPECT_EQ(EncodingMismatches(ascii::LowerHexDigit, "%x"), ""); +} + +TEST(AsciiTest, UpperHexDigit) +{ + EXPECT_EQ(EncodingMismatches(ascii::UpperHexDigit, "%X"), ""); +} + +TEST(AsciiTest, EncodeLowerHex) +{ + EXPECT_EQ(EncodeEveryByte(ascii::EncodeLowerHex), ReferenceEncoding("%02x")); +} + +TEST(AsciiTest, EncodeUpperHex) +{ + EXPECT_EQ(EncodeEveryByte(ascii::EncodeUpperHex), ReferenceEncoding("%02X")); +} diff --git a/exporters/etw/include/opentelemetry/exporters/etw/utils.h b/exporters/etw/include/opentelemetry/exporters/etw/utils.h index 5f9343bb03..e922682216 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/utils.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/utils.h @@ -13,6 +13,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/macros.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/exporters/etw/uuid.h" @@ -156,7 +157,7 @@ static inline GUID GetProviderGuid(const char *providerName) { std::string name(providerName); std::transform(name.begin(), name.end(), name.begin(), - [](unsigned char c) { return (char)::toupper(c); }); + [](char c) { return opentelemetry::common::detail::ToAsciiUpper(c); }); size_t len = name.length() * 2 + 0x10; uint8_t *buffer = new uint8_t[len]; diff --git a/exporters/etw/include/opentelemetry/exporters/etw/uuid.h b/exporters/etw/include/opentelemetry/exporters/etw/uuid.h index cb6cb5f4b1..fedc8948bd 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/uuid.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/uuid.h @@ -3,6 +3,7 @@ #pragma once +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/version.h" #include @@ -280,80 +281,77 @@ struct UUID /// This UUID object in a string. std::string to_string() const { - static char inttoHex[16] = {'0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; - const unsigned buffSize = 36 + 1; // 36 + null-terminator char buf[buffSize] = {0}; int test = (Data1 >> 28 & 0x0000000F); - buf[0] = inttoHex[test]; + buf[0] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 24 & 0x0000000F); - buf[1] = inttoHex[test]; + buf[1] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 20 & 0x0000000F); - buf[2] = inttoHex[test]; + buf[2] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 16 & 0x0000000F); - buf[3] = inttoHex[test]; + buf[3] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 12 & 0x0000000F); - buf[4] = inttoHex[test]; + buf[4] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 8 & 0x0000000F); - buf[5] = inttoHex[test]; + buf[5] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 >> 4 & 0x0000000F); - buf[6] = inttoHex[test]; + buf[6] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data1 & 0x0000000F); - buf[7] = inttoHex[test]; + buf[7] = opentelemetry::common::detail::UpperHexDigit(test); buf[8] = '-'; test = (int)(Data2 >> 12 & 0x000F); - buf[9] = inttoHex[test]; + buf[9] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data2 >> 8 & 0x000F); - buf[10] = inttoHex[test]; + buf[10] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data2 >> 4 & 0x000F); - buf[11] = inttoHex[test]; + buf[11] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data2 & 0x000F); - buf[12] = inttoHex[test]; + buf[12] = opentelemetry::common::detail::UpperHexDigit(test); buf[13] = '-'; test = (int)(Data3 >> 12 & 0x000F); - buf[14] = inttoHex[test]; + buf[14] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data3 >> 8 & 0x000F); - buf[15] = inttoHex[test]; + buf[15] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data3 >> 4 & 0x000F); - buf[16] = inttoHex[test]; + buf[16] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data3 & 0x000F); - buf[17] = inttoHex[test]; + buf[17] = opentelemetry::common::detail::UpperHexDigit(test); buf[18] = '-'; test = (int)(Data4[0] >> 4 & 0x0F); - buf[19] = inttoHex[test]; + buf[19] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[0] & 0x0F); - buf[20] = inttoHex[test]; + buf[20] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[1] >> 4 & 0x0F); - buf[21] = inttoHex[test]; + buf[21] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[1] & 0x0F); - buf[22] = inttoHex[test]; + buf[22] = opentelemetry::common::detail::UpperHexDigit(test); buf[23] = '-'; test = (int)(Data4[2] >> 4 & 0x0F); - buf[24] = inttoHex[test]; + buf[24] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[2] & 0x0F); - buf[25] = inttoHex[test]; + buf[25] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[3] >> 4 & 0x0F); - buf[26] = inttoHex[test]; + buf[26] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[3] & 0x0F); - buf[27] = inttoHex[test]; + buf[27] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[4] >> 4 & 0x0F); - buf[28] = inttoHex[test]; + buf[28] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[4] & 0x0F); - buf[29] = inttoHex[test]; + buf[29] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[5] >> 4 & 0x0F); - buf[30] = inttoHex[test]; + buf[30] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[5] & 0x0F); - buf[31] = inttoHex[test]; + buf[31] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[6] >> 4 & 0x0F); - buf[32] = inttoHex[test]; + buf[32] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[6] & 0x0F); - buf[33] = inttoHex[test]; + buf[33] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[7] >> 4 & 0x0F); - buf[34] = inttoHex[test]; + buf[34] = opentelemetry::common::detail::UpperHexDigit(test); test = (int)(Data4[7] & 0x0F); - buf[35] = inttoHex[test]; + buf[35] = opentelemetry::common::detail::UpperHexDigit(test); buf[36] = 0; return std::string(buf); diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h index c275f4eaa4..749443c3e4 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -4,11 +4,11 @@ #pragma once #include -#include #include #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/sdk/version/version.h" #include "opentelemetry/version.h" @@ -131,9 +131,11 @@ struct cmp_ic { bool operator()(const std::string &s1, const std::string &s2) const { - return std::lexicographical_compare( - s1.begin(), s1.end(), s2.begin(), s2.end(), - [](char c1, char c2) { return ::tolower(c1) < ::tolower(c2); }); + return std::lexicographical_compare(s1.begin(), s1.end(), s2.begin(), s2.end(), + [](char c1, char c2) { + return opentelemetry::common::detail::ToAsciiLower(c1) < + opentelemetry::common::detail::ToAsciiLower(c2); + }); } }; using OtlpHeaders = std::multimap; diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index 2bc24b301c..e47aab8e49 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -21,12 +21,6 @@ // IWYU pragma: no_include -#if defined(HAVE_GSL) -# include -#else -# include -#endif - #ifdef _MSC_VER # define strcasecmp _stricmp #else @@ -103,6 +97,7 @@ # define OTLP_FILE_OPEN(f, path, mode) f = std::fopen(path, mode) #endif +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/exporters/otlp/otlp_file_client.h" #include "opentelemetry/exporters/otlp/otlp_file_client_options.h" #include "opentelemetry/exporters/otlp/otlp_file_client_runtime_options.h" @@ -714,33 +709,11 @@ class OPENTELEMETRY_LOCAL_SYMBOL FileSystemUtil #endif }; -static inline char HexEncode(unsigned char byte) -{ -#if defined(HAVE_GSL) - Expects(byte <= 16); -#else - assert(byte <= 16); -#endif - if (byte >= 10) - { - return static_cast(byte - 10 + 'a'); - } - else - { - return static_cast(byte + '0'); - } -} - static std::string HexEncode(const std::string &bytes) { - std::string ret; - ret.reserve(bytes.size() * 2); - for (std::string::size_type i = 0; i < bytes.size(); ++i) - { - unsigned char byte = static_cast(bytes[i]); - ret.push_back(HexEncode(byte >> 4)); - ret.push_back(HexEncode(byte & 0x0f)); - } + std::string ret(bytes.size() * 2, '\0'); + opentelemetry::common::detail::EncodeLowerHex(reinterpret_cast(bytes.data()), + bytes.size(), &ret[0]); return ret; } diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index ee902e930a..89c26d3fb0 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -17,13 +18,8 @@ #include #include -#if defined(HAVE_GSL) -# include -#else -# include -#endif - #include "nlohmann/json.hpp" +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/timestamp.h" #include "opentelemetry/exporters/otlp/otlp_http.h" #include "opentelemetry/exporters/otlp/otlp_http_client.h" @@ -422,33 +418,11 @@ class ResponseHandler : public http_client::EventHandler bool console_debug_ = false; }; -static inline char HexEncode(unsigned char byte) -{ -#if defined(HAVE_GSL) - Expects(byte <= 16); -#else - assert(byte <= 16); -#endif - if (byte >= 10) - { - return static_cast(byte - 10 + 'a'); - } - else - { - return static_cast(byte + '0'); - } -} - static std::string HexEncode(const std::string &bytes) { - std::string ret; - ret.reserve(bytes.size() * 2); - for (std::string::size_type i = 0; i < bytes.size(); ++i) - { - unsigned char byte = static_cast(bytes[i]); - ret.push_back(HexEncode(byte >> 4)); - ret.push_back(HexEncode(byte & 0x0f)); - } + std::string ret(bytes.size() * 2, '\0'); + opentelemetry::common::detail::EncodeLowerHex(reinterpret_cast(bytes.data()), + bytes.size(), &ret[0]); return ret; } diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index bcca6bb73c..b6f817a91b 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -17,6 +17,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/exporters/prometheus/exporter_utils.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/common/attribute_utils.h" @@ -95,10 +96,8 @@ inline std::string Sanitize(std::string name, const T &valid) std::string SanitizeLabel(std::string label_key) { return Sanitize(std::move(label_key), [](int i, char c) { - return (c >= 'a' && c <= 'z') || // - (c >= 'A' && c <= 'Z') || // - c == '_' || // - (c >= '0' && c <= '9' && i > 0); + return opentelemetry::common::detail::IsAsciiAlpha(c) || c == '_' || + (opentelemetry::common::detail::IsAsciiDigit(c) && i > 0); }); } @@ -255,8 +254,8 @@ std::string PrometheusExporterUtils::SanitizeNames(std::string name) constexpr const auto replacement_dup = '='; auto valid = [](int i, char c) { - if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == ':' || - (c >= '0' && c <= '9' && i > 0)) + if (opentelemetry::common::detail::IsAsciiAlpha(c) || c == ':' || + (opentelemetry::common::detail::IsAsciiDigit(c) && i > 0)) { return true; } @@ -435,7 +434,7 @@ std::string PrometheusExporterUtils::CleanUpString(const std::string &str) } std::transform(cleaned_string.begin(), cleaned_string.end(), cleaned_string.begin(), [](const char c) { - if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) + if (opentelemetry::common::detail::IsAsciiAlnum(c)) { return c; } diff --git a/ext/include/opentelemetry/ext/http/client/http_client.h b/ext/include/opentelemetry/ext/http/client/http_client.h index b477292fed..09be3653d1 100644 --- a/ext/include/opentelemetry/ext/http/client/http_client.h +++ b/ext/include/opentelemetry/ext/http/client/http_client.h @@ -10,6 +10,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" @@ -111,9 +112,11 @@ struct cmp_ic { bool operator()(const std::string &s1, const std::string &s2) const { - return std::lexicographical_compare( - s1.begin(), s1.end(), s2.begin(), s2.end(), - [](char c1, char c2) { return ::tolower(c1) < ::tolower(c2); }); + return std::lexicographical_compare(s1.begin(), s1.end(), s2.begin(), s2.end(), + [](char c1, char c2) { + return opentelemetry::common::detail::ToAsciiLower(c1) < + opentelemetry::common::detail::ToAsciiLower(c2); + }); } }; using Headers = std::multimap; diff --git a/ext/include/opentelemetry/ext/http/common/url_parser.h b/ext/include/opentelemetry/ext/http/common/url_parser.h index 726376afe0..70d8c82c33 100644 --- a/ext/include/opentelemetry/ext/http/common/url_parser.h +++ b/ext/include/opentelemetry/ext/http/common/url_parser.h @@ -9,6 +9,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -193,16 +194,6 @@ class UrlDecoder std::string result; result.reserve(encoded.size()); - auto hex_to_int = [](int ch) -> int { - if (ch >= '0' && ch <= '9') - return ch - '0'; - if (ch >= 'a' && ch <= 'f') - return ch - 'a' + 10; - if (ch >= 'A' && ch <= 'F') - return ch - 'A' + 10; - return -1; - }; - for (size_t pos = 0; pos < encoded.size(); pos++) { auto c = encoded[pos]; @@ -213,8 +204,8 @@ class UrlDecoder return encoded; } - int hi = hex_to_int(encoded[pos + 1]); - int lo = hex_to_int(encoded[pos + 2]); + int hi = opentelemetry::common::detail::HexDigitValue(encoded[pos + 1]); + int lo = opentelemetry::common::detail::HexDigitValue(encoded[pos + 2]); if (hi == -1 || lo == -1) { diff --git a/ext/include/opentelemetry/ext/http/server/http_server.h b/ext/include/opentelemetry/ext/http/server/http_server.h index f7425b7a25..b3c57a4edb 100644 --- a/ext/include/opentelemetry/ext/http/server/http_server.h +++ b/ext/include/opentelemetry/ext/http/server/http_server.h @@ -13,6 +13,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "socket_tools.h" #ifdef HAVE_HTTP_DEBUG @@ -724,7 +725,7 @@ class HttpServer : private SocketTools::Reactor::SocketCallback static bool equalsLowercased(std::string const &str, char const *mask) { char const *ptr = str.c_str(); - while (*ptr && *mask && ::tolower(*ptr) == *mask) + while (*ptr && *mask && opentelemetry::common::detail::ToAsciiLower(*ptr) == *mask) { ptr++; mask++; @@ -740,7 +741,7 @@ class HttpServer : private SocketTools::Reactor::SocketCallback { if (first) { - ch = static_cast(::toupper(ch)); + ch = opentelemetry::common::detail::ToAsciiUpper(ch); first = false; } else if (ch == '-') @@ -749,7 +750,7 @@ class HttpServer : private SocketTools::Reactor::SocketCallback } else { - ch = static_cast(::tolower(ch)); + ch = opentelemetry::common::detail::ToAsciiLower(ch); } } return result; diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index 99c8d8dafa..e10500e2d7 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -28,7 +28,7 @@ #endif #ifdef OTEL_CURL_DEBUG -# include +# include "opentelemetry/common/detail/ascii.h" #endif #include "opentelemetry/common/string_util.h" @@ -843,7 +843,7 @@ int HttpOperation::CurlLoggerCallback(const CURL * /* handle */, { static const auto kHeaderSent = nostd::string_view("Send header => "); - while (!text_to_log.empty() && !std::iscntrl(text_to_log[0])) + while (!text_to_log.empty() && !opentelemetry::common::detail::IsAsciiControl(text_to_log[0])) { const auto pos = text_to_log.find('\n'); diff --git a/ext/src/http/client/curl/http_time_util.cc b/ext/src/http/client/curl/http_time_util.cc index 2535a4fd6b..046425ca63 100644 --- a/ext/src/http/client/curl/http_time_util.cc +++ b/ext/src/http/client/curl/http_time_util.cc @@ -3,7 +3,6 @@ #include "opentelemetry/ext/http/client/curl/http_time_util.h" -#include #include #include #include @@ -12,6 +11,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/string_util.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -35,7 +35,7 @@ bool HttpTimeUtil::ParseDelaySeconds(nostd::string_view value, std::chrono::seco std::chrono::seconds::rep result = 0; for (const char c : value) { - if (!std::isdigit(static_cast(static_cast(c)))) + if (!opentelemetry::common::detail::IsAsciiDigit(c)) { return false; } diff --git a/ext/test/w3c_tracecontext_http_test_server/main.cc b/ext/test/w3c_tracecontext_http_test_server/main.cc index f1eb0e728c..37e2ca6fa5 100644 --- a/ext/test/w3c_tracecontext_http_test_server/main.cc +++ b/ext/test/w3c_tracecontext_http_test_server/main.cc @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include #include #include #include @@ -15,6 +14,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/context/context.h" #include "opentelemetry/context/propagation/text_map_propagator.h" #include "opentelemetry/context/runtime_context.h" @@ -55,7 +55,8 @@ static bool equalsIgnoreCase(const std::string &str1, const std::string &str2) } for (size_t i = 0; i < str1.length(); i++) { - if (std::tolower(str1[i]) != std::tolower(str2[i])) + if (opentelemetry::common::detail::ToAsciiLower(str1[i]) != + opentelemetry::common::detail::ToAsciiLower(str2[i])) { return false; } diff --git a/resource_detectors/src/env_entity_detector.cc b/resource_detectors/src/env_entity_detector.cc index 6334cf08b0..de5109a7cc 100644 --- a/resource_detectors/src/env_entity_detector.cc +++ b/resource_detectors/src/env_entity_detector.cc @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include #include #include #include @@ -10,6 +9,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/string_util.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/variant.h" @@ -80,22 +80,15 @@ std::string PercentDecode(opentelemetry::nostd::string_view value) noexcept std::string result; result.reserve(value.size()); - auto IsHex = [](char c) { - return std::isdigit(static_cast(c)) || (c >= 'A' && c <= 'F') || - (c >= 'a' && c <= 'f'); - }; - - auto FromHex = [](char c) -> char { - return static_cast(std::isdigit(static_cast(c)) - ? c - '0' - : std::toupper(static_cast(c)) - 'A' + 10); - }; - for (size_t i = 0; i < value.size(); ++i) { - if (value[i] == '%' && i + 2 < value.size() && IsHex(value[i + 1]) && IsHex(value[i + 2])) + if (value[i] == '%' && i + 2 < value.size() && + opentelemetry::common::detail::IsAsciiHexDigit(value[i + 1]) && + opentelemetry::common::detail::IsAsciiHexDigit(value[i + 2])) { - result.push_back(static_cast((FromHex(value[i + 1]) << 4) | FromHex(value[i + 2]))); + result.push_back( + static_cast((opentelemetry::common::detail::HexDigitValue(value[i + 1]) << 4) | + opentelemetry::common::detail::HexDigitValue(value[i + 2]))); i += 2; } else @@ -123,7 +116,7 @@ bool IsValidSchemaUrl(const std::string &url) noexcept return false; // Empty scheme or no content after :// } // Scheme must start with letter - if (!std::isalpha(static_cast(url[0]))) + if (!opentelemetry::common::detail::IsAsciiAlpha(url[0])) { return false; } @@ -131,7 +124,7 @@ bool IsValidSchemaUrl(const std::string &url) noexcept for (size_t i = 1; i < scheme_end; ++i) { char c = url[i]; - if (!(std::isalnum(static_cast(c)) || c == '+' || c == '-' || c == '.')) + if (!(opentelemetry::common::detail::IsAsciiAlnum(c) || c == '+' || c == '-' || c == '.')) { return false; } @@ -189,14 +182,14 @@ bool ParseSingleEntity(const std::string &entity_str, ParsedEntity &out) out.type = std::string{opentelemetry::common::StringUtil::Trim(entity_str.substr(0, brace_pos))}; // Validate type matches [a-zA-Z][a-zA-Z0-9._-]* - if (out.type.empty() || !std::isalpha(static_cast(out.type[0]))) + if (out.type.empty() || !opentelemetry::common::detail::IsAsciiAlpha(out.type[0])) { return false; } for (size_t i = 1; i < out.type.size(); ++i) { char c = out.type[i]; - if (!(std::isalnum(static_cast(c)) || c == '.' || c == '_' || c == '-')) + if (!(opentelemetry::common::detail::IsAsciiAlnum(c) || c == '.' || c == '_' || c == '-')) { return false; } diff --git a/resource_detectors/src/service_detector_utils.cc b/resource_detectors/src/service_detector_utils.cc index 476052f455..2c491d944f 100644 --- a/resource_detectors/src/service_detector_utils.cc +++ b/resource_detectors/src/service_detector_utils.cc @@ -15,6 +15,7 @@ # include #endif +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/resource_detectors/detail/process_detector_utils.h" #include "opentelemetry/sdk/common/env_variables.h" #include "opentelemetry/version.h" @@ -31,21 +32,16 @@ namespace constexpr const char *kOtelServiceName = "OTEL_SERVICE_NAME"; constexpr const char *kUnknownService = "unknown_service"; constexpr const char *kUnknownServicePrefix = "unknown_service:"; -constexpr const char kUuidHexDigits[] = "0123456789abcdef"; std::string FormatUuidV4(const uint8_t bytes[16]) noexcept { - std::string uuid; - uuid.reserve(36); - for (size_t i = 0; i < 16; ++i) - { - if (i == 4 || i == 6 || i == 8 || i == 10) - { - uuid.push_back('-'); - } - uuid.push_back(kUuidHexDigits[bytes[i] >> 4]); - uuid.push_back(kUuidHexDigits[bytes[i] & 0x0F]); - } + // Groups of 8, 4, 4, 4 and 12 hexadecimal digits, separated by dashes. + std::string uuid(36, '-'); + opentelemetry::common::detail::EncodeLowerHex(&bytes[0], 4, &uuid[0]); + opentelemetry::common::detail::EncodeLowerHex(&bytes[4], 2, &uuid[9]); + opentelemetry::common::detail::EncodeLowerHex(&bytes[6], 2, &uuid[14]); + opentelemetry::common::detail::EncodeLowerHex(&bytes[8], 2, &uuid[19]); + opentelemetry::common::detail::EncodeLowerHex(&bytes[10], 6, &uuid[24]); return uuid; } diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index f4573e6002..a41ffa5d77 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -4,11 +4,11 @@ #pragma once #include -#include #include #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h" #include "opentelemetry/version.h" @@ -75,8 +75,8 @@ struct InstrumentDescriptorUtil { return lhs.size() == rhs.size() && std::equal(lhs.begin(), lhs.end(), rhs.begin(), [](char a, char b) { - return std::tolower(static_cast(a)) == - std::tolower(static_cast(b)); + return opentelemetry::common::detail::ToAsciiLower(a) == + opentelemetry::common::detail::ToAsciiLower(b); }); } @@ -227,8 +227,7 @@ struct InstrumentDescriptorHash for (char c : instrument_descriptor.name_) { - sdk::common::GetHash(hashcode, - static_cast(std::tolower(static_cast(c)))); + sdk::common::GetHash(hashcode, opentelemetry::common::detail::ToAsciiLower(c)); } sdk::common::GetHash(hashcode, instrument_descriptor.description_); diff --git a/sdk/src/common/env_variables.cc b/sdk/src/common/env_variables.cc index 060eac8e74..0aa94e561c 100644 --- a/sdk/src/common/env_variables.cc +++ b/sdk/src/common/env_variables.cc @@ -10,12 +10,12 @@ # include #endif -#include #include #include #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/version.h" @@ -92,10 +92,10 @@ static bool GetTimeoutFromString(const char *input, std::chrono::system_clock::d std::chrono::system_clock::duration::rep result = 0; // Skip spaces - for (; *input && std::isspace(*input); ++input) + for (; *input && opentelemetry::common::detail::IsAsciiSpace(*input); ++input) ; - for (; *input && std::isdigit(*input); ++input) + for (; *input && opentelemetry::common::detail::IsAsciiDigit(*input); ++input) { auto digit = (*input - '0'); diff --git a/sdk/src/configuration/document_node.cc b/sdk/src/configuration/document_node.cc index b60fac92db..9739e1196a 100644 --- a/sdk/src/configuration/document_node.cc +++ b/sdk/src/configuration/document_node.cc @@ -1,12 +1,12 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include #include #include #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/sdk/common/env_variables.h" #include "opentelemetry/sdk/configuration/document_node.h" #include "opentelemetry/sdk/configuration/invalid_schema_exception.h" @@ -157,7 +157,7 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text) const } c = text[begin_name]; - if (!std::isalpha(c) && c != '_') + if (!opentelemetry::common::detail::IsAsciiAlpha(c) && c != '_') { std::string message = illegal_msg; message.append(text); @@ -169,7 +169,7 @@ std::string DocumentNode::DoOneSubstitution(const std::string &text) const for (size_t i = begin_name + 1; i + 2 <= len; i++) { c = text[i]; - if (std::isalnum(c) || (c == '_')) + if (opentelemetry::common::detail::IsAsciiAlnum(c) || (c == '_')) { end_name = i + 1; } diff --git a/sdk/src/resource/detail/percent_decode.cc b/sdk/src/resource/detail/percent_decode.cc index 190fcb2488..208126e9c4 100644 --- a/sdk/src/resource/detail/percent_decode.cc +++ b/sdk/src/resource/detail/percent_decode.cc @@ -3,9 +3,10 @@ #include "percent_decode.h" -#include #include +#include "opentelemetry/common/detail/ascii.h" + OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -13,16 +14,6 @@ namespace resource { namespace detail { -namespace -{ - -unsigned char HexDigitValue(unsigned char c) noexcept -{ - return static_cast(std::isdigit(c) ? c - '0' : std::toupper(c) - 'A' + 10); -} - -} // namespace - std::string PercentDecode(const std::string &value) { std::string decoded; @@ -32,15 +23,12 @@ std::string PercentDecode(const std::string &value) { if (value[i] == '%' && i + 2 < value.size()) { - unsigned char high = static_cast(value[i + 1]); - unsigned char low = static_cast(value[i + 2]); + const int high = opentelemetry::common::detail::HexDigitValue(value[i + 1]); + const int low = opentelemetry::common::detail::HexDigitValue(value[i + 2]); - if (std::isxdigit(high) && std::isxdigit(low)) + if (high >= 0 && low >= 0) { - unsigned char unescaped_value = - static_cast((HexDigitValue(high) << 4) | HexDigitValue(low)); - - decoded.push_back(static_cast(unescaped_value)); + decoded.push_back(static_cast((high << 4) | low)); i += 2; continue; } diff --git a/sdk/src/trace/samplers/ot_trace_state.cc b/sdk/src/trace/samplers/ot_trace_state.cc index aa495acb6c..0a53346063 100644 --- a/sdk/src/trace/samplers/ot_trace_state.cc +++ b/sdk/src/trace/samplers/ot_trace_state.cc @@ -10,6 +10,7 @@ #include #include +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/trace/trace_id.h" @@ -19,8 +20,6 @@ namespace { -constexpr char kHexDigits[] = "0123456789abcdef"; - // TraceState::Set drops the whole tracestate when a value is longer than this, // so the entire "ot" value has to fit within it. constexpr std::size_t kMaxOtValueSize = @@ -49,7 +48,8 @@ void AppendThresholdHex(std::string &out, uint64_t threshold) int trailing_zeros = CountTrailingZeroBits(marked); for (int shift = 52; shift >= trailing_zeros - 3; shift -= 4) { - out.push_back(kHexDigits[(threshold >> shift) & 0xFU]); + out.push_back(opentelemetry::common::detail::LowerHexDigit( + static_cast((threshold >> shift) & 0xFU))); } } @@ -58,7 +58,8 @@ void AppendRandomHex(std::string &out, uint64_t value) { for (int shift = 52; shift >= 0; shift -= 4) { - out.push_back(kHexDigits[(value >> shift) & 0xFU]); + out.push_back( + opentelemetry::common::detail::LowerHexDigit(static_cast((value >> shift) & 0xFU))); } } @@ -69,17 +70,9 @@ bool ParseHex(const std::string &s, std::size_t start, std::size_t len, uint64_t uint64_t r = 0; for (std::size_t i = 0; i < len; ++i) { - char c = s[start + i]; - int nibble = 0; - if (c >= '0' && c <= '9') - { - nibble = c - '0'; - } - else if (c >= 'a' && c <= 'f') - { - nibble = c - 'a' + 10; - } - else + const char c = s[start + i]; + const int nibble = opentelemetry::common::detail::HexDigitValue(c); + if (nibble < 0 || opentelemetry::common::detail::IsAsciiUpper(c)) { return false; } From b749b65b4ea432b199f6b06c19628a6a456c4d32 Mon Sep 17 00:00:00 2001 From: Lukasz Towarek Date: Sun, 18 Oct 2026 14:09:22 +0200 Subject: [PATCH 2/5] [SDK] Remove regex from InstrumentMetaDataValidator Before GCC 16, libstdc++'s std::regex matcher recurses once per input character (GCC bug 86164), so validating a 255-character instrument name needs a deep stack and overflows small thread stacks, such as the few KB an RTOS task gets. Names and units are now checked with a loop that accepts the same patterns. Unlike the regex, which read the view as a C string, it checks exactly the view: an embedded NUL is rejected, and a view that is not NUL-terminated is no longer read past its end, which accepted an empty view over a valid name and rejected a valid view followed by invalid characters. Unlike the former non-regex fallback, it does not read the first character of an empty name, and it uses the ASCII helpers instead of the locale-dependent std::isalpha and std::isalnum. meter.h no longer includes through the validator's header. TestUnit checked its valid units with ValidateName; it now calls ValidateUnit. New cases cover an embedded NUL, non-ASCII characters (including as the first character of a name), an empty unit and views that are not NUL-terminated; each fails with the regex. A benchmark covers construction and validation. --- CHANGELOG.md | 3 + .../metrics/instrument_metadata_validator.h | 10 --- .../metrics/instrument_metadata_validator.cc | 87 +++++++++---------- sdk/test/metrics/BUILD | 15 ++++ sdk/test/metrics/CMakeLists.txt | 5 ++ ...instrument_metadata_validator_benchmark.cc | 48 ++++++++++ .../instrument_metadata_validator_test.cc | 49 ++++++++++- 7 files changed, 158 insertions(+), 59 deletions(-) create mode 100644 sdk/test/metrics/instrument_metadata_validator_benchmark.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index a8df3dbdaa..d2bd46ce43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ Increment the: ## [Unreleased] +* [API/SDK] Add ASCII helpers and remove regex from InstrumentMetaDataValidator + [#XXXX](https://github.com/open-telemetry/opentelemetry-cpp/pull/XXXX) + * [SDK] Release provider mutexes before construction-failure diagnostics so custom log handlers can retrieve cached or noop tracers, loggers, and meters from the same provider without deadlocking. diff --git a/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h b/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h index 602bab2aad..5ba719f2b2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h +++ b/sdk/include/opentelemetry/sdk/metrics/instrument_metadata_validator.h @@ -6,10 +6,6 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" -#if OPENTELEMETRY_HAVE_WORKING_REGEX -# include -#endif - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -22,12 +18,6 @@ class InstrumentMetaDataValidator bool ValidateName(nostd::string_view name) const; bool ValidateUnit(nostd::string_view unit) const; bool ValidateDescription(nostd::string_view description) const; - -private: -#if OPENTELEMETRY_HAVE_WORKING_REGEX - std::regex name_reg_key_; - std::regex unit_reg_key_; -#endif }; } // namespace metrics diff --git a/sdk/src/metrics/instrument_metadata_validator.cc b/sdk/src/metrics/instrument_metadata_validator.cc index 3e2524276d..2319411cbb 100644 --- a/sdk/src/metrics/instrument_metadata_validator.cc +++ b/sdk/src/metrics/instrument_metadata_validator.cc @@ -1,79 +1,70 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include "opentelemetry/sdk/metrics/instrument_metadata_validator.h" +#include + +#include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/metrics/instrument_metadata_validator.h" #include "opentelemetry/version.h" -#if OPENTELEMETRY_HAVE_WORKING_REGEX -# include -#else -# include -# include -#endif - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { namespace metrics { -#if OPENTELEMETRY_HAVE_WORKING_REGEX +namespace +{ // instrument-name = ALPHA 0*254 ("_" / "." / "-" / "/" / ALPHA / DIGIT) -constexpr const char *kInstrumentNamePattern = "[a-zA-Z][-_./a-zA-Z0-9]{0,254}"; -// -constexpr const char *kInstrumentUnitPattern = "[\x01-\x7F]{0,63}"; -// instrument-unit = It can have a maximum length of 63 ASCII chars -#endif +constexpr std::size_t kMaxNameSize = 255; +// instrument-unit = 0*63 ASCII characters, excluding NUL +constexpr std::size_t kMaxUnitSize = 63; -InstrumentMetaDataValidator::InstrumentMetaDataValidator() -#if OPENTELEMETRY_HAVE_WORKING_REGEX - // clang-format off - : name_reg_key_{kInstrumentNamePattern}, - unit_reg_key_{kInstrumentUnitPattern} -// clang-format on -#endif -{} +bool IsNameChar(char c) noexcept +{ + return opentelemetry::common::detail::IsAsciiAlnum(c) || c == '_' || c == '.' || c == '-' || + c == '/'; +} -bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const +bool IsUnitChar(unsigned char c) noexcept { + return c >= 0x01 && c <= 0x7F; +} +} // namespace -#if OPENTELEMETRY_HAVE_WORKING_REGEX - return std::regex_match(name.data(), name_reg_key_); -#else - const size_t kMaxSize = 255; - // size atmost 255 chars - if (name.size() > kMaxSize) +InstrumentMetaDataValidator::InstrumentMetaDataValidator() = default; + +bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const +{ + if (name.empty() || name.size() > kMaxNameSize || + !opentelemetry::common::detail::IsAsciiAlpha(name[0])) { return false; } - // first char should be alpha - if (!std::isalpha(name[0])) + for (std::size_t i = 1; i < name.size(); ++i) { - return false; + if (!IsNameChar(name[i])) + { + return false; + } } - // subsequent chars should be either of alphabets, digits, underscore, - // minus, dot, slash - return !std::any_of(std::next(name.begin()), name.end(), [](char c) { - return !std::isalnum(c) && (c != '-') && (c != '_') && (c != '.') && (c != '/'); - }); -#endif + return true; } bool InstrumentMetaDataValidator::ValidateUnit(nostd::string_view unit) const { -#if OPENTELEMETRY_HAVE_WORKING_REGEX - return std::regex_match(unit.data(), unit_reg_key_); -#else - const size_t kMaxSize = 63; - // length atmost 63 chars - if (unit.size() > kMaxSize) + if (unit.size() > kMaxUnitSize) { return false; } - // all should be ascii chars. - return !std::any_of(unit.begin(), unit.end(), - [](char c) { return static_cast(c) > 127; }); -#endif + for (const char c : unit) + { + if (!IsUnitChar(static_cast(c))) + { + return false; + } + } + return true; } bool InstrumentMetaDataValidator::ValidateDescription(nostd::string_view /*description*/) const diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD index 2e03e66cc4..f0b5801378 100644 --- a/sdk/test/metrics/BUILD +++ b/sdk/test/metrics/BUILD @@ -180,6 +180,21 @@ otel_cc_benchmark( ], ) +otel_cc_benchmark( + name = "instrument_metadata_validator_benchmark", + srcs = [ + "instrument_metadata_validator_benchmark.cc", + ], + tags = [ + "benchmark", + "metrics", + "test", + ], + deps = [ + "//sdk/src/metrics", + ], +) + otel_cc_benchmark( name = "sync_instruments_benchmark", srcs = [ diff --git a/sdk/test/metrics/CMakeLists.txt b/sdk/test/metrics/CMakeLists.txt index 93cec62bb2..98581dee63 100644 --- a/sdk/test/metrics/CMakeLists.txt +++ b/sdk/test/metrics/CMakeLists.txt @@ -119,6 +119,11 @@ if(OTELCPP_WITH_BENCHMARK) target_link_libraries( sync_instruments_benchmark benchmark::benchmark opentelemetry_metrics opentelemetry_resources ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common) + add_executable(instrument_metadata_validator_benchmark + instrument_metadata_validator_benchmark.cc) + target_link_libraries( + instrument_metadata_validator_benchmark benchmark::benchmark + opentelemetry_metrics ${CMAKE_THREAD_LIBS_INIT} opentelemetry_common) add_executable(sum_aggregation_benchmark sum_aggregation_benchmark.cc) target_link_libraries( sum_aggregation_benchmark benchmark::benchmark ${CMAKE_THREAD_LIBS_INIT} diff --git a/sdk/test/metrics/instrument_metadata_validator_benchmark.cc b/sdk/test/metrics/instrument_metadata_validator_benchmark.cc new file mode 100644 index 0000000000..ffe64d8fd9 --- /dev/null +++ b/sdk/test/metrics/instrument_metadata_validator_benchmark.cc @@ -0,0 +1,48 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include "opentelemetry/sdk/metrics/instrument_metadata_validator.h" + +using opentelemetry::sdk::metrics::InstrumentMetaDataValidator; + +namespace +{ + +void BM_ConstructValidator(benchmark::State &state) +{ + for (auto _ : state) + { + InstrumentMetaDataValidator validator; + benchmark::DoNotOptimize(validator); + } +} +BENCHMARK(BM_ConstructValidator); + +void BM_ValidateName(benchmark::State &state, const std::string &name) +{ + InstrumentMetaDataValidator validator; + for (auto _ : state) + { + benchmark::DoNotOptimize(validator.ValidateName(name)); + } +} +BENCHMARK_CAPTURE(BM_ValidateName, Typical, std::string("http.server.request.duration")); +BENCHMARK_CAPTURE(BM_ValidateName, MaxLength, std::string(255, 'a')); + +void BM_ValidateUnit(benchmark::State &state, const std::string &unit) +{ + InstrumentMetaDataValidator validator; + for (auto _ : state) + { + benchmark::DoNotOptimize(validator.ValidateUnit(unit)); + } +} +BENCHMARK_CAPTURE(BM_ValidateUnit, Typical, std::string("ms")); +BENCHMARK_CAPTURE(BM_ValidateUnit, MaxLength, std::string(63, 'u')); + +} // namespace + +BENCHMARK_MAIN(); diff --git a/sdk/test/metrics/instrument_metadata_validator_test.cc b/sdk/test/metrics/instrument_metadata_validator_test.cc index f785553a3f..b9d57d414f 100644 --- a/sdk/test/metrics/instrument_metadata_validator_test.cc +++ b/sdk/test/metrics/instrument_metadata_validator_test.cc @@ -6,6 +6,7 @@ #include #include +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/metrics/instrument_metadata_validator.h" static std::string CreateVeryLargeString(size_t multiple) @@ -31,6 +32,10 @@ TEST(InstrumentMetadataValidator, TestName) "a\\broken\\path", // contains backward slash CreateVeryLargeString(25) + "X", // total 256 characters CreateVeryLargeString(26), // string much bigger than 255 character + std::string("abc\0def", 7), // contains embedded NUL + "ab\xc3\xa9", // non-ASCII letter + "\xc3\xa9t", // starting with non-ASCII letter + "a b", // contains space }; for (auto const &str : invalid_names) { @@ -61,6 +66,7 @@ TEST(InstrumentMetadataValidator, TestUnit) CreateVeryLargeString(5) + "ABCERTYGJ", // total 64 charactes CreateVeryLargeString(7), // string bigger than 63 chars "\x31\x32\x33\xe2\x82\xac\x41\x41\x41\xe2\x82\xac\x42\x42\x42", // unicode string + std::string("m\0s", 3), // contains embedded NUL }; for (auto const &str : invalid_units) { @@ -74,9 +80,50 @@ TEST(InstrumentMetadataValidator, TestUnit) "d1234_-sdsds.sdsd344", // combination of all valid characters CreateVeryLargeString(5) + "ABCERTYG", // total 63 charactes "ASDDSDF", // uppercase + "", // empty + "{request}/s", // annotation and slash + "\x01\x7F", // lowest and highest ASCII characters }; for (auto const &str : valid_units) { - EXPECT_TRUE(validator.ValidateName(str)); + EXPECT_TRUE(validator.ValidateUnit(str)); } } + +// The views below are not NUL-terminated, so the characters after their end +// must not change the result. + +TEST(InstrumentMetadataValidator, NameViewEndsBeforeInvalidCharacter) +{ + opentelemetry::sdk::metrics::InstrumentMetaDataValidator validator; + const std::string buffer = "abc def"; + EXPECT_TRUE(validator.ValidateName(opentelemetry::nostd::string_view(buffer.data(), 3))); +} + +TEST(InstrumentMetadataValidator, EmptyNameViewOverValidName) +{ + opentelemetry::sdk::metrics::InstrumentMetaDataValidator validator; + const std::string buffer = "abcdef"; + EXPECT_FALSE(validator.ValidateName(opentelemetry::nostd::string_view(buffer.data(), 0))); +} + +TEST(InstrumentMetadataValidator, MaxLengthNameViewOverLongerName) +{ + opentelemetry::sdk::metrics::InstrumentMetaDataValidator validator; + const std::string buffer = CreateVeryLargeString(26); // 265 characters + EXPECT_TRUE(validator.ValidateName(opentelemetry::nostd::string_view(buffer.data(), 255))); +} + +TEST(InstrumentMetadataValidator, UnitViewEndsBeforeNonAsciiCharacter) +{ + opentelemetry::sdk::metrics::InstrumentMetaDataValidator validator; + const std::string buffer = "ms\xe2\x82\xac"; + EXPECT_TRUE(validator.ValidateUnit(opentelemetry::nostd::string_view(buffer.data(), 2))); +} + +TEST(InstrumentMetadataValidator, MaxLengthUnitViewOverLongerUnit) +{ + opentelemetry::sdk::metrics::InstrumentMetaDataValidator validator; + const std::string buffer = CreateVeryLargeString(7); // 75 characters + EXPECT_TRUE(validator.ValidateUnit(opentelemetry::nostd::string_view(buffer.data(), 63))); +} From 79f058825e30f1a5084fc3bea4288f2d904e487a Mon Sep 17 00:00:00 2001 From: Lukasz Towarek Date: Mon, 19 Oct 2026 00:57:26 +0200 Subject: [PATCH 3/5] Add the PR number to the CHANGELOG entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2bd46ce43..dd3561a4c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Increment the: ## [Unreleased] * [API/SDK] Add ASCII helpers and remove regex from InstrumentMetaDataValidator - [#XXXX](https://github.com/open-telemetry/opentelemetry-cpp/pull/XXXX) + [#4715](https://github.com/open-telemetry/opentelemetry-cpp/pull/4715) * [SDK] Release provider mutexes before construction-failure diagnostics so custom log handlers can retrieve cached or noop tracers, loggers, and meters From 0e8dda28aae131f452fc43dbd7be0f1952f0094b Mon Sep 17 00:00:00 2001 From: Lukasz Towarek Date: Mon, 19 Oct 2026 01:19:24 +0200 Subject: [PATCH 4/5] Drop EncodeLowerHex and EncodeUpperHex cppcheck reports their callers in SpanId, TraceId and TraceFlags as reading the uninitialized buffers that the B3 propagator passes in. The call sites go back to writing two digits per byte with LowerHexDigit and UpperHexDigit. --- api/include/opentelemetry/baggage/baggage.h | 9 ++--- .../opentelemetry/common/detail/ascii.h | 21 ----------- api/include/opentelemetry/trace/span_id.h | 6 ++- api/include/opentelemetry/trace/trace_flags.h | 3 +- api/include/opentelemetry/trace/trace_id.h | 6 ++- api/test/common/ascii_test.cc | 37 ------------------- exporters/otlp/src/otlp_file_client.cc | 11 ++++-- exporters/otlp/src/otlp_http_client.cc | 12 ++++-- .../src/service_detector_utils.cc | 18 +++++---- 9 files changed, 42 insertions(+), 81 deletions(-) diff --git a/api/include/opentelemetry/baggage/baggage.h b/api/include/opentelemetry/baggage/baggage.h index a5a133d88d..d51e6607c6 100644 --- a/api/include/opentelemetry/baggage/baggage.h +++ b/api/include/opentelemetry/baggage/baggage.h @@ -3,8 +3,6 @@ #pragma once -#include - #include "opentelemetry/common/detail/ascii.h" #include "opentelemetry/common/kv_properties.h" #include "opentelemetry/common/macros.h" @@ -230,10 +228,9 @@ class OPENTELEMETRY_EXPORT Baggage } else { - const auto byte = static_cast(c); - char escaped[] = {'%', '\0', '\0'}; - common::detail::EncodeUpperHex(&byte, 1, &escaped[1]); - ret.append(escaped, sizeof(escaped)); + ret.push_back('%'); + ret.push_back(common::detail::UpperHexDigit(static_cast(c) >> 4)); + ret.push_back(common::detail::UpperHexDigit(c)); } } diff --git a/api/include/opentelemetry/common/detail/ascii.h b/api/include/opentelemetry/common/detail/ascii.h index 3a74c42d58..6f01233f95 100644 --- a/api/include/opentelemetry/common/detail/ascii.h +++ b/api/include/opentelemetry/common/detail/ascii.h @@ -3,7 +3,6 @@ #pragma once -#include #include #include "opentelemetry/version.h" @@ -80,26 +79,6 @@ constexpr char UpperHexDigit(int value) noexcept return "0123456789ABCDEF"[value & 0xF]; } -// Writes two hexadecimal digits per byte, high nibble first, to out, which must -// have room for 2 * size characters. -inline void EncodeLowerHex(const uint8_t *bytes, std::size_t size, char *out) noexcept -{ - for (std::size_t i = 0; i < size; ++i) - { - out[2 * i] = LowerHexDigit(bytes[i] >> 4); - out[2 * i + 1] = LowerHexDigit(bytes[i]); - } -} - -inline void EncodeUpperHex(const uint8_t *bytes, std::size_t size, char *out) noexcept -{ - for (std::size_t i = 0; i < size; ++i) - { - out[2 * i] = UpperHexDigit(bytes[i] >> 4); - out[2 * i + 1] = UpperHexDigit(bytes[i]); - } -} - // 0x00 to 0x1F and 0x7F: what std::iscntrl accepts in the "C" locale. constexpr bool IsAsciiControl(char c) noexcept { diff --git a/api/include/opentelemetry/trace/span_id.h b/api/include/opentelemetry/trace/span_id.h index 503962ed4a..099a44648a 100644 --- a/api/include/opentelemetry/trace/span_id.h +++ b/api/include/opentelemetry/trace/span_id.h @@ -32,7 +32,11 @@ class SpanId final // Populates the buffer with the lowercase base16 representation of the ID. void ToLowerBase16(nostd::span buffer) const noexcept { - common::detail::EncodeLowerHex(rep_, kSize, buffer.data()); + for (size_t i = 0; i < kSize; ++i) + { + buffer[i * 2 + 0] = common::detail::LowerHexDigit(rep_[i] >> 4); + buffer[i * 2 + 1] = common::detail::LowerHexDigit(rep_[i]); + } } // Returns a nostd::span of the ID. diff --git a/api/include/opentelemetry/trace/trace_flags.h b/api/include/opentelemetry/trace/trace_flags.h index f0fc63a136..aec3ba8dd2 100644 --- a/api/include/opentelemetry/trace/trace_flags.h +++ b/api/include/opentelemetry/trace/trace_flags.h @@ -45,7 +45,8 @@ class TraceFlags final // Populates the buffer with the lowercase base16 representation of the flags. void ToLowerBase16(nostd::span buffer) const noexcept { - common::detail::EncodeUpperHex(&rep_, 1, buffer.data()); + buffer[0] = common::detail::UpperHexDigit(rep_ >> 4); + buffer[1] = common::detail::UpperHexDigit(rep_); } uint8_t flags() const noexcept { return rep_; } diff --git a/api/include/opentelemetry/trace/trace_id.h b/api/include/opentelemetry/trace/trace_id.h index 5b04ff7fb5..5433579ddd 100644 --- a/api/include/opentelemetry/trace/trace_id.h +++ b/api/include/opentelemetry/trace/trace_id.h @@ -35,7 +35,11 @@ class TraceId final // Populates the buffer with the lowercase base16 representation of the ID. void ToLowerBase16(nostd::span buffer) const noexcept { - common::detail::EncodeLowerHex(rep_, kSize, buffer.data()); + for (size_t i = 0; i < kSize; ++i) + { + buffer[i * 2 + 0] = common::detail::LowerHexDigit(rep_[i] >> 4); + buffer[i * 2 + 1] = common::detail::LowerHexDigit(rep_[i]); + } } // Returns a nostd::span of the ID. diff --git a/api/test/common/ascii_test.cc b/api/test/common/ascii_test.cc index 7d467ac31a..95e40d343e 100644 --- a/api/test/common/ascii_test.cc +++ b/api/test/common/ascii_test.cc @@ -3,7 +3,6 @@ #include #include -#include #include #include @@ -57,32 +56,6 @@ std::string EncodingMismatches(Encoder encoder, const char *format) return mismatches; } -template -std::string EncodeEveryByte(Encoder encoder) -{ - uint8_t bytes[256]; - for (int byte = 0; byte < 256; ++byte) - { - bytes[byte] = static_cast(byte); - } - std::string encoded(2 * sizeof(bytes), '\0'); - encoder(bytes, sizeof(bytes), &encoded[0]); - return encoded; -} - -// Every byte value in order, each formatted by printf with format. -std::string ReferenceEncoding(const char *format) -{ - std::string encoded; - for (int byte = 0; byte < 256; ++byte) - { - char digits[3]; - std::snprintf(digits, sizeof(digits), format, byte); - encoded += digits; - } - return encoded; -} - } // namespace static_assert(ascii::IsAsciiAlnum('z'), "helpers are usable in constant expressions"); @@ -157,13 +130,3 @@ TEST(AsciiTest, UpperHexDigit) { EXPECT_EQ(EncodingMismatches(ascii::UpperHexDigit, "%X"), ""); } - -TEST(AsciiTest, EncodeLowerHex) -{ - EXPECT_EQ(EncodeEveryByte(ascii::EncodeLowerHex), ReferenceEncoding("%02x")); -} - -TEST(AsciiTest, EncodeUpperHex) -{ - EXPECT_EQ(EncodeEveryByte(ascii::EncodeUpperHex), ReferenceEncoding("%02X")); -} diff --git a/exporters/otlp/src/otlp_file_client.cc b/exporters/otlp/src/otlp_file_client.cc index e47aab8e49..f37e4a3f1c 100644 --- a/exporters/otlp/src/otlp_file_client.cc +++ b/exporters/otlp/src/otlp_file_client.cc @@ -711,9 +711,14 @@ class OPENTELEMETRY_LOCAL_SYMBOL FileSystemUtil static std::string HexEncode(const std::string &bytes) { - std::string ret(bytes.size() * 2, '\0'); - opentelemetry::common::detail::EncodeLowerHex(reinterpret_cast(bytes.data()), - bytes.size(), &ret[0]); + std::string ret; + ret.reserve(bytes.size() * 2); + for (std::string::size_type i = 0; i < bytes.size(); ++i) + { + unsigned char byte = static_cast(bytes[i]); + ret.push_back(opentelemetry::common::detail::LowerHexDigit(byte >> 4)); + ret.push_back(opentelemetry::common::detail::LowerHexDigit(byte)); + } return ret; } diff --git a/exporters/otlp/src/otlp_http_client.cc b/exporters/otlp/src/otlp_http_client.cc index 89c26d3fb0..7a09e3762c 100644 --- a/exporters/otlp/src/otlp_http_client.cc +++ b/exporters/otlp/src/otlp_http_client.cc @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -420,9 +419,14 @@ class ResponseHandler : public http_client::EventHandler static std::string HexEncode(const std::string &bytes) { - std::string ret(bytes.size() * 2, '\0'); - opentelemetry::common::detail::EncodeLowerHex(reinterpret_cast(bytes.data()), - bytes.size(), &ret[0]); + std::string ret; + ret.reserve(bytes.size() * 2); + for (std::string::size_type i = 0; i < bytes.size(); ++i) + { + unsigned char byte = static_cast(bytes[i]); + ret.push_back(opentelemetry::common::detail::LowerHexDigit(byte >> 4)); + ret.push_back(opentelemetry::common::detail::LowerHexDigit(byte)); + } return ret; } diff --git a/resource_detectors/src/service_detector_utils.cc b/resource_detectors/src/service_detector_utils.cc index 2c491d944f..466d796117 100644 --- a/resource_detectors/src/service_detector_utils.cc +++ b/resource_detectors/src/service_detector_utils.cc @@ -35,13 +35,17 @@ constexpr const char *kUnknownServicePrefix = "unknown_service:"; std::string FormatUuidV4(const uint8_t bytes[16]) noexcept { - // Groups of 8, 4, 4, 4 and 12 hexadecimal digits, separated by dashes. - std::string uuid(36, '-'); - opentelemetry::common::detail::EncodeLowerHex(&bytes[0], 4, &uuid[0]); - opentelemetry::common::detail::EncodeLowerHex(&bytes[4], 2, &uuid[9]); - opentelemetry::common::detail::EncodeLowerHex(&bytes[6], 2, &uuid[14]); - opentelemetry::common::detail::EncodeLowerHex(&bytes[8], 2, &uuid[19]); - opentelemetry::common::detail::EncodeLowerHex(&bytes[10], 6, &uuid[24]); + std::string uuid; + uuid.reserve(36); + for (size_t i = 0; i < 16; ++i) + { + if (i == 4 || i == 6 || i == 8 || i == 10) + { + uuid.push_back('-'); + } + uuid.push_back(opentelemetry::common::detail::LowerHexDigit(bytes[i] >> 4)); + uuid.push_back(opentelemetry::common::detail::LowerHexDigit(bytes[i])); + } return uuid; } From f7b9c9c1078d64478311c0129e4757c98c65028b Mon Sep 17 00:00:00 2001 From: Lukasz Towarek Date: Mon, 19 Oct 2026 01:58:55 +0200 Subject: [PATCH 5/5] Fix include-what-you-use findings The opentracing shim propagation test declared an unused common namespace alias and included attribute_value.h only for it. The validator benchmark passes names as nostd::string_view. --- opentracing-shim/test/propagation_test.cc | 2 -- sdk/test/metrics/instrument_metadata_validator_benchmark.cc | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/opentracing-shim/test/propagation_test.cc b/opentracing-shim/test/propagation_test.cc index 6bae33fd5a..bb33e80570 100644 --- a/opentracing-shim/test/propagation_test.cc +++ b/opentracing-shim/test/propagation_test.cc @@ -15,7 +15,6 @@ #include "opentracing/util.h" #include "opentelemetry/baggage/baggage.h" -#include "opentelemetry/common/attribute_value.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/opentracingshim/propagation.h" @@ -23,7 +22,6 @@ #include "shim_mocks.h" namespace baggage = opentelemetry::baggage; -namespace common = opentelemetry::common; namespace nostd = opentelemetry::nostd; namespace shim = opentelemetry::opentracingshim; diff --git a/sdk/test/metrics/instrument_metadata_validator_benchmark.cc b/sdk/test/metrics/instrument_metadata_validator_benchmark.cc index ffe64d8fd9..9a53ef1cea 100644 --- a/sdk/test/metrics/instrument_metadata_validator_benchmark.cc +++ b/sdk/test/metrics/instrument_metadata_validator_benchmark.cc @@ -4,6 +4,7 @@ #include #include +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/metrics/instrument_metadata_validator.h" using opentelemetry::sdk::metrics::InstrumentMetaDataValidator;