diff options
author | 2020-10-15 16:47:07 +0200 | |
---|---|---|
committer | 2020-10-15 07:47:07 -0700 | |
commit | 268781d3553cca94f5e5ecbd248a537fbdf6dae8 (patch) | |
tree | 21c5e80cbdceca2e85bcc282ca77add3df8fc634 /plugin/cache/handler.go | |
parent | 6938dac21d9a79bc34f4347a52fda4d1bf24b259 (diff) | |
download | coredns-268781d3553cca94f5e5ecbd248a537fbdf6dae8.tar.gz coredns-268781d3553cca94f5e5ecbd248a537fbdf6dae8.tar.zst coredns-268781d3553cca94f5e5ecbd248a537fbdf6dae8.zip |
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 <miek@miek.nl>
Diffstat (limited to 'plugin/cache/handler.go')
-rw-r--r-- | plugin/cache/handler.go | 14 |
1 files changed, 7 insertions, 7 deletions
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) |