TrueCloudLab/coredns
16
0
Fork 2
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

file: close correctlty after AXFR (#2943)

Browse source 
* file: close correctlty after AXFR

Don't hijack, but wait for the writes to be done and then savely close
the connection.

Fixes: #2929

Signed-off-by: Miek Gieben <miek@miek.nl>

* Update comment

Signed-off-by: Miek Gieben <miek@miek.nl>

* file: close correctlty after AXFR (#2943)

apply

Signed-off-by: Miek Gieben <miek@miek.nl>
This commit is contained in:
Miek Gieben 2019-07-03 07:01:57 +01:00 • committed by GitHub
parent f8bba51f84
commit 3e5fd21e68
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
plugin/file/xfr.go
View file

@ -3,6 +3,7 @@ package file
import (
"context"
"fmt"
"sync"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/request"
@ -31,9 +32,13 @@ func (x Xfr) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (in
}
ch := make(chan *dns.Envelope)
defer close(ch)
tr := new(dns.Transfer)
go tr.Out(w, r, ch)
wg := new(sync.WaitGroup)
go func() {
wg.Add(1)
tr.Out(w, r, ch)
wg.Done()
}()
j, l := 0, 0
records = append(records, records[0]) // add closing SOA to the end
@ -49,9 +54,9 @@ func (x Xfr) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (in
if j < len(records) {
ch <- &dns.Envelope{RR: records[j:]}
}
close(ch) // Even though we close the channel here, we still have
wg.Wait() // to wait before we can return and close the connection.
w.Hijack()
// w.Close() // Client closes connection
return dns.RcodeSuccess, nil
}

38
test/file_xfr_test.go
View file

@ -2,7 +2,6 @@ package test
import (
"fmt"
"io"
"strings"
"testing"
"time"
@ -35,8 +34,7 @@ func TestLargeAXFR(t *testing.T) {
}
}
`
// Start server, and send an AXFR query to the TCP port. We set the deadline to prevent the test from hanging.
// Start server, and send an AXFR query to the TCP port. We set the deadline to prevent the test from hanging.
i, _, tcp, err := CoreDNSServerAndPorts(corefile)
if err != nil {
t.Fatalf("Could not get CoreDNS serving instance: %s", err)
@ -53,11 +51,18 @@ func TestLargeAXFR(t *testing.T) {
co.SetWriteDeadline(time.Now().Add(5 * time.Second))
err = co.WriteMsg(m)
if err != nil {
t.Fatalf("Unable to send TCP query: %s", err)
t.Fatalf("Unable to send AXFR/TCP query: %s", err)
}
nrr := 0 // total number of transferred RRs
co.SetReadDeadline(time.Now().Add(60 * time.Second)) // use a longer timeout as it involves transferring a non-trivial amount of data.
// Then send another query on the same connection. We use this to confirm that multiple outstanding queries won't cause a race.
m.SetQuestion("0.example.com.", dns.TypeAAAA)
err = co.WriteMsg(m)
if err != nil {
t.Fatalf("Unable to send AAAA/TCP query: %s", err)
}
// The AXFR query should be responded first.
nrr := 0 // total number of transferred RRs
for {
resp, err := co.ReadMsg()
if err != nil {
@ -82,18 +87,15 @@ func TestLargeAXFR(t *testing.T) {
t.Fatalf("Got an unexpected number of RRs: %d", nrr)
}
// At the time of the initial implementation of this test, the remaining check would fail due to the problem described in PR #2866, so we return here.
// Once the issue is resolved this workaround should be removed to enable the check.
return
// Once xfr is completed the server should close the connection, so a further read should result in an EOF error.
// This time we use a short timeout to make it faster in case the server doesn't behave as expected.
co.SetReadDeadline(time.Now().Add(time.Second))
_, err = co.ReadMsg()
if err == nil {
t.Fatalf("Expected failure on further read, but it succeeded")
// The file plugin shouldn't hijack or (yet) close the connection, so the second query should also be responded.
resp, err := co.ReadMsg()
if err != nil {
t.Fatalf("Expected to receive reply, but didn't: %s", err)
}
if err != io.EOF {
t.Fatalf("Expected EOF on further read, but got a different error: %v", err)
if len(resp.Answer) < 1 {
t.Fatalf("Expected a non-empty answer, but it was empty")
}
if resp.Answer[len(resp.Answer)-1].Header().Rrtype != dns.TypeAAAA {
t.Fatalf("Expected a AAAA answer, but it wasn't: type %d", resp.Answer[len(resp.Answer)-1].Header().Rrtype)
}
}