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

Messages with nothing in them are considered OtherError, they can not
serve any purpose for normal clients (i.e. dyn update or notifies might
have a use for them).

Also update a test in the cache plugin, so that we explicitaly test for
this case.

4 files changed, 35 insertions, 1 deletions

diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go
index cb7a10140..2967e1b34 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 classification: %d", mt)
+		log.Printf("[WARNING] Caching called with unknown typification: %d", mt)
 	}
 }
 
diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go
index b475f3473..63b0989b3 100644
--- a/plugin/cache/cache_test.go
+++ b/plugin/cache/cache_test.go
@@ -22,6 +22,7 @@ type cacheTestCase struct {
 	Authoritative      bool
 	RecursionAvailable bool
 	Truncated          bool
+	Response           bool
 	shouldCache        bool
 }
 
@@ -112,6 +113,15 @@ 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{
 			Qname: "example.org.", Qtype: dns.TypeMX,
@@ -140,6 +150,7 @@ 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/pkg/response/typify.go b/plugin/pkg/response/typify.go
index 9faa17d7b..a80b6b39f 100644
--- a/plugin/pkg/response/typify.go
+++ b/plugin/pkg/response/typify.go
@@ -55,6 +55,7 @@ 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 {
@@ -76,6 +77,11 @@ 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 faeaf3579..967ae76a3 100644
--- a/plugin/pkg/response/typify_test.go
+++ b/plugin/pkg/response/typify_test.go
@@ -26,6 +26,23 @@ 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()