From 4dc9e2bd688be85d40482db006614cc188c43cf5 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:32:17 -0300 Subject: [PATCH 1/3] fix(metrics): write label values that are not UTF-8 as \DDD escapes Names taken from packets, such as DNS qnames and DHCP hostnames, can hold any byte. pktvisor passed them unchanged into OTLP string attributes, Prometheus label values and JSON top-N names. A strict OTLP decoder then refused the whole export, Prometheus refused the whole scrape, and the JSON dump threw. Every label value now goes through utf8_label_value. A valid value is unchanged; each byte outside a well-formed UTF-8 sequence becomes \DDD, the decimal escape of DNS presentation format. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 ++ src/MetricLabels.h | 9 ++ src/Metrics.cpp | 80 +++++++++++++++--- src/Metrics.h | 35 ++++---- src/PrometheusSerializer.h | 7 +- src/handlers/dns/v2/tests/test_dns_layer.cpp | 48 +++++++++++ .../fixtures/dns_udp_non_utf8_qname.pcap | Bin 0 -> 210 bytes src/tests/test_metrics.cpp | 71 ++++++++++++++++ src/tests/test_prometheus_serializer.cpp | 11 +++ 9 files changed, 241 insertions(+), 27 deletions(-) create mode 100644 src/tests/fixtures/dns_udp_non_utf8_qname.pcap diff --git a/README.md b/README.md index efabd5375..64ee18002 100644 --- a/README.md +++ b/README.md @@ -536,6 +536,13 @@ To run pktvisor as a sidecar in Kubernetes and scrape it with Prometheus, see Also see [getorb.io](https://getorb.io) for information on connecting pktvisor agents to the Orb observability platform. +#### Names that are not text + +Some names in the metrics come from packets, such as the DNS query names in the top QNAME tables, and a DNS name may +hold any byte. The JSON, Prometheus and OpenTelemetry outputs write these names as UTF-8. Valid UTF-8 is kept as it is, +and each byte that is not part of a valid UTF-8 sequence is written as `\DDD`, its decimal value, as in DNS presentation +format. For example, a query for `www.bad<0xff>name.test` is counted in `top_qname2` as `.bad\255name.test`. + ### REST API REST API documentation is available in [OpenAPI Format](https://app.swaggerhub.com/apis/netboxlabs/pktvisor/3.0.0-oas3) diff --git a/src/MetricLabels.h b/src/MetricLabels.h index 78dea217b..c04582776 100644 --- a/src/MetricLabels.h +++ b/src/MetricLabels.h @@ -5,11 +5,20 @@ #include #include +#include namespace visor { using LabelMap = std::map; +// OTLP, Prometheus and JSON all require label values to be UTF-8, but values +// taken from packets, such as DNS names, can hold any byte. utf8_label_value +// returns a valid value unchanged and writes each byte that is not part of a +// well-formed UTF-8 sequence as \DDD, the decimal escape of DNS presentation +// format, so one such value cannot make a whole export or scrape fail. +bool is_utf8(std::string_view value); +std::string utf8_label_value(std::string_view value); + // Process-global static labels (e.g. instance). Storage defined in Metrics.cpp. LabelMap &prometheus_static_labels_mutable(); inline const LabelMap &prometheus_static_labels() { return prometheus_static_labels_mutable(); } diff --git a/src/Metrics.cpp b/src/Metrics.cpp index 0aba2fe73..b1a56f970 100644 --- a/src/Metrics.cpp +++ b/src/Metrics.cpp @@ -27,11 +27,7 @@ void Counter::to_opentelemetry(metrics::v1::ScopeMetrics &scope, timespec &start gauge_data_point->set_as_int(_value); gauge_data_point->set_start_time_unix_nano(timespec_to_uint64(start)); gauge_data_point->set_time_unix_nano(timespec_to_uint64(end)); - for (const auto &label: add_labels) { - auto attribute = gauge_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(gauge_data_point, add_labels); } void Rate::to_json(json &j, bool include_live) const @@ -85,11 +81,7 @@ void Cardinality::to_opentelemetry(metrics::v1::ScopeMetrics &scope, timespec &s gauge_data_point->set_as_int(lround(_set.get_estimate())); gauge_data_point->set_start_time_unix_nano(timespec_to_uint64(start)); gauge_data_point->set_time_unix_nano(timespec_to_uint64(end)); - for (const auto &label: add_labels) { - auto attribute = gauge_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(gauge_data_point, add_labels); } LabelMap &prometheus_static_labels_mutable() @@ -98,6 +90,74 @@ LabelMap &prometheus_static_labels_mutable() return labels; } +// Length of the well-formed UTF-8 sequence that starts at value[i], or 0 when +// none does. Follows Unicode Table 3-7, so overlong forms, surrogates and code +// points above U+10FFFF are rejected, as Go and protobuf decoders reject them. +static size_t utf8_sequence_length(std::string_view value, size_t i) +{ + const auto lead = static_cast(value[i]); + if (lead < 0x80) { + return 1; + } + size_t len; + unsigned char lo = 0x80, hi = 0xBF; + if (lead >= 0xC2 && lead <= 0xDF) { + len = 2; + } else if (lead >= 0xE0 && lead <= 0xEF) { + len = 3; + lo = lead == 0xE0 ? 0xA0 : 0x80; + hi = lead == 0xED ? 0x9F : 0xBF; + } else if (lead >= 0xF0 && lead <= 0xF4) { + len = 4; + lo = lead == 0xF0 ? 0x90 : 0x80; + hi = lead == 0xF4 ? 0x8F : 0xBF; + } else { + return 0; + } + if (value.size() - i < len) { + return 0; + } + const auto second = static_cast(value[i + 1]); + if (second < lo || second > hi) { + return 0; + } + for (size_t k = 2; k < len; ++k) { + const auto next = static_cast(value[i + k]); + if (next < 0x80 || next > 0xBF) { + return 0; + } + } + return len; +} + +bool is_utf8(std::string_view value) +{ + for (size_t i = 0; i < value.size();) { + const auto len = utf8_sequence_length(value, i); + if (!len) { + return false; + } + i += len; + } + return true; +} + +std::string utf8_label_value(std::string_view value) +{ + std::string out; + out.reserve(value.size()); + for (size_t i = 0; i < value.size();) { + if (const auto len = utf8_sequence_length(value, i)) { + out.append(value.substr(i, len)); + i += len; + } else { + fmt::format_to(std::back_inserter(out), "\\{:03d}", static_cast(value[i])); + ++i; + } + } + return out; +} + void Metric::name_json_assign(json &j, const json &val) const { json *j_part = &j; diff --git a/src/Metrics.h b/src/Metrics.h index 0067cb56e..16adefa89 100644 --- a/src/Metrics.h +++ b/src/Metrics.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #define HIST_MIN_EXP -9 @@ -61,6 +62,16 @@ static inline uint64_t timespec_to_uint64(timespec &stamp) return stamp.tv_sec * 1000000000ULL + stamp.tv_nsec; } +template +static inline void add_label_attributes(DataPoint *data_point, const LabelMap &labels) +{ + for (const auto &label : labels) { + auto attribute = data_point->add_attributes(); + attribute->set_key(label.first); + attribute->mutable_value()->set_string_value(utf8_label_value(label.second)); + } +} + // Single source of truth for the SLA quantiles pktvisor reports, in the order // get_quantiles() produces them. Each emit path (JSON, Prometheus, OpenTelemetry) // derives its label/rank from this table, so values and labels can never desync. @@ -338,11 +349,7 @@ class Histogram final : public Metric } hist_data_point->set_count(_sketch.get_n()); - for (const auto &label : add_labels) { - auto attribute = hist_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(hist_data_point, add_labels); } }; @@ -481,11 +488,7 @@ class Quantile final : public Metric quantile->set_quantile(QUANTILES[i].rank); quantile->set_value(quantiles[i]); } - for (const auto &label : add_labels) { - auto attribute = summary_data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(summary_data_point, add_labels); } }; @@ -534,11 +537,7 @@ class TopN final : public Metric data_point->set_as_int(value); data_point->set_start_time_unix_nano(start); data_point->set_time_unix_nano(end); - for (const auto &label : l) { - auto attribute = data_point->add_attributes(); - attribute->set_key(label.first); - attribute->mutable_value()->set_string_value(label.second); - } + add_label_attributes(data_point, l); } public: @@ -668,7 +667,11 @@ class TopN final : public Metric auto threshold = _get_threshold(items); for (uint64_t i = 0; i < std::min(_top_count, items.size()); i++) { if (items[i].get_estimate() >= threshold) { - section[i]["name"] = items[i].get_item(); + if constexpr (std::is_same_v) { + section[i]["name"] = utf8_label_value(items[i].get_item()); + } else { + section[i]["name"] = items[i].get_item(); + } section[i]["estimate"] = items[i].get_estimate(); } else { break; diff --git a/src/PrometheusSerializer.h b/src/PrometheusSerializer.h index 3d53a254d..851d857e2 100644 --- a/src/PrometheusSerializer.h +++ b/src/PrometheusSerializer.h @@ -92,9 +92,14 @@ class PrometheusSerializer } } - // Label-value escaping: backslash, double-quote, newline. + // Label-value escaping: backslash, double-quote, newline. A value that is not + // UTF-8 is first rendered by utf8_label_value, as the OTLP output does. static void append_escaped(std::string &out, const std::string &v) { + if (!is_utf8(v)) { + append_escaped(out, utf8_label_value(v)); + return; + } for (char c : v) { switch (c) { case '\\': out.append("\\\\"); break; diff --git a/src/handlers/dns/v2/tests/test_dns_layer.cpp b/src/handlers/dns/v2/tests/test_dns_layer.cpp index d6db6198f..89d4f9526 100644 --- a/src/handlers/dns/v2/tests/test_dns_layer.cpp +++ b/src/handlers/dns/v2/tests/test_dns_layer.cpp @@ -1059,3 +1059,51 @@ TEST_CASE("DNS v2 specialized_merge aggregates two buckets", "[dns][unit]") target->to_opentelemetry(scope_after, st, et, {}); CHECK(otel_gauge_sum(scope_after, "dns_xacts") == pre_b1 + pre_b2); } + +TEST_CASE("DNS qname that is not UTF-8 is escaped in every output", "[pcap][dns]") +{ + PcapInputStream stream{"pcap-test"}; + stream.config_set("pcap_file", "tests/fixtures/dns_udp_non_utf8_qname.pcap"); + stream.config_set("bpf", ""); + + visor::Config c; + auto stream_proxy = stream.add_event_proxy(c); + c.config_set("num_periods", 1); + DnsStreamHandler dns_handler{"dns-test", stream_proxy, &c}; + + dns_handler.start(); + stream.start(); + dns_handler.stop(); + stream.stop(); + + // The fixture's qname is www.bad<0xff>name.test; byte 0xff is written as \255. + const std::string escaped = R"(.bad\255name.test)"; + + json j; + dns_handler.metrics()->bucket(0)->to_json(j); + CHECK(j["unknown"]["top_qname2_xacts"][0]["name"] == escaped); + CHECK_NOTHROW(j.dump()); + + visor::PrometheusSerializer prom; + dns_handler.metrics()->bucket(0)->to_prometheus(prom, {}); + CHECK(prom.finalize().find(R"(qname=".bad\\255name.test")") != std::string::npos); + + opentelemetry::proto::metrics::v1::ScopeMetrics scope; + timespec start_ts{}, end_ts{}; + dns_handler.metrics()->bucket(0)->to_opentelemetry(scope, start_ts, end_ts, {}); + std::string qname; + for (const auto &m : scope.metrics()) { + if (m.name() == "dns_top_qname2_xacts") { + for (const auto &a : m.gauge().data_points(0).attributes()) { + if (a.key() == "qname") { + qname = a.value().string_value(); + } + } + } + } + CHECK(qname == escaped); + + // A strict decoder, like the agent's, refuses the whole export when one string is not UTF-8. + opentelemetry::proto::metrics::v1::ScopeMetrics decoded; + CHECK(decoded.ParseFromString(scope.SerializeAsString())); +} diff --git a/src/tests/fixtures/dns_udp_non_utf8_qname.pcap b/src/tests/fixtures/dns_udp_non_utf8_qname.pcap new file mode 100644 index 0000000000000000000000000000000000000000..47508514e94aeef9b5701409e1dc26118101aae9 GIT binary patch literal 210 zcmca|c+)~A1{MYw`2U}Qff2;{7@P{Ce4&^L#9;!Hj2sNE3=H-lWe$Shat|;taRIUE zc@73s25q1YBcKG>2nGh`^73+yq{Ni}d5O8HEG4PMB_JtAkl`2Di8S04WB~)nc(CD( I4Fn7a08W@B%>V!Z literal 0 HcmV?d00001 diff --git a/src/tests/test_metrics.cpp b/src/tests/test_metrics.cpp index e0f9ec0d6..fa92e235c 100644 --- a/src/tests/test_metrics.cpp +++ b/src/tests/test_metrics.cpp @@ -601,3 +601,74 @@ TEST_CASE("Rate metrics", "[metrics][rate]") r.to_opentelemetry(scope, stamp, stamp, {{"policy", "default"}}); } } + +static std::string otel_attribute(const metrics::v1::NumberDataPoint &point, const std::string &key) +{ + for (const auto &attribute : point.attributes()) { + if (attribute.key() == key) { + return attribute.value().string_value(); + } + } + return {}; +} + +TEST_CASE("Label values that are not UTF-8", "[metrics][utf8]") +{ + SECTION("valid UTF-8 is unchanged") + { + for (const auto &value : std::vector{"", "dns.example", "caf\xc3\xa9", "\xed\x9f\xbf", "\xee\x80\x80", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", std::string("a\0b", 3)}) { + CHECK(is_utf8(value)); + CHECK(utf8_label_value(value) == value); + } + } + + SECTION("each byte outside a UTF-8 sequence gets a decimal escape") + { + const std::vector> cases{ + {"\xff", R"(\255)"}, + {"a\x80", R"(a\128)"}, // lone continuation byte + {"\xc3(", R"(\195()"}, // truncated sequence, next byte kept + {"\xc0\xaf", R"(\192\175)"}, // overlong form + {"\xe0\x80\xaf", R"(\224\128\175)"}, // overlong form + {"\xed\xa0\x80", R"(\237\160\128)"}, // surrogate + {"\xf4\x90\x80\x80", R"(\244\144\128\128)"}, // above U+10FFFF + {"\xf5\x80\x80\x80", R"(\245\128\128\128)"}, + {"caf\xc3\xa9\xff", "caf\xc3\xa9\\255"}, + }; + for (const auto &[value, escaped] : cases) { + CHECK_FALSE(is_utf8(value)); + CHECK(utf8_label_value(value) == escaped); + CHECK(is_utf8(utf8_label_value(value))); + } + } + + SECTION("every output escapes them") + { + Metric::reset_static_labels(); + const std::string raw = "bad\xff" + "name"; + const std::string escaped = R"(bad\255name)"; + TopN top("root", "string", {"test", "metric"}, "A topn test metric"); + top.update(raw); + + json j; + top.to_json(j); + CHECK(j["test"]["metric"][0]["name"] == escaped); + CHECK_NOTHROW(j.dump()); + + PrometheusSerializer ser; + top.to_prometheus(ser, {{"policy", "default"}}); + CHECK(ser.finalize().find(R"(string="bad\\255name")") != std::string::npos); + + metrics::v1::ScopeMetrics scope; + timespec stamp{}; + top.to_opentelemetry(scope, stamp, stamp, {{"policy", "default"}}); + Counter c("root", {"counter"}, "A counter test metric"); + c.to_opentelemetry(scope, stamp, stamp, {{"policy", raw}}); + CHECK(otel_attribute(scope.metrics(0).gauge().data_points(0), "string") == escaped); + CHECK(otel_attribute(scope.metrics(1).gauge().data_points(0), "policy") == escaped); + + metrics::v1::ScopeMetrics decoded; + CHECK(decoded.ParseFromString(scope.SerializeAsString())); + } +} diff --git a/src/tests/test_prometheus_serializer.cpp b/src/tests/test_prometheus_serializer.cpp index fee40f724..ac3def42f 100644 --- a/src/tests/test_prometheus_serializer.cpp +++ b/src/tests/test_prometheus_serializer.cpp @@ -71,6 +71,17 @@ TEST_CASE("label values are escaped", "[prometheus][serializer]") "m{desc=\"a\\\"b\\\\c\\nd\"} 1\n"); } +TEST_CASE("label values that are not UTF-8 get decimal byte escapes", "[prometheus][serializer]") +{ + Metric::reset_static_labels(); + PrometheusSerializer s; + s.write("m", Type::Gauge, "d", {}, {{"desc", "a\"b\xff"}}, 1); + CHECK(s.finalize() == + "# HELP m d\n" + "# TYPE m gauge\n" + "m{desc=\"a\\\"b\\\\255\"} 1\n"); +} + TEST_CASE("numeric formatting matches ostream operator<<", "[prometheus][serializer]") { Metric::reset_static_labels(); From a91d5866b6ed748e089fa68740e7cc4d488f0ab1 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:44:54 -0300 Subject: [PATCH 2/3] fix(metrics): keep escaped label values distinct from literal backslash text A backslash followed by three digits is now written as \092, so the text \255 and the byte 0xff no longer render as the same label. Valid values take a fast path, and the tests cover truncated, overlong and boundary sequences. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- src/MetricLabels.h | 9 ++++--- src/Metrics.cpp | 28 +++++++++++++++++++- src/Metrics.h | 2 +- src/PrometheusSerializer.h | 14 +++++----- src/handlers/dns/v2/tests/test_dns_layer.cpp | 8 +++--- src/tests/test_metrics.cpp | 20 +++++++++++++- src/tests/test_prometheus_serializer.cpp | 4 ++- 8 files changed, 71 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 64ee18002..759d7c78d 100644 --- a/README.md +++ b/README.md @@ -541,7 +541,8 @@ Also see [getorb.io](https://getorb.io) for information on connecting pktvisor a Some names in the metrics come from packets, such as the DNS query names in the top QNAME tables, and a DNS name may hold any byte. The JSON, Prometheus and OpenTelemetry outputs write these names as UTF-8. Valid UTF-8 is kept as it is, and each byte that is not part of a valid UTF-8 sequence is written as `\DDD`, its decimal value, as in DNS presentation -format. For example, a query for `www.bad<0xff>name.test` is counted in `top_qname2` as `.bad\255name.test`. +format. For example, a query for `www.bad<0xff>name.test` is counted in `top_qname2` as `.bad\255name.test`. A +backslash followed by three digits is written as `\092`, so a name that holds the text `\255` stays distinct. ### REST API diff --git a/src/MetricLabels.h b/src/MetricLabels.h index c04582776..f7dd55b8c 100644 --- a/src/MetricLabels.h +++ b/src/MetricLabels.h @@ -13,10 +13,13 @@ using LabelMap = std::map; // OTLP, Prometheus and JSON all require label values to be UTF-8, but values // taken from packets, such as DNS names, can hold any byte. utf8_label_value -// returns a valid value unchanged and writes each byte that is not part of a -// well-formed UTF-8 sequence as \DDD, the decimal escape of DNS presentation -// format, so one such value cannot make a whole export or scrape fail. +// writes each byte that is not part of a well-formed UTF-8 sequence as \DDD, +// the decimal escape of DNS presentation format, so one such value cannot make +// a whole export or scrape fail. A backslash followed by three digits becomes +// \092, so two different values never render the same. Any other value is +// returned unchanged; label_value_needs_escape tells the two cases apart. bool is_utf8(std::string_view value); +bool label_value_needs_escape(std::string_view value); std::string utf8_label_value(std::string_view value); // Process-global static labels (e.g. instance). Storage defined in Metrics.cpp. diff --git a/src/Metrics.cpp b/src/Metrics.cpp index b1a56f970..dbde83dc9 100644 --- a/src/Metrics.cpp +++ b/src/Metrics.cpp @@ -130,6 +130,14 @@ static size_t utf8_sequence_length(std::string_view value, size_t i) return len; } +// True when value[i] is a backslash followed by three ASCII digits, which would +// read as an escaped byte unless it is escaped itself. +static bool backslash_before_digits(std::string_view value, size_t i) +{ + auto digit = [&](size_t k) { return value[k] >= '0' && value[k] <= '9'; }; + return value[i] == '\\' && value.size() - i > 3 && digit(i + 1) && digit(i + 2) && digit(i + 3); +} + bool is_utf8(std::string_view value) { for (size_t i = 0; i < value.size();) { @@ -142,12 +150,30 @@ bool is_utf8(std::string_view value) return true; } +bool label_value_needs_escape(std::string_view value) +{ + for (size_t i = 0; i < value.size();) { + const auto len = utf8_sequence_length(value, i); + if (!len || backslash_before_digits(value, i)) { + return true; + } + i += len; + } + return false; +} + std::string utf8_label_value(std::string_view value) { + if (!label_value_needs_escape(value)) { + return std::string(value); + } std::string out; out.reserve(value.size()); for (size_t i = 0; i < value.size();) { - if (const auto len = utf8_sequence_length(value, i)) { + if (backslash_before_digits(value, i)) { + out.append("\\092"); + ++i; + } else if (const auto len = utf8_sequence_length(value, i)) { out.append(value.substr(i, len)); i += len; } else { diff --git a/src/Metrics.h b/src/Metrics.h index 16adefa89..47534f9a0 100644 --- a/src/Metrics.h +++ b/src/Metrics.h @@ -63,7 +63,7 @@ static inline uint64_t timespec_to_uint64(timespec &stamp) } template -static inline void add_label_attributes(DataPoint *data_point, const LabelMap &labels) +inline void add_label_attributes(DataPoint *data_point, const LabelMap &labels) { for (const auto &label : labels) { auto attribute = data_point->add_attributes(); diff --git a/src/PrometheusSerializer.h b/src/PrometheusSerializer.h index 851d857e2..a3f2a760b 100644 --- a/src/PrometheusSerializer.h +++ b/src/PrometheusSerializer.h @@ -92,15 +92,17 @@ class PrometheusSerializer } } - // Label-value escaping: backslash, double-quote, newline. A value that is not - // UTF-8 is first rendered by utf8_label_value, as the OTLP output does. + // Label-value escaping: backslash, double-quote, newline. A value that needs it + // is first rendered by utf8_label_value, as the OTLP output does. static void append_escaped(std::string &out, const std::string &v) { - if (!is_utf8(v)) { - append_escaped(out, utf8_label_value(v)); - return; + std::string rendered; + std::string_view text = v; + if (label_value_needs_escape(v)) { + rendered = utf8_label_value(v); + text = rendered; } - for (char c : v) { + for (char c : text) { switch (c) { case '\\': out.append("\\\\"); break; case '"': out.append("\\\""); break; diff --git a/src/handlers/dns/v2/tests/test_dns_layer.cpp b/src/handlers/dns/v2/tests/test_dns_layer.cpp index 89d4f9526..aa23e9059 100644 --- a/src/handlers/dns/v2/tests/test_dns_layer.cpp +++ b/src/handlers/dns/v2/tests/test_dns_layer.cpp @@ -1094,9 +1094,11 @@ TEST_CASE("DNS qname that is not UTF-8 is escaped in every output", "[pcap][dns] std::string qname; for (const auto &m : scope.metrics()) { if (m.name() == "dns_top_qname2_xacts") { - for (const auto &a : m.gauge().data_points(0).attributes()) { - if (a.key() == "qname") { - qname = a.value().string_value(); + for (const auto &point : m.gauge().data_points()) { + for (const auto &a : point.attributes()) { + if (a.key() == "qname") { + qname = a.value().string_value(); + } } } } diff --git a/src/tests/test_metrics.cpp b/src/tests/test_metrics.cpp index fa92e235c..3249c8126 100644 --- a/src/tests/test_metrics.cpp +++ b/src/tests/test_metrics.cpp @@ -616,20 +616,37 @@ TEST_CASE("Label values that are not UTF-8", "[metrics][utf8]") { SECTION("valid UTF-8 is unchanged") { - for (const auto &value : std::vector{"", "dns.example", "caf\xc3\xa9", "\xed\x9f\xbf", "\xee\x80\x80", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", std::string("a\0b", 3)}) { + for (const auto &value : std::vector{"", "dns.example", "caf\xc3\xa9", "\xe0\xa0\x80", "\xed\x9f\xbf", "\xee\x80\x80", "\xf0\x90\x80\x80", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", std::string("a\0b", 3)}) { CHECK(is_utf8(value)); + CHECK_FALSE(label_value_needs_escape(value)); CHECK(utf8_label_value(value) == value); } } + SECTION("a backslash is escaped only before three digits") + { + for (const auto &value : std::vector{R"(\Device\NPF_{0})", R"(a\12)", R"(a\12b)", "\\"}) { + CHECK_FALSE(label_value_needs_escape(value)); + CHECK(utf8_label_value(value) == value); + } + CHECK(utf8_label_value(R"(a\255b)") == R"(a\092255b)"); + CHECK(utf8_label_value("\\\xff") == R"(\\255)"); + // The literal text \255 and the byte 0xff must not render the same. + CHECK(utf8_label_value(R"(\255)") != utf8_label_value("\xff")); + } + SECTION("each byte outside a UTF-8 sequence gets a decimal escape") { const std::vector> cases{ {"\xff", R"(\255)"}, {"a\x80", R"(a\128)"}, // lone continuation byte {"\xc3(", R"(\195()"}, // truncated sequence, next byte kept + {"\xe2\x82", R"(\226\130)"}, // truncated at the end of the value + {"\xf0\x9f\x98", R"(\240\159\152)"}, // truncated at the end of the value {"\xc0\xaf", R"(\192\175)"}, // overlong form + {"\xc1\x81", R"(\193\129)"}, // overlong form {"\xe0\x80\xaf", R"(\224\128\175)"}, // overlong form + {"\xf0\x8f\xbf\xbf", R"(\240\143\191\191)"}, // overlong form {"\xed\xa0\x80", R"(\237\160\128)"}, // surrogate {"\xf4\x90\x80\x80", R"(\244\144\128\128)"}, // above U+10FFFF {"\xf5\x80\x80\x80", R"(\245\128\128\128)"}, @@ -637,6 +654,7 @@ TEST_CASE("Label values that are not UTF-8", "[metrics][utf8]") }; for (const auto &[value, escaped] : cases) { CHECK_FALSE(is_utf8(value)); + CHECK(label_value_needs_escape(value)); CHECK(utf8_label_value(value) == escaped); CHECK(is_utf8(utf8_label_value(value))); } diff --git a/src/tests/test_prometheus_serializer.cpp b/src/tests/test_prometheus_serializer.cpp index ac3def42f..b5ca92d26 100644 --- a/src/tests/test_prometheus_serializer.cpp +++ b/src/tests/test_prometheus_serializer.cpp @@ -76,10 +76,12 @@ TEST_CASE("label values that are not UTF-8 get decimal byte escapes", "[promethe Metric::reset_static_labels(); PrometheusSerializer s; s.write("m", Type::Gauge, "d", {}, {{"desc", "a\"b\xff"}}, 1); + s.write("m", Type::Gauge, "d", {}, {{"desc", "a\\255"}}, 2); CHECK(s.finalize() == "# HELP m d\n" "# TYPE m gauge\n" - "m{desc=\"a\\\"b\\\\255\"} 1\n"); + "m{desc=\"a\\\"b\\\\255\"} 1\n" + "m{desc=\"a\\\\092255\"} 2\n"); } TEST_CASE("numeric formatting matches ostream operator<<", "[prometheus][serializer]") From 28989b733ebf73f9ba179bf8b1090453ad9427cf Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:49:27 -0300 Subject: [PATCH 3/3] docs: drop the README note; the label format is documented in orb-agent Co-Authored-By: Claude Opus 5.5 --- README.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/README.md b/README.md index 759d7c78d..efabd5375 100644 --- a/README.md +++ b/README.md @@ -536,14 +536,6 @@ To run pktvisor as a sidecar in Kubernetes and scrape it with Prometheus, see Also see [getorb.io](https://getorb.io) for information on connecting pktvisor agents to the Orb observability platform. -#### Names that are not text - -Some names in the metrics come from packets, such as the DNS query names in the top QNAME tables, and a DNS name may -hold any byte. The JSON, Prometheus and OpenTelemetry outputs write these names as UTF-8. Valid UTF-8 is kept as it is, -and each byte that is not part of a valid UTF-8 sequence is written as `\DDD`, its decimal value, as in DNS presentation -format. For example, a query for `www.bad<0xff>name.test` is counted in `top_qname2` as `.bad\255name.test`. A -backslash followed by three digits is written as `\092`, so a name that holds the text `\255` stays distinct. - ### REST API REST API documentation is available in [OpenAPI Format](https://app.swaggerhub.com/apis/netboxlabs/pktvisor/3.0.0-oas3)