From 813cc5d08e2b3e7472472f4da7615e1243b371d9 Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <14937183+ykhr53@users.noreply.github.com>
Date: Mon, 24 Feb 2020 19:03:53 +0000
Subject: [PATCH] plugin/hosts: Modifies NODATA handling (#3536)

* Modifies NODATA handling

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* fix some comments

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* fix test code to take care NODATA

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* add if statement to avoid nil pointer

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* Modifies NODATA handling

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* fix some comments

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* fix test code to take care NODATA

Signed-off-by: ykhr53 <ykhr53@yokohei.com>

* add if statement to avoid nil pointer

Signed-off-by: ykhr53 <ykhr53@yokohei.com>
---
 plugin/hosts/hosts.go      | 10 +++----
 plugin/hosts/hosts_test.go | 54 ++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/plugin/hosts/hosts.go b/plugin/hosts/hosts.go
index a94c65a7e1e..5c644e75220 100644
--- a/plugin/hosts/hosts.go
+++ b/plugin/hosts/hosts.go
@@ -52,15 +52,15 @@ func (h Hosts) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 		answers = aaaa(qname, h.options.ttl, ips)
 	}
 
-	if len(answers) == 0 {
+	// Only on NXDOMAIN we will fallthrough.
+	if len(answers) == 0 && !h.otherRecordsExist(qname) {
 		if h.Fall.Through(qname) {
 			return plugin.NextOrFailure(h.Name(), h.Next, ctx, w, r)
 		}
-		// We want to send an NXDOMAIN, but because of /etc/hosts' setup we don't have a SOA, so we make it REFUSED
+
+		// We want to send an NXDOMAIN, but because of /etc/hosts' setup we don't have a SOA, so we make it SERVFAIL
 		// to at least give an answer back to signals we're having problems resolving this.
-		if !h.otherRecordsExist(qname) {
-			return dns.RcodeServerFailure, nil
-		}
+		return dns.RcodeServerFailure, nil
 	}
 
 	m := new(dns.Msg)
diff --git a/plugin/hosts/hosts_test.go b/plugin/hosts/hosts_test.go
index 062c998513c..320655adbbe 100644
--- a/plugin/hosts/hosts_test.go
+++ b/plugin/hosts/hosts_test.go
@@ -6,38 +6,53 @@ import (
 	"testing"
 
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
+	"github.com/coredns/coredns/plugin/pkg/fall"
 	"github.com/coredns/coredns/plugin/test"
 
 	"github.com/miekg/dns"
 )
 
 func TestLookupA(t *testing.T) {
-	h := Hosts{
-		Next: test.ErrorHandler(),
-		Hostsfile: &Hostsfile{
-			Origins: []string{"."},
-			hmap:    newMap(),
-			inline:  newMap(),
-			options: newOptions(),
-		},
-	}
-	h.hmap = h.parse(strings.NewReader(hostsExample))
-
-	ctx := context.TODO()
-
 	for _, tc := range hostsTestCases {
 		m := tc.Msg()
 
+		var tcFall fall.F
+		isFall := tc.Qname == "fallthrough-example.org."
+		if isFall {
+			tcFall = fall.Root
+		} else {
+			tcFall = fall.Zero
+		}
+
+		h := Hosts{
+			Next: test.NextHandler(dns.RcodeNameError, nil),
+			Hostsfile: &Hostsfile{
+				Origins: []string{"."},
+				hmap:    newMap(),
+				inline:  newMap(),
+				options: newOptions(),
+			},
+			Fall: tcFall,
+		}
+		h.hmap = h.parse(strings.NewReader(hostsExample))
+
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
-		_, err := h.ServeDNS(ctx, rec, m)
+
+		rcode, err := h.ServeDNS(context.Background(), rec, m)
 		if err != nil {
 			t.Errorf("Expected no error, got %v", err)
 			return
 		}
 
-		resp := rec.Msg
-		if err := test.SortAndCheck(resp, tc); err != nil {
-			t.Error(err)
+		if isFall && tc.Rcode != rcode {
+			t.Errorf("Expected rcode is %d, but got %d", tc.Rcode, rcode)
+			return
+		}
+
+		if resp := rec.Msg; rec.Msg != nil {
+			if err := test.SortAndCheck(resp, tc); err != nil {
+				t.Error(err)
+			}
 		}
 	}
 }
@@ -88,6 +103,10 @@ var hostsTestCases = []test.Case{
 		Qname: "example.org.", Qtype: dns.TypeMX,
 		Answer: []dns.RR{},
 	},
+	{
+		Qname: "fallthrough-example.org.", Qtype: dns.TypeAAAA,
+		Answer: []dns.RR{}, Rcode: dns.RcodeSuccess,
+	},
 }
 
 const hostsExample = `
@@ -95,6 +114,7 @@ const hostsExample = `
 ::1 localhost localhost.domain
 10.0.0.1 example.org
 ::FFFF:10.0.0.2 example.com
+10.0.0.3 fallthrough-example.org
 reload 5s
 timeout 3600
 `