From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 24 Feb 2016 15:46:14 +0000 (+0100)
Subject: dnsdist: Make PoolAction() stop the rule processing again
X-Git-Tag: rec-4.0.0-alpha2~61^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=88d05ca17e02ffc6a77c840f5e2cb681f799e4da;p=pdns

dnsdist: Make PoolAction() stop the rule processing again

It could clearly be confusing, and can easily be done by
ordering rules correctly anyway.
Thanks @rygl for the very valuable feedback.
---

diff --git a/pdns/README-dnsdist.md b/pdns/README-dnsdist.md
index 8c088ed17..40fd5e6fb 100644
--- a/pdns/README-dnsdist.md
+++ b/pdns/README-dnsdist.md
@@ -356,7 +356,6 @@ Some specific actions do not stop the processing when they match, contrary to al
  * Log
  * MacAddr
  * No Recurse
- * Route to a pool
  * and of course None
 
 A convenience function `makeRule()` is supplied which will make a NetmaskGroupRule for you or a SuffixMatchNodeRule
@@ -454,7 +453,7 @@ Valid return values for `LuaAction` functions are:
  * DNSAction.HeaderModify: indicate that the query has been turned into a response
  * DNSAction.None: continue to the next rule
  * DNSAction.Nxdomain: return a response with a NXDomain rcode
- * DNSAction.Pool: use the specified pool to forward this query, continue to the next rule
+ * DNSAction.Pool: use the specified pool to forward this query
  * DNSAction.Spoof: spoof the response using the supplied IPv4 (A), IPv6 (AAAA) or string (CNAME) value
 
 DNSSEC
diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index e2014152f..cce365842 100644
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -297,10 +297,11 @@ void* tcpClientThread(int pipefd)
             case DNSAction::Action::HeaderModify:
               done = true;
               break;
-            /* non-terminal actions follow */
             case DNSAction::Action::Pool:
               poolname=ruleresult;
+              done = true;
               break;
+            /* non-terminal actions follow */
             case DNSAction::Action::Delay:
             case DNSAction::Action::None:
               break;
diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 9bd5d2de1..0febcb497 100644
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -752,10 +752,11 @@ try
           case DNSAction::Action::HeaderModify:
             done = true;
             break;
-          /* non-terminal actions follow */
           case DNSAction::Action::Pool:
             poolname=ruleresult;
+            done = true;
             break;
+          /* non-terminal actions follow */
           case DNSAction::Action::Delay:
             delayMsec = static_cast<int>(pdns_stou(ruleresult)); // sorry
             break;
diff --git a/regression-tests.dnsdist/test_Advanced.py b/regression-tests.dnsdist/test_Advanced.py
index 754fcdb43..7fccadc36 100644
--- a/regression-tests.dnsdist/test_Advanced.py
+++ b/regression-tests.dnsdist/test_Advanced.py
@@ -758,3 +758,45 @@ class TestAdvancedQClass(DNSDistTest):
         receivedQuery.id = query.id
         self.assertEquals(query, receivedQuery)
         self.assertEquals(response, receivedResponse)
+
+class TestAdvancedNonTerminalRule(DNSDistTest):
+
+    _config_template = """
+    newServer{address="127.0.0.1:%s", pool="real"}
+    addAction(AllRule(), DisableValidationAction())
+    addAction(AllRule(), PoolAction("real"))
+    addAction(AllRule(), DropAction())
+    """
+    def testAdvancedNonTerminalRules(self):
+        """
+        Advanced: Non terminal rules
+
+        We check that DisableValidationAction() is applied
+        but does not stop the processing, then that
+        PoolAction() is applied _and_ stop the processing.
+        """
+        name = 'nonterminal.advanced.tests.powerdns.com.'
+        query = dns.message.make_query(name, 'A', 'IN')
+        expectedQuery = dns.message.make_query(name, 'A', 'IN')
+        expectedQuery.flags |= dns.flags.CD
+        response = dns.message.make_response(query)
+        rrset = dns.rrset.from_text(name,
+                                    3600,
+                                    dns.rdataclass.IN,
+                                    dns.rdatatype.A,
+                                    '192.2.0.1')
+        response.answer.append(rrset)
+
+        (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response)
+        self.assertTrue(receivedQuery)
+        self.assertTrue(receivedResponse)
+        receivedQuery.id = expectedQuery.id
+        self.assertEquals(expectedQuery, receivedQuery)
+        self.assertEquals(response, receivedResponse)
+
+        (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response)
+        self.assertTrue(receivedQuery)
+        self.assertTrue(receivedResponse)
+        receivedQuery.id = expectedQuery.id
+        self.assertEquals(expectedQuery, receivedQuery)
+        self.assertEquals(response, receivedResponse)