fix truncation bug (#2261)

* fix truncation bug

* Generate records with generic RRs

* Remove SoundCloud from test name

* Comment for binary-search -1 adjustment

Explain why the binary search may have exited with
a reply size that is too large by one record.

* Refactor to remove sub variable

patch suggested by miek removes unnecessary sub
variable for removing a single line from the
reply.Extra length.

1 files changed, 19 insertions, 11 deletions

diff --git a/request/request.go b/request/request.go
index f560612c8..260d73f5e 100644
--- a/request/request.go
+++ b/request/request.go
@@ -250,18 +250,16 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 	}
 
 	// Account for the OPT record that gets added in SizeAndDo(), subtract that length.
-	sub := 0
+	re := len(reply.Extra)
 	if r.Req.IsEdns0() != nil {
-		sub = optLen
+		size -= optLen
+		// re can never be 0 because we have an OPT RR.
+		re--
 	}
 
-	// subtract to make spaces for re-added EDNS0 OPT RR.
-	re := len(reply.Extra) - sub
-	size -= sub
-
 	l, m := 0, 0
 	origExtra := reply.Extra
-	for l < re {
+	for l <= re {
 		m = (l + re) / 2
 		reply.Extra = origExtra[:m]
 		rl = reply.Len()
@@ -278,13 +276,18 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 		}
 	}
 
-	// We may come out of this loop with one rotation too many, m makes it too large, but m-1 works.
+	// The binary search only breaks on an exact match, which will be
+	// pretty rare. Normally, the loop will exit when l > re, meaning that
+	// in the previous iteration either:
+	// rl < size: no need to do anything.
+	// rl > size: the final size is too large, and if m > 0, the preceeding
+	// iteration the size was too small. Select that preceeding size.
 	if rl > size && m > 0 {
 		reply.Extra = origExtra[:m-1]
 		rl = reply.Len()
 	}
 
-	if rl < size {
+	if rl <= size {
 		r.SizeAndDo(reply)
 		return reply
 	}
@@ -292,7 +295,7 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 	ra := len(reply.Answer)
 	l, m = 0, 0
 	origAnswer := reply.Answer
-	for l < ra {
+	for l <= ra {
 		m = (l + ra) / 2
 		reply.Answer = origAnswer[:m]
 		rl = reply.Len()
@@ -309,7 +312,12 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 		}
 	}
 
-	// We may come out of this loop with one rotation too many, m makes it too large, but m-1 works.
+	// The binary search only breaks on an exact match, which will be
+	// pretty rare. Normally, the loop will exit when l > ra, meaning that
+	// in the previous iteration either:
+	// rl < size: no need to do anything.
+	// rl > size: the final size is too large, and if m > 0, the preceeding
+	// iteration the size was too small. Select that preceeding size.
 	if rl > size && m > 0 {
 		reply.Answer = origAnswer[:m-1]
 		// No need to recalc length, as we don't use it. We set truncated anyway. Doing