From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 14 Feb 2023 11:00:10 +0000 (+0100)
Subject: dnsdist: No trailing ':' after the protobuf tag key when the value is empty
X-Git-Tag: dnsdist-1.8.0-rc1~15^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b9302bd3b6d1a20d156ea208730744725a74aa8;p=thirdparty%2Fpdns.git

dnsdist: No trailing ':' after the protobuf tag key when the value is empty
---

diff --git a/pdns/dnsdist-protobuf.cc b/pdns/dnsdist-protobuf.cc
index 8bfd704f97..c1250ba154 100644
--- a/pdns/dnsdist-protobuf.cc
+++ b/pdns/dnsdist-protobuf.cc
@@ -333,7 +333,13 @@ const ProtoBufMetaKey::TypeContainer ProtoBufMetaKey::s_types = {
       return result;
     }
     for (const auto& [key, value] : *dq.ids.qTag) {
-      result.push_back(key + ":" + value);
+      if (value.empty()) {
+        /* avoids a spurious ':' when the value is empty */
+        result.push_back(key);
+      }
+      else {
+        result.push_back(key + ":" + value);
+      }
     }
     return result;
   } },
diff --git a/pdns/dnsdistdist/docs/rules-actions.rst b/pdns/dnsdistdist/docs/rules-actions.rst
index 068c5ee1f4..8db6d1cb36 100644
--- a/pdns/dnsdistdist/docs/rules-actions.rst
+++ b/pdns/dnsdistdist/docs/rules-actions.rst
@@ -1340,7 +1340,7 @@ The following actions exist.
   * ``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>" strings
+  * ``tags``: the list of all tags, and their values, as a "<key1>:<value1>", ..., "<keyN>:<valueN>" strings. Note that a tag with an empty value will be exported as "<key>", not "<key>:".
 
   Subsequent rules are processed after this action.
 
diff --git a/regression-tests.dnsdist/test_Protobuf.py b/regression-tests.dnsdist/test_Protobuf.py
index b409f8d97e..125788915b 100644
--- a/regression-tests.dnsdist/test_Protobuf.py
+++ b/regression-tests.dnsdist/test_Protobuf.py
@@ -418,6 +418,7 @@ class TestProtobufMetaTags(DNSDistProtobufTest):
     rl = newRemoteLogger('127.0.0.1:%d')
 
     addAction(AllRule(), SetTagAction('my-tag-key', 'my-tag-value'))
+    addAction(AllRule(), SetTagAction('my-empty-key', ''))
     addAction(AllRule(), RemoteLogAction(rl, nil, {serverID='dnsdist-server-1'}, {b64='b64-content', ['my-tag-export-name']='tag:my-tag-key'}))
     addResponseAction(AllRule(), SetTagResponseAction('my-tag-key2', 'my-tag-value2'))
     addResponseAction(AllRule(), RemoteLogResponseAction(rl, nil, false, {serverID='dnsdist-server-1'}, {['my-tag-export-name']='tags'}))
@@ -468,9 +469,11 @@ class TestProtobufMetaTags(DNSDistProtobufTest):
         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(len(msg.meta[0].value.stringVal), 2)
+        self.assertEqual(len(msg.meta[0].value.stringVal), 3)
         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)
+        # no ':' when the value is empty
+        self.assertIn('my-empty-key', msg.meta[0].value.stringVal)
 
 class TestProtobufMetaDOH(DNSDistProtobufTest):