Skip to content

Commit b56d985

Browse files
committed
dnsx/resolver: unset errs if answer non-empty
In some cases, the transports both error out (EOF) and send a response; and in such cases (EOF denotes a force close), errors should not override the actual answer, that is, it being overlooked by caching and alg layers. This results in rest of the engine behaving as if these queries were never resolved (neither cached nor mapped) when in fact they were. A wrapper func, dnsx.Req, unset err if returned answer is not empty.
1 parent b7111ee commit b56d985

File tree

5 files changed

+43
-24
lines changed

5 files changed

+43
-24
lines changed

intra/bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func (b *bootstrap) Query(network string, q []byte, summary *x.DNSSummary) ([]by
199199
tr := b.Transport
200200
if tr != nil {
201201
log.V("dns: default: query %s %d", network, len(q))
202-
return tr.Query(network, q, summary)
202+
return dnsx.Req(tr, network, q, summary)
203203
}
204204
return nil, errDefaultTransportNotReady
205205
}

intra/dnsx/alg.go

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -186,19 +186,17 @@ func (t *dnsgateway) querySecondary(t2 Transport, network string, q []byte, out
186186
ticker.Stop()
187187
return
188188
}
189-
} else { // query secondary to get answer for q
190-
if r, err = t2.Query(network, q, result.summary); err != nil {
191-
log.D("alg: skip; sec transport %s err %v", t2.ID(), err)
192-
return
193-
}
189+
} else {
190+
// query secondary to get answer for q
191+
r, err = Req(t2, network, q, result.summary)
194192
}
195193

196-
if len(r) == 0 {
197-
log.W("alg: skip; no primary or sec ans")
194+
if err != nil {
195+
log.D("alg: skip; sec transport %s err? %v", t2.ID(), err)
198196
return
199197
}
200198

201-
// check if answer r is blocked
199+
// check if answer r is blocked; r is either from t2 or from <-in
202200
if ans2 := xdns.AsMsg(r); ans2 == nil {
203201
// not a valid dns answer
204202
return
@@ -254,16 +252,19 @@ func (t *dnsgateway) q(t1, t2 Transport, preset []*netip.Addr, network string, q
254252
if usepreset {
255253
r, err = synthesizeOrQuery(preset, t1, q, network, innersummary)
256254
} else {
257-
r, err = query(t1, network, q, innersummary)
255+
r, err = Req(t1, network, q, innersummary)
258256
}
259257
resch <- r
260258

261259
// override relevant values in summary
262260
fillSummary(innersummary, summary)
263261

264262
if err != nil {
265-
log.D("alg: abort; qerr %v", err)
266-
return
263+
if len(r) <= 0 {
264+
log.D("alg: abort; r: 0, qerr %v", err)
265+
return
266+
}
267+
log.D("alg: err but r ok; r: %d, qerr %v", len(r), err)
267268
}
268269

269270
ansin := &dns.Msg{}
@@ -824,7 +825,7 @@ func hash48(s string) uint64 {
824825
func synthesizeOrQuery(pre []*netip.Addr, tr Transport, q []byte, network string, smm *x.DNSSummary) ([]byte, error) {
825826
// synthesize a response with the given ips
826827
if len(pre) == 0 {
827-
return query(tr, network, q, smm)
828+
return Req(tr, network, q, smm)
828829
}
829830
msg := xdns.AsMsg(q)
830831
if msg == nil {
@@ -840,7 +841,7 @@ func synthesizeOrQuery(pre []*netip.Addr, tr Transport, q []byte, network string
840841
// if no ips are of the same family as the question xdns.AQuadAForQuery returns error
841842
ans, err := xdns.AQuadAForQuery(msg, unptr(pre)...)
842843
if err != nil { // errors on invalid msg, question, or mismatched ips
843-
return query(tr, network, q, smm)
844+
return Req(tr, network, q, smm)
844845
}
845846
withPresetSummary(smm)
846847
smm.RCode = xdns.Rcode(ans)
@@ -850,7 +851,7 @@ func synthesizeOrQuery(pre []*netip.Addr, tr Transport, q []byte, network string
850851
log.D("alg: synthesize: q(4? %t / 6? %t) rdata(%s)", qname, is4, is6, smm.RData)
851852
return ans.Pack()
852853
} else if isHTTPS || isSVCB {
853-
r, err := tr.Query(network, q, smm)
854+
r, err := Req(tr, network, q, smm)
854855
if err != nil {
855856
return r, err
856857
}
@@ -877,12 +878,29 @@ func synthesizeOrQuery(pre []*netip.Addr, tr Transport, q []byte, network string
877878

878879
return ans.Pack()
879880
} else {
880-
return query(tr, network, q, smm)
881+
return Req(tr, network, q, smm)
881882
}
882883
}
883884

884-
func query(t Transport, network string, q []byte, smm *x.DNSSummary) ([]byte, error) {
885-
return t.Query(network, q, smm)
885+
// Req sends q to transport t and returns the answer, if any;
886+
// errors are unset unless answer is empty;
887+
// smm, the in/out parameter, is dns summary as got from t.
888+
func Req(t Transport, network string, q []byte, smm *x.DNSSummary) ([]byte, error) {
889+
if t == nil {
890+
return nil, errNoSuchTransport
891+
}
892+
if len(q) <= 0 {
893+
return nil, errNoQuestion
894+
}
895+
if smm == nil { // discard smm
896+
discarded := new(x.DNSSummary)
897+
smm = discarded
898+
}
899+
r, err := t.Query(network, q, smm)
900+
if len(r) > 0 {
901+
return r, nil
902+
}
903+
return r, err
886904
}
887905

888906
func splitIPFamilies(ips []*netip.Addr) (ip4s, ip6s []*netip.Addr) {

intra/dnsx/cacher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ func (t *ctransport) fetch(network string, q []byte, msg *dns.Msg, summary *x.DN
310310
fsmm.Type = t.Transport.Type()
311311

312312
v, _ := t.reqbarrier.Do(key, func() (*cres, error) {
313-
ans, qerr := t.Transport.Query(network, q, fsmm)
313+
ans, qerr := Req(t.Transport, network, q, fsmm)
314314
// cb.put no-ops when len(ans) is 0
315315
cb.put(key, ans, fsmm)
316316
// cres.ans may be nil

intra/doh/doh.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,12 @@ func (t *transport) Query(network string, q []byte, smm *x.DNSSummary) (r []byte
483483
}
484484

485485
status := dnsx.Complete
486+
ans := xdns.AsMsg(r)
487+
486488
if qerr != nil {
487489
status = qerr.Status()
488490
err = qerr.Unwrap()
489491
}
490-
ans := xdns.AsMsg(r)
491492
t.status = status
492493

493494
t.est.Add(elapsed.Seconds())

intra/x64/dns64.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (d *dns64) AddResolver(id string, r dnsx.Transport) bool {
102102
d.register(id)
103103

104104
discarded := new(x.DNSSummary)
105-
b, err := r.Query(dnsx.NetTypeUDP, arpa64, discarded)
105+
b, err := dnsx.Req(r, dnsx.NetTypeUDP, arpa64, discarded)
106106
if err != nil {
107107
log.W("dns64: udp: could not query resolver %s", id)
108108
return false
@@ -114,7 +114,7 @@ func (d *dns64) AddResolver(id string, r dnsx.Transport) bool {
114114
return false
115115
} else if ans.Truncated { // should never be the case for DOH, ODOH, DOT
116116
// else if: returned response is truncated dns ans, retry over tcp
117-
b, err = r.Query(dnsx.NetTypeTCP, arpa64, discarded)
117+
b, err = dnsx.Req(r, dnsx.NetTypeTCP, arpa64, discarded)
118118
if err != nil {
119119
log.W("dns64: tcp: could not query resolver %s", id)
120120
return false
@@ -240,7 +240,7 @@ func (d *dns64) query64(msg6 *dns.Msg, r dnsx.Transport) (*dns.Msg, error) {
240240
}
241241

242242
discarded := new(x.DNSSummary)
243-
a4, err := r.Query(dnsx.NetTypeUDP, q4, discarded)
243+
a4, err := dnsx.Req(r, dnsx.NetTypeUDP, q4, discarded)
244244
log.D("dns64: udp: upstream q(%s) / a(%d) / e(%v) / e-not-nil(%t)", xdns.QName(msg4), len(a4), err, err != nil)
245245
if err != nil {
246246
return nil, err
@@ -253,7 +253,7 @@ func (d *dns64) query64(msg6 *dns.Msg, r dnsx.Transport) (*dns.Msg, error) {
253253
return nil, err
254254
} else if res.Truncated { // should never be the case for DOH, ODOH, DOT
255255
// else if: returned response is truncated dns ans, retry over tcp
256-
a4, err = r.Query(dnsx.NetTypeTCP, q4, discarded)
256+
a4, err = dnsx.Req(r, dnsx.NetTypeTCP, q4, discarded)
257257
log.D("dns64: tcp: upstream q(%s) / a(%d) / e(%v) / e-not-nil(%t)", xdns.QName(msg4), len(a4), err, err != nil)
258258
if err != nil {
259259
return nil, err

0 commit comments

Comments
 (0)