From 8cce06cba10c42ce501a57bd3b7d06ca6064d774 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Sat, 17 Feb 2018 19:45:52 +0100
Subject: [PATCH] Type.extra (#1538)

* Revert "pkg/typify: empty messages are OtherError (#1531)"

This reverts commit fc1d73ffa9ae193c4cfca4adc194ae43f9360dbb.

* plugin/cache: add failsafeTTL

If we can not see what TTL we should put on a message to be cached, use
5 seconds as minimal TTL. We used to apply the maximum TTL to these
  messages.
---
 plugin/cache/cache.go              |  7 ++++---
 plugin/cache/cache_test.go         | 11 -----------
 plugin/cache/item.go               |  5 +++++
 plugin/pkg/response/typify.go      |  6 ------
 plugin/pkg/response/typify_test.go | 17 -----------------
 5 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go
index 2967e1b34..a8776afac 100644
--- a/plugin/cache/cache.go
+++ b/plugin/cache/cache.go
@@ -171,7 +171,7 @@ func (w *ResponseWriter) set(m *dns.Msg, key int, mt response.Type, duration tim
 	case response.OtherError:
 		// don't cache these
 	default:
-		log.Printf("[WARNING] Caching called with unknown typification: %d", mt)
+		log.Printf("[WARNING] Caching called with unknown classification: %d", mt)
 	}
 }
 
@@ -186,8 +186,9 @@ func (w *ResponseWriter) Write(buf []byte) (int, error) {
 }
 
 const (
-	maxTTL  = 1 * time.Hour
-	maxNTTL = 30 * time.Minute
+	maxTTL      = 1 * time.Hour
+	maxNTTL     = 30 * time.Minute
+	failSafeTTL = 5 * time.Second
 
 	defaultCap = 10000 // default capacity of the cache.
 
diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go
index 63b0989b3..b475f3473 100644
--- a/plugin/cache/cache_test.go
+++ b/plugin/cache/cache_test.go
@@ -22,7 +22,6 @@ type cacheTestCase struct {
 	Authoritative      bool
 	RecursionAvailable bool
 	Truncated          bool
-	Response           bool
 	shouldCache        bool
 }
 
@@ -112,15 +111,6 @@ var cacheTestCases = []cacheTestCase{
 		},
 		shouldCache: false,
 	},
-	{
-		// Response with only something in the additional, this should not be cached.
-		Response: true,
-		in: test.Case{
-			Qname: "example.org.", Qtype: dns.TypeMX,
-			Extra: []dns.RR{test.MX("example.org.	1800	IN	MX	1 mx.example.org.")},
-		},
-		shouldCache: false,
-	},
 	{
 		RecursionAvailable: true, Authoritative: true,
 		Case: test.Case{
@@ -150,7 +140,6 @@ func cacheMsg(m *dns.Msg, tc cacheTestCase) *dns.Msg {
 	m.AuthenticatedData = tc.AuthenticatedData
 	m.Authoritative = tc.Authoritative
 	m.Rcode = tc.Rcode
-	m.Response = tc.Response
 	m.Truncated = tc.Truncated
 	m.Answer = tc.in.Answer
 	m.Ns = tc.in.Ns
diff --git a/plugin/cache/item.go b/plugin/cache/item.go
index 3943ff4ae..55d618a39 100644
--- a/plugin/cache/item.go
+++ b/plugin/cache/item.go
@@ -95,6 +95,11 @@ func minMsgTTL(m *dns.Msg, mt response.Type) time.Duration {
 		return 0
 	}
 
+	// No data to examine, return a short ttl as a fail safe.
+	if len(m.Answer)+len(m.Ns) == 0 {
+		return failSafeTTL
+	}
+
 	minTTL := maxTTL
 	for _, r := range append(m.Answer, m.Ns...) {
 		switch mt {
diff --git a/plugin/pkg/response/typify.go b/plugin/pkg/response/typify.go
index a80b6b39f..9faa17d7b 100644
--- a/plugin/pkg/response/typify.go
+++ b/plugin/pkg/response/typify.go
@@ -55,7 +55,6 @@ func Typify(m *dns.Msg, t time.Time) (Type, *dns.OPT) {
 	if m == nil {
 		return OtherError, nil
 	}
-
 	opt := m.IsEdns0()
 	do := false
 	if opt != nil {
@@ -77,11 +76,6 @@ func Typify(m *dns.Msg, t time.Time) (Type, *dns.OPT) {
 		}
 	}
 
-	if m.Response && len(m.Answer) == 0 && len(m.Ns) == 0 {
-		// Response with nothing in it, maybe stuff in the additional section, this is not useful.
-		return OtherError, opt
-	}
-
 	// If our message contains any expired sigs and we care about that, we should return expired
 	if do {
 		if expired := typifyExpired(m, t); expired {
diff --git a/plugin/pkg/response/typify_test.go b/plugin/pkg/response/typify_test.go
index 967ae76a3..faeaf3579 100644
--- a/plugin/pkg/response/typify_test.go
+++ b/plugin/pkg/response/typify_test.go
@@ -26,23 +26,6 @@ func TestTypifyDelegation(t *testing.T) {
 	}
 }
 
-func TestTypifyEmptyMessage(t *testing.T) {
-	m := new(dns.Msg)
-
-	// Normal question, with response = false
-	m.SetQuestion("example.org.", dns.TypeAAAA)
-	mt, _ := Typify(m, time.Now().UTC())
-	if mt != NoError {
-		t.Errorf("message is wrongly typified, expected NoError, got %s", mt)
-	}
-	// In case of a Reponse = true, this weird.
-	m.Response = true
-	mt, _ = Typify(m, time.Now().UTC())
-	if mt != OtherError {
-		t.Errorf("message is wrongly typified, expected OtherError, got %s", mt)
-	}
-}
-
 func TestTypifyRRSIG(t *testing.T) {
 	now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017")
 	utc := now.UTC()