From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Wed, 12 Nov 2025 11:52:12 +0000 (+0100)
Subject: fix(dnsdist): use RAII to set InternalQueryState::appliedRulesToQuery
X-Git-Tag: rec-5.4.0-alpha1~72^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96b41d1797a44657fd94341ab3971770037ac624;p=thirdparty%2Fpdns.git

fix(dnsdist): use RAII to set InternalQueryState::appliedRulesToQuery
---

diff --git a/pdns/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc
index 6f9c11f634..b572463689 100644
--- a/pdns/dnsdistdist/dnsdist-idstate.cc
+++ b/pdns/dnsdistdist/dnsdist-idstate.cc
@@ -88,7 +88,7 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
   // getTracer returns a Tracer when tracing is globally enabled
   // tracingEnabled tells us whether or not tracing is enabled for this query
   // Should tracing be disabled, *but* we have not processed query rules, we will still return a closer if tracing is globally enabled
-  if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !tracingPastProcessRules)) {
+  if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) {
     ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(name, parentSpanID));
   }
 #endif
diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh
index a64597ed8d..59f2ffe517 100644
--- a/pdns/dnsdistdist/dnsdist-idstate.hh
+++ b/pdns/dnsdistdist/dnsdist-idstate.hh
@@ -235,7 +235,22 @@ public:
   bool cacheHit{false};
   bool staleCacheHit{false};
   bool tracingEnabled{false}; // Whether or not Open Telemetry tracing is enabled for this query
-  bool tracingPastProcessRules{false}; // Whether processRules has been called for the query, used to determine if we can trace
+  bool rulesAppliedToQuery{false}; // Whether applyRulesToQuery has been called for the query, used to determine if we need to trace
+  struct rulesAppliedToQuerySetter
+  {
+    rulesAppliedToQuerySetter(bool& pastProcessRules) :
+      d_rulesAppliedToQuery(pastProcessRules)
+    {
+      d_rulesAppliedToQuery = false;
+    }
+    ~rulesAppliedToQuerySetter()
+    {
+      d_rulesAppliedToQuery = true;
+    }
+
+  private:
+    bool& d_rulesAppliedToQuery;
+  };
 };
 
 struct IDState
diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 99d2164fa4..3e28057ce8 100644
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -1046,6 +1046,7 @@ static bool applyRulesChainToQuery(const std::vector<dnsdist::rules::RuleAction>
 
 static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
 {
+  InternalQueryState::rulesAppliedToQuerySetter tpprs(dnsQuestion.ids.rulesAppliedToQuery); // Ensure IDS knows we are past the rules processing when we exit this function
   auto closer = dnsQuestion.ids.getCloser("applyRulesToQuery");
   if (g_rings.shouldRecordQueries()) {
     g_rings.insertQuery(now, dnsQuestion.ids.origRemote, dnsQuestion.ids.qname, dnsQuestion.ids.qtype, dnsQuestion.getData().size(), *dnsQuestion.getHeader(), dnsQuestion.getProtocol());
@@ -1100,7 +1101,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
         updateBlockStats();
 
         setRCode(RCode::NXDomain);
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
 
       case DNSAction::Action::Refused:
@@ -1108,7 +1108,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
         updateBlockStats();
 
         setRCode(RCode::Refused);
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
 
       case DNSAction::Action::Truncate:
@@ -1123,7 +1122,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
             header.ad = false;
             return true;
           });
-          dnsQuestion.ids.tracingPastProcessRules = true;
           return true;
         }
         else {
@@ -1137,7 +1135,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
           header.rd = false;
           return true;
         });
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
       case DNSAction::Action::SetTag: {
         if (!got->second.tagSettings) {
@@ -1155,7 +1152,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
       default:
         updateBlockStats();
         vinfolog("Query from %s dropped because of dynamic block", dnsQuestion.ids.origRemote.toStringWithPort());
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return false;
       }
     }
@@ -1181,14 +1177,12 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
         updateBlockStats();
 
         setRCode(RCode::NXDomain);
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
       case DNSAction::Action::Refused:
         vinfolog("Query from %s for %s refused because of dynamic block", dnsQuestion.ids.origRemote.toStringWithPort(), dnsQuestion.ids.qname.toLogString());
         updateBlockStats();
 
         setRCode(RCode::Refused);
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
       case DNSAction::Action::Truncate:
         if (!dnsQuestion.overTCP()) {
@@ -1203,7 +1197,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
             header.ad = false;
             return true;
           });
-          dnsQuestion.ids.tracingPastProcessRules = true;
           return true;
         }
         else {
@@ -1217,7 +1210,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
           header.rd = false;
           return true;
         });
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return true;
       case DNSAction::Action::SetTag: {
         if (!got->tagSettings) {
@@ -1235,7 +1227,6 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
       default:
         updateBlockStats();
         vinfolog("Query from %s for %s dropped because of dynamic block", dnsQuestion.ids.origRemote.toStringWithPort(), dnsQuestion.ids.qname.toLogString());
-        dnsQuestion.ids.tracingPastProcessRules = true;
         return false;
       }
     }
@@ -1244,9 +1235,7 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
 
   const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains;
   const auto& queryRules = dnsdist::rules::getRuleChain(chains, dnsdist::rules::RuleChain::Rules);
-  auto ret = applyRulesChainToQuery(queryRules, dnsQuestion);
-  dnsQuestion.ids.tracingPastProcessRules = true;
-  return ret;
+  return applyRulesChainToQuery(queryRules, dnsQuestion);
 }
 
 ssize_t udpClientSendRequestToBackend(const std::shared_ptr<DownstreamState>& backend, const int socketDesc, const PacketBuffer& request, bool healthCheck)