aboutsummaryrefslogtreecommitdiff
path: root/request
diff options
context:
space:
mode:
authorGravatar Miek Gieben <miek@miek.nl> 2018-03-24 11:50:55 +0000
committerGravatar GitHub <noreply@github.com> 2018-03-24 11:50:55 +0000
commita8d02d970c3707b6d40c280a4f0cd35590c6ed59 (patch)
tree0896c3ba0f5a42ca40db09897ec0ea645366ea2e /request
parent1c6efbd96297d03831e57f1cc2816e4e09a0dbb3 (diff)
downloadcoredns-a8d02d970c3707b6d40c280a4f0cd35590c6ed59.tar.gz
coredns-a8d02d970c3707b6d40c280a4f0cd35590c6ed59.tar.zst
coredns-a8d02d970c3707b6d40c280a4f0cd35590c6ed59.zip
request.Scrub: test for rl==size case (#1631)
* request.Scrub: test for rl==size case Make a test case for the new break statement in Scrub and also account for the OPT record that may get re-added in SizeAndDo() - otherwise we may break clients that expect this. * Fix comment
Diffstat (limited to 'request')
-rw-r--r--request/request.go21
-rw-r--r--request/request_test.go50
2 files changed, 63 insertions, 8 deletions
diff --git a/request/request.go b/request/request.go
index 9c9512f3a..0629a2123 100644
--- a/request/request.go
+++ b/request/request.go
@@ -167,7 +167,6 @@ func (r *Request) SizeAndDo(m *dns.Msg) bool {
if odo {
o.SetDo()
}
-
m.Extra = append(m.Extra, o)
return true
}
@@ -202,8 +201,13 @@ func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
return reply, ScrubIgnored
}
+ // Account for the OPT record that gets added in SizeAndDo(), subtract that length.
+ sub := 0
+ if r.Do() {
+ sub = optLen
+ }
origExtra := reply.Extra
- re := len(reply.Extra)
+ re := len(reply.Extra) - sub
l, m := 0, 0
for l < re {
m = (l + re) / 2
@@ -221,8 +225,8 @@ func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
break
}
}
- // We may come out of this loop with one rotation too many as we don't break on rl == size.
- // I.e. m makes it too large, but m-1 works.
+
+ // We may come out of this loop with one rotation too many, m makes it too large, but m-1 works.
if rl > size && m > 0 {
reply.Extra = origExtra[:m-1]
rl = reply.Len()
@@ -252,16 +256,16 @@ func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
break
}
}
- // We may come out of this loop with one rotation too many as we don't break on rl == size.
- // I.e. m makes it too large, but m-1 works.
+
+ // We may come out of this loop with one rotation too many, m makes it too large, but m-1 works.
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
// this extra m-1 step does make it fit in the client's buffer however.
}
- // It now fits, but Truncated.
- r.SizeAndDo(reply)
+ // It now fits, but Truncated. We can't call sizeAndDo() because that adds a new record (OPT)
+ // in the additional section.
reply.Truncated = true
return reply, ScrubAnswer
}
@@ -389,4 +393,5 @@ const (
// TODO(miek): make this less awkward.
doTrue = 1
doFalse = 2
+ optLen = 12 // OPT record length.
)
diff --git a/request/request_test.go b/request/request_test.go
index ff9efcfd0..abb38dea3 100644
--- a/request/request_test.go
+++ b/request/request_test.go
@@ -109,6 +109,56 @@ func TestRequestScrubExtra(t *testing.T) {
}
}
+func TestRequestScrubExtraEdns0(t *testing.T) {
+ m := new(dns.Msg)
+ m.SetQuestion("large.example.com.", dns.TypeSRV)
+ m.SetEdns0(4096, true)
+ req := Request{W: &test.ResponseWriter{}, Req: m}
+
+ reply := new(dns.Msg)
+ reply.SetReply(m)
+ for i := 1; i < 200; i++ {
+ reply.Extra = append(reply.Extra, test.SRV(
+ fmt.Sprintf("large.example.com. 10 IN SRV 0 0 80 10-0-0-%d.default.pod.k8s.example.com.", i)))
+ }
+
+ _, got := req.Scrub(reply)
+ if want := ScrubExtra; want != got {
+ t.Errorf("want scrub result %d, got %d", want, got)
+ }
+ if want, got := req.Size(), reply.Len(); want < got {
+ t.Errorf("want scrub to reduce message length below %d bytes, got %d bytes", want, got)
+ }
+ if reply.Truncated {
+ t.Errorf("want scrub to not set truncated bit")
+ }
+ opt := reply.Extra[len(reply.Extra)-1]
+ if opt.Header().Rrtype != dns.TypeOPT {
+ t.Errorf("Last RR must be OPT record")
+ }
+}
+
+func TestRequestScrubAnswerExact(t *testing.T) {
+ m := new(dns.Msg)
+ m.SetQuestion("large.example.com.", dns.TypeSRV)
+ m.SetEdns0(867, false) // Bit fiddly, but this hits the rl == size break clause in Scrub, 52 RRs should remain.
+ 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.A(fmt.Sprintf("large.example.com. 10 IN A 127.0.0.%d", i)))
+ }
+
+ _, got := req.Scrub(reply)
+ if want := ScrubAnswer; want != got {
+ t.Errorf("want scrub result %d, got %d", want, got)
+ }
+ if want, got := req.Size(), reply.Len(); want < got {
+ t.Errorf("want scrub to reduce message length below %d bytes, got %d bytes", want, got)
+ }
+}
+
func TestRequestMatch(t *testing.T) {
st := testRequest()
reply := new(dns.Msg)