From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 15 Dec 2022 09:24:39 +0000 (+0100)
Subject: dnsdist: Move DNSQuestion's EDNS options to a unique pointer
X-Git-Tag: dnsdist-1.8.0-rc1~134^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=582d5b58d6b05e81ef1c643baa510172ae2a78f5;p=thirdparty%2Fpdns.git

dnsdist: Move DNSQuestion's EDNS options to a unique pointer
---

diff --git a/pdns/dnsdist-ecs.cc b/pdns/dnsdist-ecs.cc
index af3240a63f..9ef0681413 100644
--- a/pdns/dnsdist-ecs.cc
+++ b/pdns/dnsdist-ecs.cc
@@ -263,7 +263,7 @@ bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket,
   return true;
 }
 
-static bool slowParseEDNSOptions(const PacketBuffer& packet, std::shared_ptr<std::map<uint16_t, EDNSOptionView> >& options)
+static bool slowParseEDNSOptions(const PacketBuffer& packet, std::map<uint16_t, EDNSOptionView>& options)
 {
   if (packet.size() < sizeof(dnsheader)) {
     return false;
@@ -304,7 +304,7 @@ static bool slowParseEDNSOptions(const PacketBuffer& packet, std::shared_ptr<std
         }
         /* if we survive this call, we can parse it safely */
         dpm.skipRData();
-        return getEDNSOptions(reinterpret_cast<const char*>(&packet.at(offset)), packet.size() - offset, *options) == 0;
+        return getEDNSOptions(reinterpret_cast<const char*>(&packet.at(offset)), packet.size() - offset, options) == 0;
       }
       else {
         dpm.skipRData();
@@ -515,7 +515,7 @@ bool parseEDNSOptions(const DNSQuestion& dq)
     return true;
   }
 
-  dq.ednsOptions = std::make_shared<std::map<uint16_t, EDNSOptionView> >();
+  dq.ednsOptions = std::make_unique<std::map<uint16_t, EDNSOptionView> >();
 
   if (ntohs(dh->arcount) == 0) {
     /* nothing in additional so no EDNS */
@@ -523,7 +523,7 @@ bool parseEDNSOptions(const DNSQuestion& dq)
   }
 
   if (ntohs(dh->ancount) != 0 || ntohs(dh->nscount) != 0 || ntohs(dh->arcount) > 1) {
-    return slowParseEDNSOptions(dq.getData(), dq.ednsOptions);
+    return slowParseEDNSOptions(dq.getData(), *dq.ednsOptions);
   }
 
   size_t remaining = 0;
@@ -1051,12 +1051,11 @@ bool queryHasEDNS(const DNSQuestion& dq)
   return false;
 }
 
-bool getEDNS0Record(const DNSQuestion& dq, EDNS0Record& edns0)
+bool getEDNS0Record(const PacketBuffer& packet, EDNS0Record& edns0)
 {
   uint16_t optStart;
   size_t optLen = 0;
   bool last = false;
-  const auto& packet = dq.getData();
   int res = locateEDNSOptRR(packet, &optStart, &optLen, &last);
   if (res != 0) {
     // no EDNS OPT RR
diff --git a/pdns/dnsdist-ecs.hh b/pdns/dnsdist-ecs.hh
index 043756a3fb..f5dbc56c25 100644
--- a/pdns/dnsdist-ecs.hh
+++ b/pdns/dnsdist-ecs.hh
@@ -54,6 +54,6 @@ bool parseEDNSOptions(const DNSQuestion& dq);
 
 int getEDNSZ(const DNSQuestion& dq);
 bool queryHasEDNS(const DNSQuestion& dq);
-bool getEDNS0Record(const DNSQuestion& dq, EDNS0Record& edns0);
+bool getEDNS0Record(const PacketBuffer& packet, EDNS0Record& edns0);
 
 bool setEDNSOption(DNSQuestion& dq, uint16_t ednsCode, const std::string& data);
diff --git a/pdns/dnsdist-idstate.hh b/pdns/dnsdist-idstate.hh
index 944fed5c69..2da6be6472 100644
--- a/pdns/dnsdist-idstate.hh
+++ b/pdns/dnsdist-idstate.hh
@@ -122,7 +122,7 @@ struct InternalQueryState
   std::shared_ptr<DNSDistPacketCache> packetCache{nullptr}; // 16
   std::unique_ptr<DNSCryptQuery> dnsCryptQuery{nullptr}; // 8
   std::unique_ptr<QTag> qTag{nullptr}; // 8
-  boost::optional<uint32_t> tempFailureTTL; // 8
+  boost::optional<uint32_t> tempFailureTTL{boost::none}; // 8
   ClientState* cs{nullptr}; // 8
   std::unique_ptr<DOHUnit, void (*)(DOHUnit*)> du; // 8
   uint32_t cacheKey{0}; // 4
diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index f7111d131e..f7dbac5e90 100644
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -146,7 +146,7 @@ public:
   InternalQueryState& ids;
   std::unique_ptr<Netmask> ecs{nullptr};
   std::string sni; /* Server Name Indication, if any (DoT or DoH) */
-  mutable std::shared_ptr<std::map<uint16_t, EDNSOptionView> > ednsOptions;
+  mutable std::unique_ptr<std::map<uint16_t, EDNSOptionView> > ednsOptions; /* this needs to be mutable because it is parsed just in time, when DNSQuestion is read-only */
   std::unique_ptr<std::vector<ProxyProtocolValue>> proxyProtocolValues{nullptr};
   uint16_t ecsPrefixLength;
   uint8_t ednsRCode{0};
diff --git a/pdns/dnsdistdist/dnsdist-rules.hh b/pdns/dnsdistdist/dnsdist-rules.hh
index fdf9edc01f..13ffe52318 100644
--- a/pdns/dnsdistdist/dnsdist-rules.hh
+++ b/pdns/dnsdistdist/dnsdist-rules.hh
@@ -924,7 +924,7 @@ public:
     }
 
     EDNS0Record edns0;
-    if (!getEDNS0Record(*dq, edns0)) {
+    if (!getEDNS0Record(dq->getData(), edns0)) {
       return false;
     }
 
@@ -948,7 +948,7 @@ public:
   bool matches(const DNSQuestion* dq) const override
   {
     EDNS0Record edns0;
-    if (!getEDNS0Record(*dq, edns0)) {
+    if (!getEDNS0Record(dq->getData(), edns0)) {
       return false;
     }
 
diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc
index be7a43d3e6..71e140b99c 100644
--- a/pdns/test-dnsdist_cc.cc
+++ b/pdns/test-dnsdist_cc.cc
@@ -2215,7 +2215,7 @@ BOOST_AUTO_TEST_CASE(test_setEDNSOption)
   BOOST_CHECK_EQUAL(mdp.d_answers.at(0).first.d_name, g_rootdnsname);
 
   EDNS0Record edns0;
-  BOOST_REQUIRE(getEDNS0Record(dq, edns0));
+  BOOST_REQUIRE(getEDNS0Record(dq.getData(), edns0));
   BOOST_CHECK_EQUAL(edns0.version, 0U);
   BOOST_CHECK_EQUAL(edns0.extRCode, 0U);
   BOOST_CHECK_EQUAL(edns0.extFlags, EDNS_HEADER_FLAG_DO);