fix(core): reject invalid label names

core/include/prometheus/check_names.h

@@ -3,9 +3,11 @@
 #include <string>
 
 #include "prometheus/detail/core_export.h"
+#include "prometheus/metric_type.h"
 
 namespace prometheus {
 
 PROMETHEUS_CPP_CORE_EXPORT bool CheckMetricName(const std::string& name);
-PROMETHEUS_CPP_CORE_EXPORT bool CheckLabelName(const std::string& name);
+PROMETHEUS_CPP_CORE_EXPORT bool CheckLabelName(const std::string& name,
+                                               MetricType type);
 }  // namespace prometheus

core/src/check_names.cc

@@ -62,8 +62,9 @@ bool CheckMetricName(const std::string& name) {
 /// \see https://prometheus.io/docs/concepts/data_model/
 ///
 /// \param name label name
+/// \param type metric type
 /// \return true is valid, false otherwise
-bool CheckLabelName(const std::string& name) {
+bool CheckLabelName(const std::string& name, MetricType type) {
   if (!nameStartsValid(name)) {
     return false;
   }
@@ -72,6 +73,11 @@ bool CheckLabelName(const std::string& name) {
     return isLocaleIndependentAlphaNumeric(c) || c == '_';
   };
 
+  if ((type == MetricType::Histogram && name == "le") ||
+      (type == MetricType::Summary && name == "quantile")) {
+    return false;
+  }
+
   auto mismatch =
       std::find_if_not(std::begin(name), std::end(name), validLabelCharacters);
   return mismatch == std::end(name);

core/src/family.cc

@@ -24,7 +24,7 @@ Family<T>::Family(const std::string& name, const std::string& help,
   }
   for (auto& label_pair : constant_labels_) {
     auto& label_name = label_pair.first;
-    if (!CheckLabelName(label_name)) {
+    if (!CheckLabelName(label_name, T::metric_type)) {
       throw std::invalid_argument("Invalid label name");
     }
   }
@@ -41,7 +41,7 @@ T& Family<T>::Add(const Labels& labels, std::unique_ptr<T> object) {
     // insertion took place, retroactively check for unlikely issues
     for (auto& label_pair : labels) {
       const auto& label_name = label_pair.first;
-      if (!CheckLabelName(label_name)) {
+      if (!CheckLabelName(label_name, T::metric_type)) {
         metrics_.erase(insert_result.first);
         throw std::invalid_argument("Invalid label name");
       }

core/tests/CMakeLists.txt

@@ -1,7 +1,8 @@
 
 add_executable(prometheus_core_test
   builder_test.cc
-  check_names_test.cc
+  check_label_name_test.cc
+  check_metric_name_test.cc
   counter_test.cc
   family_test.cc
   gauge_test.cc

core/tests/check_label_name_test.cc

@@ -0,0 +1,47 @@
+#include <gtest/gtest.h>
+
+#include "prometheus/check_names.h"
+
+namespace prometheus {
+namespace {
+
+class CheckLabelNameTest : public testing::TestWithParam<MetricType> {
+ protected:
+  bool CheckLabelName(const std::string& name) {
+    return ::prometheus::CheckLabelName(name, GetParam());
+  }
+};
+
+TEST_P(CheckLabelNameTest, empty_label_name) {
+  EXPECT_FALSE(CheckLabelName(""));
+}
+TEST_P(CheckLabelNameTest, invalid_label_name) {
+  EXPECT_FALSE(CheckLabelName("log-level"));
+}
+TEST_P(CheckLabelNameTest, leading_invalid_label_name) {
+  EXPECT_FALSE(CheckLabelName("-abcd"));
+}
+TEST_P(CheckLabelNameTest, trailing_invalid_label_name) {
+  EXPECT_FALSE(CheckLabelName("abcd-"));
+}
+TEST_P(CheckLabelNameTest, good_label_name) {
+  EXPECT_TRUE(CheckLabelName("type"));
+}
+TEST_P(CheckLabelNameTest, reserved_label_name) {
+  EXPECT_FALSE(CheckLabelName("__some_reserved_label"));
+}
+TEST_P(CheckLabelNameTest, reject_le_for_histogram) {
+  EXPECT_EQ(GetParam() != MetricType::Histogram, CheckLabelName("le"));
+}
+TEST_P(CheckLabelNameTest, reject_quantile_for_histogram) {
+  EXPECT_EQ(GetParam() != MetricType::Summary, CheckLabelName("quantile"));
+}
+
+INSTANTIATE_TEST_SUITE_P(AllMetricTypes, CheckLabelNameTest,
+                         testing::Values(MetricType::Counter, MetricType::Gauge,
+                                         MetricType::Histogram,
+                                         MetricType::Summary,
+                                         MetricType::Untyped));
+
+}  // namespace
+}  // namespace prometheus

core/tests/check_metric_name_test.cc

@@ -0,0 +1,22 @@
+#include <gtest/gtest.h>
+
+#include "prometheus/check_names.h"
+
+namespace prometheus {
+namespace {
+
+TEST(CheckMetricNameTest, empty_metric_name) {
+  EXPECT_FALSE(CheckMetricName(""));
+}
+TEST(CheckMetricNameTest, good_metric_name) {
+  EXPECT_TRUE(CheckMetricName("prometheus_notifications_total"));
+}
+TEST(CheckMetricNameTest, reserved_metric_name) {
+  EXPECT_FALSE(CheckMetricName("__some_reserved_metric"));
+}
+TEST(CheckMetricNameTest, malformed_metric_name) {
+  EXPECT_FALSE(CheckMetricName("fa mi ly with space in name or |"));
+}
+
+}  // namespace
+}  // namespace prometheus

core/tests/family_test.cc

@@ -9,6 +9,7 @@
 #include "prometheus/counter.h"
 #include "prometheus/histogram.h"
 #include "prometheus/labels.h"
+#include "prometheus/summary.h"
 
 namespace prometheus {
 namespace {
@@ -115,5 +116,28 @@ TEST(FamilyTest, query_family_if_metric_already_exists) {
   EXPECT_FALSE(family.Has({{"name", "couner2"}}));
 }
 
+TEST(FamilyTest, reject_histogram_with_constant_le_label) {
+  auto labels = Labels{{"le", "test"}};
+  EXPECT_ANY_THROW(std::make_unique<Family<Histogram>>("name", "help", labels));
+}
+
+TEST(FamilyTest, reject_histogram_with_le_label) {
+  Family<Histogram> family{"name", "help", {}};
+  auto labels = Labels{{"le", "test"}};
+  EXPECT_ANY_THROW(family.Add(labels, Histogram::BucketBoundaries{0, 1, 2}));
+}
+
+TEST(FamilyTest, reject_summary_with_constant_quantile_label) {
+  auto labels = Labels{{"quantile", "test"}};
+  EXPECT_ANY_THROW(std::make_unique<Family<Summary>>("name", "help", labels));
+}
+
+TEST(FamilyTest, reject_summary_with_quantile_label) {
+  Family<Summary> family{"name", "help", {}};
+  auto labels = Labels{{"quantile", "test"}};
+  auto quantiles = Summary::Quantiles{{0.5, 0.05}};
+  EXPECT_ANY_THROW(family.Add(labels, quantiles));
+}
+
 }  // namespace
 }  // namespace prometheus