From 6abbe231e5a587940d7cc64de4348ced05b70828 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Wed, 9 Nov 2016 10:01:26 +0000
Subject: [PATCH] middleware/cache: cache 0 will be capped at 5 (#408)

* middleware/cache: cache 0 will be capped at 5

cache 0 would return TTL=0 records, up that to the documented minimum of
5 seconds.

* middleware/cache: check for 0 TTL

Handle 0 TTL differently and return an error, we might need to
special case this in the future.
---
 middleware/cache/README.md     |  2 +-
 middleware/cache/cache.go      |  3 ++-
 middleware/cache/item.go       | 10 ++++++----
 middleware/cache/setup.go      | 13 +++++++++++++
 middleware/cache/setup_test.go |  5 +++++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/middleware/cache/README.md b/middleware/cache/README.md
index 0f47acb77..352d2101b 100644
--- a/middleware/cache/README.md
+++ b/middleware/cache/README.md
@@ -14,7 +14,7 @@ cache [TTL] [ZONES...]
 
 Each element in the cache is cached according to its TTL (with **TTL** as the max).
 For the negative cache, the SOA's MinTTL value is used. A cache can contain up to 10,000 items by
-default.
+default. A TTL of zero is not allowed.
 
 If you want more control:
 
diff --git a/middleware/cache/cache.go b/middleware/cache/cache.go
index a1db2039e..758c298aa 100644
--- a/middleware/cache/cache.go
+++ b/middleware/cache/cache.go
@@ -125,7 +125,8 @@ func (c *ResponseWriter) Write(buf []byte) (int, error) {
 const (
 	maxTTL  = 1 * time.Hour
 	maxNTTL = 30 * time.Minute
-	minTTL  = 5 * time.Second
+
+	minTTL = 5 // seconds
 
 	defaultCap = 10000 // default capacity of the cache.
 
diff --git a/middleware/cache/item.go b/middleware/cache/item.go
index f2d05f0a0..0af3b67ef 100644
--- a/middleware/cache/item.go
+++ b/middleware/cache/item.go
@@ -63,9 +63,6 @@ func (i *item) toMsg(m *dns.Msg) *dns.Msg {
 	m1.Extra = i.Extra
 
 	ttl := int(i.origTTL) - int(time.Now().UTC().Sub(i.stored).Seconds())
-	if ttl < int(minTTL.Seconds()) {
-		ttl = int(minTTL.Seconds())
-	}
 	setMsgTTL(m1, uint32(ttl))
 	return m1
 }
@@ -75,8 +72,13 @@ func (i *item) expired(now time.Time) bool {
 	return ttl < 0
 }
 
-// setMsgTTL sets the ttl on all RRs in all sections.
+// setMsgTTL sets the ttl on all RRs in all sections. If ttl is smaller than minTTL
+// that value is used.
 func setMsgTTL(m *dns.Msg, ttl uint32) {
+	if ttl < minTTL {
+		ttl = minTTL
+	}
+
 	for _, r := range m.Answer {
 		r.Header().Ttl = ttl
 	}
diff --git a/middleware/cache/setup.go b/middleware/cache/setup.go
index 11a35ddc4..337930fe4 100644
--- a/middleware/cache/setup.go
+++ b/middleware/cache/setup.go
@@ -1,6 +1,7 @@
 package cache
 
 import (
+	"fmt"
 	"strconv"
 	"time"
 
@@ -49,6 +50,10 @@ func cacheParse(c *caddy.Controller) (*Cache, error) {
 			// first args may be just a number, then it is the ttl, if not it is a zone
 			ttl, err := strconv.Atoi(args[0])
 			if err == nil {
+				// Reserve 0 (and smaller for future things)
+				if ttl <= 0 {
+					return nil, fmt.Errorf("cache TTL can not be zero or negative: %d", ttl)
+				}
 				ca.pttl = time.Duration(ttl) * time.Second
 				ca.nttl = time.Duration(ttl) * time.Second
 				args = args[1:]
@@ -77,6 +82,10 @@ func cacheParse(c *caddy.Controller) (*Cache, error) {
 					if err != nil {
 						return nil, err
 					}
+					// Reserve 0 (and smaller for future things)
+					if pttl <= 0 {
+						return nil, fmt.Errorf("cache TTL can not be zero or negative: %d", pttl)
+					}
 					ca.pttl = time.Duration(pttl) * time.Second
 				}
 			case Denial:
@@ -94,6 +103,10 @@ func cacheParse(c *caddy.Controller) (*Cache, error) {
 					if err != nil {
 						return nil, err
 					}
+					// Reserve 0 (and smaller for future things)
+					if nttl <= 0 {
+						return nil, fmt.Errorf("cache TTL can not be zero or negative: %d", nttl)
+					}
 					ca.nttl = time.Duration(nttl) * time.Second
 				}
 			default:
diff --git a/middleware/cache/setup_test.go b/middleware/cache/setup_test.go
index ee8976a49..f46a93b76 100644
--- a/middleware/cache/setup_test.go
+++ b/middleware/cache/setup_test.go
@@ -44,6 +44,11 @@ func TestSetup(t *testing.T) {
 			positive 15
 			negative aaa
 		}`, true, defaultCap, defaultCap, maxTTL, maxTTL},
+		{`cache 0 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL},
+		{`cache -1 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL},
+		{`cache 1 example.nl {
+			positive 0
+		}`, true, defaultCap, defaultCap, maxTTL, maxTTL},
 	}
 	for i, test := range tests {
 		c := caddy.NewTestController("dns", test.input)