dnsdist: Use repeated string values for MetaValue protobuf field

diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc
index 175f365c423ed3fabc35b843c2419a66b40e815d..c4d8e9c87ab6fe5148660ab91b7047654009f8a9 100644 (file)
--- a/pdns/dnsdist-lua-actions.cc
+++ b/pdns/dnsdist-lua-actions.cc
@@ -1485,7 +1485,7 @@ private:
 static void addMetaDataToProtobuf(DNSDistProtoBufMessage& message, const DNSQuestion& dq, const std::vector<std::pair<std::string, ProtoBufMetaKey>>& metas)
 {
   for (const auto& [name, meta] : metas) {
-    message.addMeta(name, meta.getValue(dq));
+    message.addMeta(name, meta.getValues(dq));
   }
 }
 

diff --git a/pdns/dnsdist-protobuf.cc b/pdns/dnsdist-protobuf.cc
index b20437bc61e7e020e657eb6027c6723cd74b12bf..8bfd704f97aa42b66bfbe5dc2741c832b237c0fe 100644 (file)
--- a/pdns/dnsdist-protobuf.cc
+++ b/pdns/dnsdist-protobuf.cc
@@ -104,9 +104,12 @@ void DNSDistProtoBufMessage::addTag(const std::string& strValue)
   d_additionalTags.push_back(strValue);
 }
 
-void DNSDistProtoBufMessage::addMeta(const std::string& key, std::string&& value)
+void DNSDistProtoBufMessage::addMeta(const std::string& key, std::vector<std::string>&& values)
 {
-  d_metaTags.push_back({key, std::move(value)});
+  auto& entry = d_metaTags[key];
+  for (auto& value : values) {
+    entry.insert(std::move(value));
+  }
 }
 
 void DNSDistProtoBufMessage::addRR(DNSName&& qname, uint16_t uType, uint16_t uClass, uint32_t uTTL, const std::string& strBlob)
@@ -193,8 +196,8 @@ void DNSDistProtoBufMessage::serialize(std::string& data) const
 
   m.commitResponse();
 
-  for (const auto& [key, value] : d_metaTags) {
-    m.setMeta(key, {value}, {});
+  for (const auto& [key, values] : d_metaTags) {
+    m.setMeta(key, values, {});
   }
 }
 
@@ -233,12 +236,12 @@ ProtoBufMetaKey::ProtoBufMetaKey(const std::string& key)
   throw std::runtime_error("Invalid ProtoBuf key '" + key + "'");
 }
 
-std::string ProtoBufMetaKey::getValue(const DNSQuestion& dq) const
+std::vector<std::string> ProtoBufMetaKey::getValues(const DNSQuestion& dq) const
 {
   auto& idx = s_types.get<TypeTag>();
   auto it = idx.find(d_type);
   if (it == idx.end()) {
-    throw std::runtime_error("Trying to get the value of an unsupported type: " + std::to_string(static_cast<uint8_t>(d_type)));
+    throw std::runtime_error("Trying to get the values of an unsupported type: " + std::to_string(static_cast<uint8_t>(d_type)));
   }
   return (it->d_func)(dq, d_subKey, d_numericSubKey);
 }
@@ -254,69 +257,83 @@ const std::string& ProtoBufMetaKey::getName() const
 }
 
 const ProtoBufMetaKey::TypeContainer ProtoBufMetaKey::s_types = {
-  ProtoBufMetaKey::KeyTypeDescription{ "sni", Type::SNI, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::string { return dq.sni; }, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "pool", Type::Pool, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::string { return dq.ids.poolName; }, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "b64-content", Type::B64Content, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::string { const auto& data = dq.getData(); return Base64Encode(std::string(data.begin(), data.end())); }, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "doh-header", Type::DoHHeader, [](const DNSQuestion& dq , const std::string& name, uint8_t) {
+  ProtoBufMetaKey::KeyTypeDescription{ "sni", Type::SNI, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> { return {dq.sni}; }, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "pool", Type::Pool, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> { return {dq.ids.poolName}; }, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "b64-content", Type::B64Content, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> { const auto& data = dq.getData(); return {Base64Encode(std::string(data.begin(), data.end()))}; }, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "doh-header", Type::DoHHeader, [](const DNSQuestion& dq , const std::string& name, uint8_t) -> std::vector<std::string> {
     if (!dq.ids.du) {
-      return std::string();
+      return {};
     }
     auto headers = dq.ids.du->getHTTPHeaders();
     auto it = headers.find(name);
     if (it != headers.end()) {
-      return it->second;
+      return {it->second};
+    }
+    return {};
+  }, true, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "doh-host", Type::DoHHost, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    if (dq.ids.du) {
+      return {dq.ids.du->getHTTPHost()};
     }
-    return std::string();
+    return {};
   }, true, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "doh-host", Type::DoHHost, [](const DNSQuestion& dq, const std::string&, uint8_t) { return (dq.ids.du ? dq.ids.du->getHTTPHost() : ""); }, true, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "doh-path", Type::DoHPath, [](const DNSQuestion& dq, const std::string&, uint8_t) { return (dq.ids.du ? dq.ids.du->getHTTPPath() : ""); }, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "doh-query-string", Type::DoHQueryString, [](const DNSQuestion& dq, const std::string&, uint8_t) { return (dq.ids.du ? dq.ids.du->getHTTPQueryString() : ""); }, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "doh-scheme", Type::DoHScheme, [](const DNSQuestion& dq, const std::string&, uint8_t) { return (dq.ids.du ? dq.ids.du->getHTTPScheme() : ""); }, false, false },
-  ProtoBufMetaKey::KeyTypeDescription{ "proxy-protocol-value", Type::ProxyProtocolValue, [](const DNSQuestion& dq, const std::string&, uint8_t numericSubKey) {
+  ProtoBufMetaKey::KeyTypeDescription{ "doh-path", Type::DoHPath, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    if (dq.ids.du) {
+      return {dq.ids.du->getHTTPPath()};
+    }
+    return {};
+    }, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "doh-query-string", Type::DoHQueryString, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    if (dq.ids.du) {
+      return {dq.ids.du->getHTTPQueryString()};
+    }
+    return {};
+    }, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "doh-scheme", Type::DoHScheme, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    if (dq.ids.du) {
+      return {dq.ids.du->getHTTPScheme()};
+    }
+    return {};
+    }, false, false },
+  ProtoBufMetaKey::KeyTypeDescription{ "proxy-protocol-value", Type::ProxyProtocolValue, [](const DNSQuestion& dq, const std::string&, uint8_t numericSubKey) -> std::vector<std::string> {
     if (!dq.proxyProtocolValues) {
-      return std::string();
+      return {};
     }
     for (const auto& value : *dq.proxyProtocolValues) {
       if (value.type == numericSubKey) {
-        return value.content;
+        return {value.content};
       }
     }
-    return std::string();
+    return {};
   }, true, false, true },
-  ProtoBufMetaKey::KeyTypeDescription{ "proxy-protocol-values", Type::ProxyProtocolValues, [](const DNSQuestion& dq, const std::string&, uint8_t) {
+  ProtoBufMetaKey::KeyTypeDescription{ "proxy-protocol-values", Type::ProxyProtocolValues, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    std::vector<std::string> result;
     if (!dq.proxyProtocolValues) {
-      return std::string();
+      return result;
     }
-    std::string result;
     for (const auto& value : *dq.proxyProtocolValues) {
-      if (!result.empty()) {
-        result += ", ";
-      }
-      result += std::to_string(value.type) + ":" + value.content;
+      result.push_back(std::to_string(value.type) + ":" + value.content);
     }
     return result;
   } },
-  ProtoBufMetaKey::KeyTypeDescription{ "tag", Type::Tag, [](const DNSQuestion& dq, const std::string& subKey, uint8_t) {
+  ProtoBufMetaKey::KeyTypeDescription{ "tag", Type::Tag, [](const DNSQuestion& dq, const std::string& subKey, uint8_t) -> std::vector<std::string> {
     if (!dq.ids.qTag) {
-      return std::string();
+      return {};
     }
     for (const auto& [key, value] : *dq.ids.qTag) {
       if (key == subKey) {
-        return value;
+        return {value};
       }
     }
-    return std::string();
+    return {};
   }, true, true },
-  ProtoBufMetaKey::KeyTypeDescription{ "tags", Type::Tags, [](const DNSQuestion& dq, const std::string&, uint8_t) {
+  ProtoBufMetaKey::KeyTypeDescription{ "tags", Type::Tags, [](const DNSQuestion& dq, const std::string&, uint8_t) -> std::vector<std::string> {
+    std::vector<std::string> result;
     if (!dq.ids.qTag) {
-      return std::string();
+      return result;
     }
-    std::string result;
     for (const auto& [key, value] : *dq.ids.qTag) {
-      if (!result.empty()) {
-        result += ", ";
-      }
-      result += key + ":" + value;
+      result.push_back(key + ":" + value);
     }
     return result;
   } },

diff --git a/pdns/dnsdist-protobuf.hh b/pdns/dnsdist-protobuf.hh
index 1133f231f35d49b4fdae9a790c975ba346aef383..39305383d3968f0d01e550f1c688e79eacc6da03 100644 (file)
--- a/pdns/dnsdist-protobuf.hh
+++ b/pdns/dnsdist-protobuf.hh
@@ -47,7 +47,7 @@ public:
   void setEDNSSubnet(const Netmask& nm);
 
   void addTag(const std::string& strValue);
-  void addMeta(const std::string& key, std::string&& value);
+  void addMeta(const std::string& key, std::vector<std::string>&& values);
   void addRR(DNSName&& qname, uint16_t uType, uint16_t uClass, uint32_t uTTL, const std::string& data);
 
   void serialize(std::string& data) const;
@@ -76,7 +76,7 @@ private:
 
   std::vector<PBRecord> d_additionalRRs;
   std::vector<std::string> d_additionalTags;
-  std::vector<std::pair<std::string, std::string>> d_metaTags;
+  std::unordered_map<std::string, std::unordered_set<std::string>> d_metaTags;
 
   const DNSQuestion& d_dq;
   const DNSResponse* d_dr{nullptr};
@@ -104,7 +104,7 @@ class ProtoBufMetaKey
   {
     const std::string d_name;
     const Type d_type;
-    const std::function<std::string(const DNSQuestion&, const std::string&, uint8_t)> d_func;
+    const std::function<std::vector<std::string>(const DNSQuestion&, const std::string&, uint8_t)> d_func;
     bool d_prefix{false};
     bool d_caseSensitive{true};
     bool d_numeric{false};
@@ -127,7 +127,7 @@ public:
   ProtoBufMetaKey(const std::string& key);
 
   const std::string& getName() const;
-  std::string getValue(const DNSQuestion& dq) const;
+  std::vector<std::string> getValues(const DNSQuestion& dq) const;
 private:
   std::string d_subKey;
   uint8_t d_numericSubKey{0};

diff --git a/pdns/dnsdistdist/docs/rules-actions.rst b/pdns/dnsdistdist/docs/rules-actions.rst
index 45817741c3cbd8acf03d4602f27b5460d4297dd2..068c5ee1f40e68cfc8f02661d7743c0eaa60a756 100644 (file)
--- a/pdns/dnsdistdist/docs/rules-actions.rst
+++ b/pdns/dnsdistdist/docs/rules-actions.rst
@@ -1336,11 +1336,11 @@ The following actions exist.
   * ``doh-scheme``: the HTTP scheme for DoH queries, empty otherwise
   * ``pool``: the currently selected pool of servers
   * ``proxy-protocol-value:<TYPE>``: the content of the proxy protocol value of type ``<TYPE>``, if any
-  * ``proxy-protocol-values``: the content of all proxy protocol values as a "<type1>:<value1>,...,<typeN>:<valueN>" string
+  * ``proxy-protocol-values``: the content of all proxy protocol values as a "<type1>:<value1>", ..., "<typeN>:<valueN>" strings
   * ``b64-content``: the base64-encoded DNS payload of the current query
   * ``sni``: the Server Name Indication value for queries received over DoT or DoH. Empty otherwise.
   * ``tag:<TAG>``: the content of the corresponding ``<TAG>`` if any
-  * ``tags``: the list of all tags, and their values, as a "<key1>:<value1>,...,<keyN>:<valueN>" string
+  * ``tags``: the list of all tags, and their values, as a "<key1>:<value1>", ..., "<keyN>:<valueN>" strings
 
   Subsequent rules are processed after this action.
 

diff --git a/regression-tests.dnsdist/test_Protobuf.py b/regression-tests.dnsdist/test_Protobuf.py
index 31dfc085daefd8d3ec4fed8cf971d63f6981a4b3..b409f8d97e17db60889c1acf562b9e7f1088e83a 100644 (file)
--- a/regression-tests.dnsdist/test_Protobuf.py
+++ b/regression-tests.dnsdist/test_Protobuf.py
@@ -452,18 +452,25 @@ class TestProtobufMetaTags(DNSDistProtobufTest):
 
         self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)
         self.assertEqual(len(msg.meta), 2)
-        self.assertEqual(msg.meta[0].key, 'b64')
+        tags = {}
+        for entry in msg.meta:
+            tags[entry.key] = entry.value.stringVal
+
+        self.assertIn('b64', tags)
+        self.assertIn('my-tag-export-name', tags)
+
         b64EncodedQuery = base64.b64encode(query.to_wire()).decode('ascii')
-        self.assertEqual(msg.meta[0].value.stringVal[0], b64EncodedQuery)
-        self.assertEqual(msg.meta[1].key, 'my-tag-export-name')
-        self.assertEqual(msg.meta[1].value.stringVal[0], 'my-tag-value')
+        self.assertEqual(tags['b64'], [b64EncodedQuery])
+        self.assertEqual(tags['my-tag-export-name'], ['my-tag-value'])
 
         # check the protobuf message corresponding to the UDP response
         msg = self.getFirstProtobufMessage()
         self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, response)
         self.assertEqual(len(msg.meta), 1)
         self.assertEqual(msg.meta[0].key, 'my-tag-export-name')
-        self.assertEqual(msg.meta[0].value.stringVal[0], 'my-tag-key2:my-tag-value2, my-tag-key:my-tag-value')
+        self.assertEqual(len(msg.meta[0].value.stringVal), 2)
+        self.assertIn('my-tag-key:my-tag-value', msg.meta[0].value.stringVal)
+        self.assertIn('my-tag-key2:my-tag-value2', msg.meta[0].value.stringVal)
 
 class TestProtobufMetaDOH(DNSDistProtobufTest):
 
@@ -602,13 +609,14 @@ class TestProtobufMetaProxy(DNSDistProtobufTest):
         self.assertEqual(len(msg.meta), 2)
         tags = {}
         for entry in msg.meta:
-            self.assertEqual(len(entry.value.stringVal), 1)
-            tags[entry.key] = entry.value.stringVal[0]
+            tags[entry.key] = entry.value.stringVal
 
         self.assertIn('pp42', tags)
-        self.assertEqual(tags['pp42'], 'proxy')
+        self.assertEqual(tags['pp42'], ['proxy'])
         self.assertIn('pp', tags)
-        self.assertEqual(tags['pp'], '2:foo, 42:proxy')
+        self.assertEqual(len(tags['pp']), 2)
+        self.assertIn('2:foo', tags['pp'])
+        self.assertIn('42:proxy', tags['pp'])
 
 class TestProtobufIPCipher(DNSDistProtobufTest):
     _config_params = ['_testServerPort', '_protobufServerPort', '_protobufServerID', '_protobufServerID']