Sanitize qname on top metrics

dmachard · Dec 15, 2023 · ae96478 · ae96478
1 parent b2f385b
commit ae96478
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 14 deletions.
diff --git a/loggers/prometheus.go b/loggers/prometheus.go
@@ -9,6 +9,7 @@ import (
 	"os"
 	"regexp"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -521,42 +522,42 @@ func (c *PrometheusCountersSet) Collect(ch chan<- prometheus.Metric) {
 	)
 	for _, r := range c.topDomains.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopDomains, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topNxDomains.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopNxDomains, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topSfDomains.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopSfDomains, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topRequesters.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopRequesters, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topTlds.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopTlds, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topETLDPlusOne.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopETldsPlusOne, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topSuspicious.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopSuspicious, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	for _, r := range c.topEvicted.Get() {
 		ch <- prometheus.MustNewConstMetric(c.prom.gaugeTopEvicted, prometheus.GaugeValue,
-			float64(r.Hit), r.Name)
+			float64(r.Hit), strings.ToValidUTF8(r.Name, "�"))
 	}
 
 	ch <- prometheus.MustNewConstMetric(c.prom.gaugeEps, prometheus.GaugeValue,

diff --git a/loggers/prometheus_test.go b/loggers/prometheus_test.go
@@ -329,14 +329,35 @@ func TestPrometheus_QnameInvalidChars(t *testing.T) {
 	// config.Loggers.Prometheus.HistogramMetricsEnabled = true
 	g := NewPrometheus(config, logger.New(false), "test")
 
+	// prepare qname
+	qnameInvalid := "lb._dns-sd._udp.\xd0\xdfP\x01"
+	qnameValidUTF8 := strings.ToValidUTF8(qnameInvalid, "�")
+
 	// record one dns message to simulate some incoming data
-	query := dnsutils.GetFakeDNSMessage()
-	query.DNS.Qname = "lb._dns-sd._udp.\xd0\xdfP\x01"
+	dm := dnsutils.GetFakeDNSMessage()
+	dm.DNS.Qname = qnameInvalid
+	g.Record(dm)
 
-	g.Record(query)
+	// record one dns message to simulate some incoming data
+	dmNx := dnsutils.GetFakeDNSMessage()
+	dmNx.DNS.Qname = qnameInvalid
+	dmNx.DNS.Rcode = "NXDOMAIN"
+	g.Record(dmNx)
 
-	// check metric
-	mf := getMetrics(g, t)
-	ensureMetricValue(t, mf, "dnscollector_top_domains", map[string]string{"domain": "dns.collector"}, 1)
+	// record one dns message to simulate some incoming data
+	dmSf := dnsutils.GetFakeDNSMessage()
+	dmSf.DNS.Qname = qnameInvalid
+	dmSf.DNS.Rcode = "SERVFAIL"
+	g.Record(dmSf)
 
+	mf := getMetrics(g, t)
+	if !ensureMetricValue(t, mf, "dnscollector_top_domains", map[string]string{"domain": qnameValidUTF8}, 1) {
+		t.Errorf("Cannot validate dnscollector_top_domains!")
+	}
+	if !ensureMetricValue(t, mf, "dnscollector_top_nxdomains", map[string]string{"domain": qnameValidUTF8}, 1) {
+		t.Errorf("Cannot validate dnscollector_top_nxdomains!")
+	}
+	if !ensureMetricValue(t, mf, "dnscollector_top_sfdomains", map[string]string{"domain": qnameValidUTF8}, 1) {
+		t.Errorf("Cannot validate dnscollector_top_sfdomains!")
+	}
 }