From 8c994ce7724747aae0e8ec5bdcb9ccaf4d1babbb Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Mon, 12 Dec 2022 14:37:25 +0900 Subject: [PATCH 1/8] Add timeout --- go.mod | 2 ++ go.sum | 5 ++++ p2p/transport.go | 5 +++- p2p/transport_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 4a8265d98..682897376 100644 --- a/go.mod +++ b/go.mod @@ -48,6 +48,8 @@ require ( gotest.tools v2.2.0+incompatible // indirect ) +require github.com/miekg/dns v1.1.43 + require ( github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/DataDog/zstd v1.4.1 // indirect diff --git a/go.sum b/go.sum index 0a60e4295..02d29f5a3 100644 --- a/go.sum +++ b/go.sum @@ -307,6 +307,8 @@ github.com/magiconair/properties v1.8.6/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPK github.com/mattn/go-sqlite3 v1.14.9 h1:10HX2Td0ocZpYEjhilsuo6WWtUqttj2Kb0KtD86/KYA= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= +github.com/miekg/dns v1.1.43 h1:JKfpVSCB84vrAmHzyrsxB5NAr5kLoMXZArPSw7Qlgyg= +github.com/miekg/dns v1.1.43/go.mod h1:+evo5L0630/F6ca/Z9+GAqzhjGyn8/c+TBaOyfEl0V4= github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643 h1:hLDRPB66XQT/8+wG9WsDpiCvZf1yKO7sz7scAjSlBa0= github.com/mimoo/StrobeGo v0.0.0-20181016162300-f8f6d4d2b643/go.mod h1:43+3pMjjKimDBf5Kr4ZFNGbLql1zKkbImw+fZbw3geM= github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g= @@ -586,6 +588,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -638,6 +642,7 @@ golang.org/x/sys v0.0.0-20210104204734-6f8348627aad/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210303074136-134d130e1a04/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/p2p/transport.go b/p2p/transport.go index 2165d28bf..4f7cdde70 100644 --- a/p2p/transport.go +++ b/p2p/transport.go @@ -595,7 +595,10 @@ func resolveIPs(resolver IPResolver, c net.Conn) ([]net.IP, error) { return nil, err } - addrs, err := resolver.LookupIPAddr(context.Background(), host) + timeoutCtx, cancelFunc := context.WithTimeout(context.Background(), 1*time.Second) + defer cancelFunc() + + addrs, err := resolver.LookupIPAddr(timeoutCtx, host) if err != nil { return nil, err } diff --git a/p2p/transport_test.go b/p2p/transport_test.go index 7b5af5656..5a502eba7 100644 --- a/p2p/transport_test.go +++ b/p2p/transport_test.go @@ -1,11 +1,13 @@ package p2p import ( + "context" "fmt" "math/rand" "net" "reflect" "runtime" + "strconv" "strings" "testing" "time" @@ -14,6 +16,7 @@ import ( "github.com/line/ostracon/libs/protoio" "github.com/line/ostracon/p2p/conn" tmp2p "github.com/line/ostracon/proto/ostracon/p2p" + "github.com/miekg/dns" "github.com/stretchr/testify/require" ) @@ -638,6 +641,22 @@ func TestTransportAddChannel(t *testing.T) { } } +func TestTransportResolveIPs(t *testing.T) { + go startFakeDNS() + + r := &net.Resolver{ + PreferGo: true, + Dial: func(ctx context.Context, network, address string) (net.Conn, error) { + d := net.Dialer{ + Timeout: time.Millisecond * time.Duration(500), + } + return d.DialContext(ctx, network, "127.0.0.1:5355") + }, + } + _, err := resolveIPs(r, &testTransportConn{}) + require.Contains(t, err.Error(), "lookup test.local: i/o timeout") +} + // create listener func testSetupMultiplexTransport(t *testing.T) *MultiplexTransport { var ( @@ -706,3 +725,47 @@ func (c *testTransportConn) SetWriteDeadline(_ time.Time) error { func (c *testTransportConn) Write(_ []byte) (int, error) { return -1, fmt.Errorf("write() not implemented") } + +// fake dns server +var records = map[string]string{ + "test.local.": "192.168.0.2", +} + +func parseQuery(m *dns.Msg) { + time.Sleep(5 * time.Minute) + for _, q := range m.Question { + switch q.Qtype { + case dns.TypeA: + ip := records[q.Name] + if ip != "" { + rr, err := dns.NewRR(fmt.Sprintf("%s A %s", q.Name, ip)) + if err == nil { + m.Answer = append(m.Answer, rr) + } + } + } + } +} + +func handleDnsRequest(w dns.ResponseWriter, r *dns.Msg) { + m := new(dns.Msg) + m.SetReply(r) + m.Compress = false + switch r.Opcode { + case dns.OpcodeQuery: + parseQuery(m) + } + _ = w.WriteMsg(m) +} + +func startFakeDNS() { + // attach request handler func + dns.HandleFunc("local.", handleDnsRequest) + + // start server + port := 5355 + server := &dns.Server{Addr: ":" + strconv.Itoa(port), Net: "udp"} + err := server.ListenAndServe() + defer server.Shutdown() + panic(err.Error()) +} From 70988a9944b204774cf5ef0af403f64e4e7c837f Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Mon, 12 Dec 2022 14:42:48 +0900 Subject: [PATCH 2/8] Add no lint tags --- p2p/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/transport_test.go b/p2p/transport_test.go index 5a502eba7..4d73d0b35 100644 --- a/p2p/transport_test.go +++ b/p2p/transport_test.go @@ -766,6 +766,6 @@ func startFakeDNS() { port := 5355 server := &dns.Server{Addr: ":" + strconv.Itoa(port), Net: "udp"} err := server.ListenAndServe() - defer server.Shutdown() + defer server.Shutdown() // nolint: errcheck panic(err.Error()) } From ba94865ec653195f2e78d6dec28cc64995028a23 Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Mon, 12 Dec 2022 16:10:32 +0900 Subject: [PATCH 3/8] Add error handling --- p2p/transport_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/p2p/transport_test.go b/p2p/transport_test.go index 4d73d0b35..4ea64125f 100644 --- a/p2p/transport_test.go +++ b/p2p/transport_test.go @@ -767,5 +767,7 @@ func startFakeDNS() { server := &dns.Server{Addr: ":" + strconv.Itoa(port), Net: "udp"} err := server.ListenAndServe() defer server.Shutdown() // nolint: errcheck - panic(err.Error()) + if err != nil { + panic(err.Error()) + } } From f542429544c3193f28644aa4130d29208f2cadd9 Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Mon, 12 Dec 2022 19:40:01 +0900 Subject: [PATCH 4/8] Fix `go.mod` --- go.mod | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 682897376..b67812e54 100644 --- a/go.mod +++ b/go.mod @@ -25,6 +25,7 @@ require ( github.com/herumi/bls-eth-go-binary v0.0.0-20220509081320-2d8ab06de53c github.com/lib/pq v1.10.7 github.com/libp2p/go-buffer-pool v0.1.0 + github.com/miekg/dns v1.1.43 github.com/minio/highwayhash v1.0.2 github.com/ory/dockertest v3.3.5+incompatible github.com/pkg/errors v0.9.1 @@ -48,8 +49,6 @@ require ( gotest.tools v2.2.0+incompatible // indirect ) -require github.com/miekg/dns v1.1.43 - require ( github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/DataDog/zstd v1.4.1 // indirect From a58312f568566803ab6232a638f5ece76af1b41d Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Tue, 13 Dec 2022 11:49:28 +0900 Subject: [PATCH 5/8] Add retry --- p2p/transport.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/p2p/transport.go b/p2p/transport.go index 4f7cdde70..65b87c249 100644 --- a/p2p/transport.go +++ b/p2p/transport.go @@ -595,10 +595,17 @@ func resolveIPs(resolver IPResolver, c net.Conn) ([]net.IP, error) { return nil, err } - timeoutCtx, cancelFunc := context.WithTimeout(context.Background(), 1*time.Second) - defer cancelFunc() - - addrs, err := resolver.LookupIPAddr(timeoutCtx, host) + // implement with reference to https://learn.microsoft.com/en-us/troubleshoot/windows-server/networking/dns-client-resolution-timeouts#what-is-the-default-behavior-of-a-dns-client-when-a-single-dns-server-is-configured-on-the-nic + timeouts := []time.Duration{1, 1, 2, 4, 2} + addrs := []net.IPAddr{} + for _, to := range timeouts { + timeoutCtx, cancelFunc := context.WithTimeout(context.Background(), to*time.Second) + defer cancelFunc() + addrs, err = resolver.LookupIPAddr(timeoutCtx, host) + if err == nil { + break + } + } if err != nil { return nil, err } From b2cd6d6a8485f6757a3e331859caa62499861814 Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Tue, 13 Dec 2022 14:50:00 +0900 Subject: [PATCH 6/8] Shutdown dns after test --- TestTransportResolveIPs$ | 3 +++ p2p/transport_test.go | 21 +++++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 TestTransportResolveIPs$ diff --git a/TestTransportResolveIPs$ b/TestTransportResolveIPs$ new file mode 100644 index 000000000..3081f3439 --- /dev/null +++ b/TestTransportResolveIPs$ @@ -0,0 +1,3 @@ +flag needs an argument: -count +usage: go test [build/test flags] [packages] [build/test flags & test binary flags] +Run 'go help test' and 'go help testflag' for details. diff --git a/p2p/transport_test.go b/p2p/transport_test.go index 4ea64125f..f448f6ba9 100644 --- a/p2p/transport_test.go +++ b/p2p/transport_test.go @@ -642,7 +642,7 @@ func TestTransportAddChannel(t *testing.T) { } func TestTransportResolveIPs(t *testing.T) { - go startFakeDNS() + server := startFakeDNS() r := &net.Resolver{ PreferGo: true, @@ -655,6 +655,9 @@ func TestTransportResolveIPs(t *testing.T) { } _, err := resolveIPs(r, &testTransportConn{}) require.Contains(t, err.Error(), "lookup test.local: i/o timeout") + + err = server.Shutdown() + require.NoError(t, err) } // create listener @@ -732,7 +735,7 @@ var records = map[string]string{ } func parseQuery(m *dns.Msg) { - time.Sleep(5 * time.Minute) + time.Sleep(5 * time.Second) for _, q := range m.Question { switch q.Qtype { case dns.TypeA: @@ -758,16 +761,18 @@ func handleDnsRequest(w dns.ResponseWriter, r *dns.Msg) { _ = w.WriteMsg(m) } -func startFakeDNS() { +func startFakeDNS() *dns.Server { // attach request handler func dns.HandleFunc("local.", handleDnsRequest) // start server port := 5355 server := &dns.Server{Addr: ":" + strconv.Itoa(port), Net: "udp"} - err := server.ListenAndServe() - defer server.Shutdown() // nolint: errcheck - if err != nil { - panic(err.Error()) - } + go func() { + err := server.ListenAndServe() + if err != nil { + panic(err.Error()) + } + }() + return server } From fb950c3e67b3f770071498343a4bb0e4981059ac Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Tue, 13 Dec 2022 14:54:31 +0900 Subject: [PATCH 7/8] Remove files --- TestTransportResolveIPs$ | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 TestTransportResolveIPs$ diff --git a/TestTransportResolveIPs$ b/TestTransportResolveIPs$ deleted file mode 100644 index 3081f3439..000000000 --- a/TestTransportResolveIPs$ +++ /dev/null @@ -1,3 +0,0 @@ -flag needs an argument: -count -usage: go test [build/test flags] [packages] [build/test flags & test binary flags] -Run 'go help test' and 'go help testflag' for details. From 9f9998ab4bd5f0bc385b8ff2ffbd2bbd7acd87c8 Mon Sep 17 00:00:00 2001 From: Shogo Hyodo Date: Tue, 13 Dec 2022 15:33:35 +0900 Subject: [PATCH 8/8] Use `defer` --- p2p/transport_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/p2p/transport_test.go b/p2p/transport_test.go index f448f6ba9..58555a671 100644 --- a/p2p/transport_test.go +++ b/p2p/transport_test.go @@ -643,6 +643,10 @@ func TestTransportAddChannel(t *testing.T) { func TestTransportResolveIPs(t *testing.T) { server := startFakeDNS() + defer func() { + err := server.Shutdown() + require.NoError(t, err) + }() r := &net.Resolver{ PreferGo: true, @@ -655,9 +659,6 @@ func TestTransportResolveIPs(t *testing.T) { } _, err := resolveIPs(r, &testTransportConn{}) require.Contains(t, err.Error(), "lookup test.local: i/o timeout") - - err = server.Shutdown() - require.NoError(t, err) } // create listener