logger prometheus: fix panic error with invalid qname (#514)

* new test to reproduce #509 * Sanitize qname on top metrics
dmachard · Dec 15, 2023 · 3d9564d · 3d9564d
1 parent ed8d789
commit 3d9564d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 8 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
@@ -323,3 +323,41 @@ func getMetrics(prom *Prometheus, t *testing.T) map[string]*dto.MetricFamily {
 	}
 	return mf
 }
+
+func TestPrometheus_QnameInvalidChars(t *testing.T) {
+	config := pkgconfig.GetFakeConfig()
+	// 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
+	dm := dnsutils.GetFakeDNSMessage()
+	dm.DNS.Qname = qnameInvalid
+	g.Record(dm)
+
+	// record one dns message to simulate some incoming data
+	dmNx := dnsutils.GetFakeDNSMessage()
+	dmNx.DNS.Qname = qnameInvalid
+	dmNx.DNS.Rcode = "NXDOMAIN"
+	g.Record(dmNx)
+
+	// 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!")
+	}
+}