|
1 From bb8706890bc5bab01894f7faf382b15364635d29 Mon Sep 17 00:00:00 2001 |
|
2 From: Matthew Dempsky <[email protected]> |
|
3 Date: Mon, 29 Aug 2016 13:53:32 -0700 |
|
4 Subject: [PATCH 09/38] [release-branch.go1.7] net: restore per-query timeout |
|
5 logic |
|
6 |
|
7 The handling of "options timeout:n" is supposed to be per individual |
|
8 DNS server exchange, not per Lookup call. |
|
9 |
|
10 Fixes #16865. |
|
11 |
|
12 Change-Id: I2304579b9169c1515292f142cb372af9d37ff7c1 |
|
13 Reviewed-on: https://go-review.googlesource.com/28057 |
|
14 Run-TryBot: Matthew Dempsky <[email protected]> |
|
15 TryBot-Result: Gobot Gobot <[email protected]> |
|
16 Reviewed-by: Brad Fitzpatrick <[email protected]> |
|
17 Reviewed-on: https://go-review.googlesource.com/28640 |
|
18 --- |
|
19 src/net/dnsclient_unix.go | 17 ++++---- |
|
20 src/net/dnsclient_unix_test.go | 96 ++++++++++++++++++++++++++++++++++++------ |
|
21 2 files changed, 92 insertions(+), 21 deletions(-) |
|
22 |
|
23 diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go |
|
24 index d12944c..b5b6ffb 100644 |
|
25 --- a/src/net/dnsclient_unix.go |
|
26 +++ b/src/net/dnsclient_unix.go |
|
27 @@ -141,7 +141,7 @@ func (d *Dialer) dialDNS(ctx context.Context, network, server string) (dnsConn, |
|
28 } |
|
29 |
|
30 // exchange sends a query on the connection and hopes for a response. |
|
31 -func exchange(ctx context.Context, server, name string, qtype uint16) (*dnsMsg, error) { |
|
32 +func exchange(ctx context.Context, server, name string, qtype uint16, timeout time.Duration) (*dnsMsg, error) { |
|
33 d := testHookDNSDialer() |
|
34 out := dnsMsg{ |
|
35 dnsMsgHdr: dnsMsgHdr{ |
|
36 @@ -152,6 +152,12 @@ func exchange(ctx context.Context, server, name string, qtype uint16) (*dnsMsg, |
|
37 }, |
|
38 } |
|
39 for _, network := range []string{"udp", "tcp"} { |
|
40 + // TODO(mdempsky): Refactor so defers from UDP-based |
|
41 + // exchanges happen before TCP-based exchange. |
|
42 + |
|
43 + ctx, cancel := context.WithDeadline(ctx, time.Now().Add(timeout)) |
|
44 + defer cancel() |
|
45 + |
|
46 c, err := d.dialDNS(ctx, network, server) |
|
47 if err != nil { |
|
48 return nil, err |
|
49 @@ -180,17 +186,10 @@ func tryOneName(ctx context.Context, cfg *dnsConfig, name string, qtype uint16) |
|
50 return "", nil, &DNSError{Err: "no DNS servers", Name: name} |
|
51 } |
|
52 |
|
53 - deadline := time.Now().Add(cfg.timeout) |
|
54 - if old, ok := ctx.Deadline(); !ok || deadline.Before(old) { |
|
55 - var cancel context.CancelFunc |
|
56 - ctx, cancel = context.WithDeadline(ctx, deadline) |
|
57 - defer cancel() |
|
58 - } |
|
59 - |
|
60 var lastErr error |
|
61 for i := 0; i < cfg.attempts; i++ { |
|
62 for _, server := range cfg.servers { |
|
63 - msg, err := exchange(ctx, server, name, qtype) |
|
64 + msg, err := exchange(ctx, server, name, qtype, cfg.timeout) |
|
65 if err != nil { |
|
66 lastErr = &DNSError{ |
|
67 Err: err.Error(), |
|
68 diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go |
|
69 index c953c1e..6ebeeae 100644 |
|
70 --- a/src/net/dnsclient_unix_test.go |
|
71 +++ b/src/net/dnsclient_unix_test.go |
|
72 @@ -40,9 +40,9 @@ func TestDNSTransportFallback(t *testing.T) { |
|
73 testenv.MustHaveExternalNetwork(t) |
|
74 |
|
75 for _, tt := range dnsTransportFallbackTests { |
|
76 - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(tt.timeout)*time.Second) |
|
77 + ctx, cancel := context.WithCancel(context.Background()) |
|
78 defer cancel() |
|
79 - msg, err := exchange(ctx, tt.server, tt.name, tt.qtype) |
|
80 + msg, err := exchange(ctx, tt.server, tt.name, tt.qtype, time.Second) |
|
81 if err != nil { |
|
82 t.Error(err) |
|
83 continue |
|
84 @@ -82,9 +82,9 @@ func TestSpecialDomainName(t *testing.T) { |
|
85 |
|
86 server := "8.8.8.8:53" |
|
87 for _, tt := range specialDomainNameTests { |
|
88 - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) |
|
89 + ctx, cancel := context.WithCancel(context.Background()) |
|
90 defer cancel() |
|
91 - msg, err := exchange(ctx, server, tt.name, tt.qtype) |
|
92 + msg, err := exchange(ctx, server, tt.name, tt.qtype, 3*time.Second) |
|
93 if err != nil { |
|
94 t.Error(err) |
|
95 continue |
|
96 @@ -501,7 +501,7 @@ func TestErrorForOriginalNameWhenSearching(t *testing.T) { |
|
97 d := &fakeDNSDialer{} |
|
98 testHookDNSDialer = func() dnsDialer { return d } |
|
99 |
|
100 - d.rh = func(s string, q *dnsMsg) (*dnsMsg, error) { |
|
101 + d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) { |
|
102 r := &dnsMsg{ |
|
103 dnsMsgHdr: dnsMsgHdr{ |
|
104 id: q.id, |
|
105 @@ -540,14 +540,15 @@ func TestIgnoreLameReferrals(t *testing.T) { |
|
106 } |
|
107 defer conf.teardown() |
|
108 |
|
109 - if err := conf.writeAndUpdate([]string{"nameserver 192.0.2.1", "nameserver 192.0.2.2"}); err != nil { |
|
110 + if err := conf.writeAndUpdate([]string{"nameserver 192.0.2.1", // the one that will give a lame referral |
|
111 + "nameserver 192.0.2.2"}); err != nil { |
|
112 t.Fatal(err) |
|
113 } |
|
114 |
|
115 d := &fakeDNSDialer{} |
|
116 testHookDNSDialer = func() dnsDialer { return d } |
|
117 |
|
118 - d.rh = func(s string, q *dnsMsg) (*dnsMsg, error) { |
|
119 + d.rh = func(s string, q *dnsMsg, _ time.Time) (*dnsMsg, error) { |
|
120 t.Log(s, q) |
|
121 r := &dnsMsg{ |
|
122 dnsMsgHdr: dnsMsgHdr{ |
|
123 @@ -634,28 +635,30 @@ func BenchmarkGoLookupIPWithBrokenNameServer(b *testing.B) { |
|
124 |
|
125 type fakeDNSDialer struct { |
|
126 // reply handler |
|
127 - rh func(s string, q *dnsMsg) (*dnsMsg, error) |
|
128 + rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error) |
|
129 } |
|
130 |
|
131 func (f *fakeDNSDialer) dialDNS(_ context.Context, n, s string) (dnsConn, error) { |
|
132 - return &fakeDNSConn{f.rh, s}, nil |
|
133 + return &fakeDNSConn{f.rh, s, time.Time{}}, nil |
|
134 } |
|
135 |
|
136 type fakeDNSConn struct { |
|
137 - rh func(s string, q *dnsMsg) (*dnsMsg, error) |
|
138 + rh func(s string, q *dnsMsg, t time.Time) (*dnsMsg, error) |
|
139 s string |
|
140 + t time.Time |
|
141 } |
|
142 |
|
143 func (f *fakeDNSConn) Close() error { |
|
144 return nil |
|
145 } |
|
146 |
|
147 -func (f *fakeDNSConn) SetDeadline(time.Time) error { |
|
148 +func (f *fakeDNSConn) SetDeadline(t time.Time) error { |
|
149 + f.t = t |
|
150 return nil |
|
151 } |
|
152 |
|
153 func (f *fakeDNSConn) dnsRoundTrip(q *dnsMsg) (*dnsMsg, error) { |
|
154 - return f.rh(f.s, q) |
|
155 + return f.rh(f.s, q, f.t) |
|
156 } |
|
157 |
|
158 // UDP round-tripper algorithm should ignore invalid DNS responses (issue 13281). |
|
159 @@ -725,3 +728,72 @@ func TestIgnoreDNSForgeries(t *testing.T) { |
|
160 t.Errorf("got address %v, want %v", got, TestAddr) |
|
161 } |
|
162 } |
|
163 + |
|
164 +// Issue 16865. If a name server times out, continue to the next. |
|
165 +func TestRetryTimeout(t *testing.T) { |
|
166 + origTestHookDNSDialer := testHookDNSDialer |
|
167 + defer func() { testHookDNSDialer = origTestHookDNSDialer }() |
|
168 + |
|
169 + conf, err := newResolvConfTest() |
|
170 + if err != nil { |
|
171 + t.Fatal(err) |
|
172 + } |
|
173 + defer conf.teardown() |
|
174 + |
|
175 + if err := conf.writeAndUpdate([]string{"nameserver 192.0.2.1", // the one that will timeout |
|
176 + "nameserver 192.0.2.2"}); err != nil { |
|
177 + t.Fatal(err) |
|
178 + } |
|
179 + |
|
180 + d := &fakeDNSDialer{} |
|
181 + testHookDNSDialer = func() dnsDialer { return d } |
|
182 + |
|
183 + var deadline0 time.Time |
|
184 + |
|
185 + d.rh = func(s string, q *dnsMsg, deadline time.Time) (*dnsMsg, error) { |
|
186 + t.Log(s, q, deadline) |
|
187 + |
|
188 + if deadline.IsZero() { |
|
189 + t.Error("zero deadline") |
|
190 + } |
|
191 + |
|
192 + if s == "192.0.2.1:53" { |
|
193 + deadline0 = deadline |
|
194 + time.Sleep(10 * time.Millisecond) |
|
195 + return nil, errTimeout |
|
196 + } |
|
197 + |
|
198 + if deadline == deadline0 { |
|
199 + t.Error("deadline didn't change") |
|
200 + } |
|
201 + |
|
202 + r := &dnsMsg{ |
|
203 + dnsMsgHdr: dnsMsgHdr{ |
|
204 + id: q.id, |
|
205 + response: true, |
|
206 + recursion_available: true, |
|
207 + }, |
|
208 + question: q.question, |
|
209 + answer: []dnsRR{ |
|
210 + &dnsRR_CNAME{ |
|
211 + Hdr: dnsRR_Header{ |
|
212 + Name: q.question[0].Name, |
|
213 + Rrtype: dnsTypeCNAME, |
|
214 + Class: dnsClassINET, |
|
215 + }, |
|
216 + Cname: "golang.org", |
|
217 + }, |
|
218 + }, |
|
219 + } |
|
220 + return r, nil |
|
221 + } |
|
222 + |
|
223 + _, err = goLookupCNAME(context.Background(), "www.golang.org") |
|
224 + if err != nil { |
|
225 + t.Fatal(err) |
|
226 + } |
|
227 + |
|
228 + if deadline0.IsZero() { |
|
229 + t.Error("deadline0 still zero", deadline0) |
|
230 + } |
|
231 +} |
|
232 -- |
|
233 2.7.4 |
|
234 |