From 268781d3553cca94f5e5ecbd248a537fbdf6dae8 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Thu, 15 Oct 2020 16:47:07 +0200 Subject: cache: do the msg copy right (#4207) Not sure why this is proving so difficult.. pointers are hard? [Was tempted to rollback all tweaks here, but the original issue we're fixing it too important to not have a proper fix]. But we need to make a copy of the message at the earliest point in the handler because we are changing it (adding an opt rr). If we do this on the original message (which is a pointer) we change it (obvs). When undoing those changes we do work on a copy. Re: testing. There isn't a explicit test for this, so I've added on to the top-level test/ directory, which indeed makes the issue visible: master: ~~~ go test -v -run=TestLookupCacheWithoutEdns === RUN TestLookupCacheWithoutEdns cache_test.go:154: Expected no OPT RR, but got: ;; OPT PSEUDOSECTION: ; EDNS: version 0; flags: do; udp: 2048 --- FAIL: TestLookupCacheWithoutEdns (0.01s) FAIL ~~~ This branch: ~~~ % go test -v -run=TestLookupCacheWithoutEdns === RUN TestLookupCacheWithoutEdns --- PASS: TestLookupCacheWithoutEdns (0.01s) PASS ok github.com/coredns/coredns/test 0.109s ~~~ Signed-off-by: Miek Gieben --- plugin/cache/handler.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'plugin/cache/handler.go') diff --git a/plugin/cache/handler.go b/plugin/cache/handler.go index 987dd61b2..406ece8e6 100644 --- a/plugin/cache/handler.go +++ b/plugin/cache/handler.go @@ -14,12 +14,13 @@ import ( // ServeDNS implements the plugin.Handler interface. func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { - state := request.Request{W: w, Req: r} + rc := r.Copy() // We potentially modify r, to prevent other plugins from seeing this (r is a pointer), copy r into rc. + state := request.Request{W: w, Req: rc} do := state.Do() zone := plugin.Zones(c.Zones).Matches(state.Name()) if zone == "" { - return plugin.NextOrFailure(c.Name(), c.Next, ctx, w, r) + return plugin.NextOrFailure(c.Name(), c.Next, ctx, w, rc) } now := c.now().UTC() @@ -39,22 +40,21 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) } if i == nil { if !do { - setDo(r) + setDo(rc) } crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do} - return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r) + return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, rc) } if ttl < 0 { servedStale.WithLabelValues(server).Inc() // Adjust the time to get a 0 TTL in the reply built from a stale item. now = now.Add(time.Duration(ttl) * time.Second) go func() { - r := r.Copy() if !do { - setDo(r) + setDo(rc) } crr := &ResponseWriter{Cache: c, state: state, server: server, prefetch: true, remoteAddr: w.LocalAddr(), do: do} - plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r) + plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, rc) }() } resp := i.toMsg(r, now, do) -- cgit v1.2.3