fix(metrics): write label values that are not UTF-8 as \DDD escapes

src/MetricLabels.h

@@ -5,11 +5,23 @@
 
 #include <map>
 #include <string>
+#include <string_view>
 
 namespace visor {
 
 using LabelMap = std::map<std::string, std::string>;
 
+// 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
+// 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.
 LabelMap &prometheus_static_labels_mutable();
 inline const LabelMap &prometheus_static_labels() { return prometheus_static_labels_mutable(); }

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,100 @@ 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<unsigned char>(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<unsigned char>(value[i + 1]);
+    if (second < lo || second > hi) {
+        return 0;
+    }
+    for (size_t k = 2; k < len; ++k) {
+        const auto next = static_cast<unsigned char>(value[i + k]);
+        if (next < 0x80 || next > 0xBF) {
+            return 0;
+        }
+    }
+    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();) {
+        const auto len = utf8_sequence_length(value, i);
+        if (!len) {
+            return false;
+        }
+        i += len;
+    }
+    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 (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 {
+            fmt::format_to(std::back_inserter(out), "\\{:03d}", static_cast<unsigned char>(value[i]));
+            ++i;
+        }
+    }
+    return out;
+}
+
 void Metric::name_json_assign(json &j, const json &val) const
 {
     json *j_part = &j;

src/Metrics.h

@@ -31,6 +31,7 @@
 #include <regex>
 #include <set>
 #include <shared_mutex>
+#include <type_traits>
 #include <vector>
 
 #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 <typename DataPoint>
+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<T, std::string>) {
+                    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;

src/PrometheusSerializer.h

@@ -92,10 +92,17 @@ class PrometheusSerializer
         }
     }
 
-    // Label-value escaping: backslash, double-quote, newline.
+    // 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)
     {
-        for (char c : v) {
+        std::string rendered;
+        std::string_view text = v;
+        if (label_value_needs_escape(v)) {
+            rendered = utf8_label_value(v);
+            text = rendered;
+        }
+        for (char c : text) {
             switch (c) {
             case '\\': out.append("\\\\"); break;
             case '"': out.append("\\\""); break;

src/handlers/dns/v2/tests/test_dns_layer.cpp

@@ -1059,3 +1059,53 @@ 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<uint64_t>("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 &point : m.gauge().data_points()) {
+                for (const auto &a : point.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()));
+}

src/tests/test_metrics.cpp

@@ -601,3 +601,92 @@ 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<std::string>{"", "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<std::string>{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<std::pair<std::string, std::string>> 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)"},
+            {"caf\xc3\xa9\xff", "caf\xc3\xa9\\255"},
+        };
+        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)));
+        }
+    }
+
+    SECTION("every output escapes them")
+    {
+        Metric::reset_static_labels();
+        const std::string raw = "bad\xff"
+                                "name";
+        const std::string escaped = R"(bad\255name)";
+        TopN<std::string> 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()));
+    }
+}