From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 1 Apr 2019 09:30:06 +0000 (+0200)
Subject: Only apply "do not cache" if both limits are set and satisfied.  Doc tweaks.
X-Git-Tag: dnsdist-1.4.0-alpha1~37^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=42f418044c137ae2835f7f6550da093cb8b6ce7c;p=thirdparty%2Fpdns.git

Only apply "do not cache" if both limits are set and satisfied.  Doc tweaks.
---

diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst
index 21a698a019..ceb92bcf05 100644
--- a/pdns/recursordist/docs/settings.rst
+++ b/pdns/recursordist/docs/settings.rst
@@ -386,7 +386,8 @@ Number of bits of client IPv4 address to pass when sending EDNS Client Subnet ad
 -  Integer
 -  Default: 24
 
-Maximum number of bits of client IPv4 address used by the authoritative server (as indicated by the EDNS Client Subnet scope in the answer) for an answer to be inserted into the query cache.
+Maximum number of bits of client IPv4 address used by the authoritative server (as indicated by the EDNS Client Subnet scope in the answer) for an answer to be inserted into the query cache. This condition applies in conjunction with ``ecs-cache-limit-ttl``.
+That is, only if both the limits apply, the record will not be cached.
 
 .. _setting-ecs-ipv6-bits:
 
@@ -408,7 +409,8 @@ Number of bits of client IPv6 address to pass when sending EDNS Client Subnet ad
 -  Integer
 -  Default: 56
 
-Maximum number of bits of client IPv6 address used by the authoritative server (as indicated by the EDNS Client Subnet scope in the answer) for an answer to be inserted into the query cache.
+Maximum number of bits of client IPv6 address used by the authoritative server (as indicated by the EDNS Client Subnet scope in the answer) for an answer to be inserted into the query cache. This condition applies in conjunction with ``ecs-cache-limit-ttl``.
+That is, only if both the limits apply, the record will not be cached.
 
 .. _setting-ecs-minimum-ttl-override:
 
@@ -430,7 +432,8 @@ Can be set at runtime using ``rec_control set-ecs-minimum-ttl 3600``.
 -  Integer
 -  Default: 0 (disabled)
 
-The minumum TTL for an ECS-specific answer to be inserted into the query cache.
+The minumum TTL for an ECS-specific answer to be inserted into the query cache. This condition applies in conjunction with ``ecs-ipv4-cache-bits`` or ``ecs-ipv6-cache-bits``.
+That is, only if both the limits apply, the record will not be cached.
 
 .. _setting-ecs-scope-zero-address:
 
diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index bfeac25455..52ca667af6 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -2143,7 +2143,7 @@ BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_allowed) {
   BOOST_REQUIRE_EQUAL(cached.size(), 1);
 }
 
-BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_denied) {
+BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_no_ttl_limit_allowed) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
 
@@ -2175,11 +2175,11 @@ BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_denied) {
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(ret.size(), 1);
 
-  /* should have NOT been cached because /24 is more specific than /16 */
+  /* should have been cached because /24 is more specific than /16 but TTL limit is nof efective */
   const ComboAddress who("192.0.2.128");
   vector<DNSRecord> cached;
-  BOOST_REQUIRE_LT(t_RC->get(now, target, QType(QType::A), true, &cached, who), 0);
-  BOOST_REQUIRE_EQUAL(cached.size(), 0);
+  BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::A), true, &cached, who), 0);
+  BOOST_REQUIRE_EQUAL(cached.size(), 1);
 }
 
 BOOST_AUTO_TEST_CASE(test_ecs_cache_ttllimit_allowed) {
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index cb7cbf4df2..3af79100d4 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2426,6 +2426,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       if (!doCache && ednsmask) {
         bool manyMaskBits = (ednsmask->isIpv4() && ednsmask->getBits() > SyncRes::s_ecsipv4cachelimit) ||
             (ednsmask->isIpv6() && ednsmask->getBits() > SyncRes::s_ecsipv6cachelimit);
+        doCache = true;
 
         if (SyncRes::s_ecscachelimitttl > 0) {
           if (manyMaskBits) {
@@ -2439,13 +2440,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
               // Case: many bits and ttlIsSmall
               doCache = false;
             }
-          } else {
-              // Case: few mask bits
-              doCache = true;
           }
-        } else {
-          // no applicable TTL limit, scope determines cacheability
-          doCache = !manyMaskBits;
         }
       }
       if (doCache) {