Fix truncation of messages longer than permitted by the client (#1417)

* Fix truncation of messages longer than permitted by the client

CoreDNS currently doesn't respect the maximum response size advertised
by the client and returns the full answer on a message with the TC bit
set. This breaks client implementations which rely on DNS servers
respecting the advertised size limit, for example the Ruby stdlib
client. It also has negative network performance implications, as large
messages will be split up into multiple UDP packets, even though the
client will discard the truncated response anyway.

While RFC 2181 permits the response of partial RRSets, finding the
correct number of records fitting into the advertised response size is
non-trivial. As clients should ignore truncated messages, this change
simply removes the full RRSet on truncated messages.

* Remove incorrect etcd test assertion

If a client requests a TXT record larger than its advertised buffer
size, a DNS server should _not_ respond with the answer, but truncate
the message and set the TC bit, so that the client can retry using TCP.

2 files changed, 34 insertions, 6 deletions

diff --git a/request/request.go b/request/request.go
index 2ce2c7acc..9672edeb1 100644
--- a/request/request.go
+++ b/request/request.go
@@ -180,11 +180,11 @@ const (
 	ScrubDone
 )
 
-// Scrub scrubs the reply message so that it will fit the client's buffer. If even after dropping
-// the additional section, it still does not fit the TC bit will be set on the message. Note,
-// the TC bit will be set regardless of protocol, even TCP message will get the bit, the client
-// should then retry with pigeons.
-// TODO(referral).
+// Scrub scrubs the reply message so that it will fit the client's buffer. If
+// even after dropping the additional section it does not fit, the answer will
+// be cleared and the TC bit will be set on the message. Note, the TC bit will
+// be set regardless of protocol, even TCP message will get the bit, the client
+// should then retry with pigeons. TODO(referral).
 func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
 	size := r.Size()
 	l := reply.Len()
@@ -200,8 +200,9 @@ func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
 	if size >= l {
 		return reply, ScrubDone
 	}
-	// Still?!! does not fit.
+
 	reply.Truncated = true
+	reply.Answer = nil
 	return reply, ScrubDone
 }
 
diff --git a/request/request_test.go b/request/request_test.go
index 2311d89ea..49b825627 100644
--- a/request/request_test.go
+++ b/request/request_test.go
@@ -1,6 +1,7 @@
 package request
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/coredns/coredns/plugin/test"
@@ -60,6 +61,32 @@ func TestRequestMalformed(t *testing.T) {
 	}
 }
 
+func TestRequestScrub(t *testing.T) {
+	m := new(dns.Msg)
+	m.SetQuestion("large.example.com.", dns.TypeSRV)
+	req := Request{W: &test.ResponseWriter{}, Req: m}
+
+	reply := new(dns.Msg)
+	reply.SetReply(m)
+	for i := 1; i < 200; i++ {
+		reply.Answer = append(reply.Answer, test.SRV(fmt.Sprintf(
+			"large.example.com. 10 IN SRV 0 0 80 10-0-0-%d.default.pod.k8s.example.com.",
+			i,
+		)))
+	}
+
+	msg, got := req.Scrub(reply)
+	if want := ScrubDone; want != got {
+		t.Errorf("want scrub result %d, got %d", want, got)
+	}
+	if want, got := req.Size(), msg.Len(); want < got {
+		t.Errorf("want scrub to reduce message length below %d bytes, got %d bytes", want, got)
+	}
+	if !msg.Truncated {
+		t.Errorf("want scrub to set truncated bit")
+	}
+}
+
 func BenchmarkRequestDo(b *testing.B) {
 	st := testRequest()