Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: dns cache with empty response #1183

Merged
merged 18 commits into from
Aug 10, 2021
Merged

fix: dns cache with empty response #1183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0b2b84c
nil check on []ip length
rurirei Aug 3, 2021
a860aa9
useless
rurirei Aug 4, 2021
e5bdf0d
typo
rurirei Aug 4, 2021
b051509
nil check on returned ips
rurirei Aug 4, 2021
a84d5e5
revert
rurirei Aug 4, 2021
fa74827
check found
rurirei Aug 4, 2021
12f3170
fix the nil check
rurirei Aug 4, 2021
13d03f5
fix the check found
rurirei Aug 4, 2021
a8d4289
revert
rurirei Aug 4, 2021
29e35c0
use non-nil ips
rurirei Aug 4, 2021
c38ff1d
Update nameserver_quic.go
rurirei Aug 4, 2021
409241e
Update nameserver_tcp.go
rurirei Aug 4, 2021
767dfd4
Update nameserver_udp.go
rurirei Aug 4, 2021
d9bf267
Update nameserver_udp.go
rurirei Aug 4, 2021
6cc9948
Update nameserver_tcp.go
rurirei Aug 4, 2021
a379dbd
Update nameserver_udp.go
rurirei Aug 4, 2021
3872172
Update nameserver_udp.go
rurirei Aug 4, 2021
b08d67f
Refine DNS
Loyalsoldier Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/dns/dnscommon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ L:
case dnsmessage.TypeAAAA:
ans, err := parser.AAAAResource()
if err != nil {
newError("failed to parse A record for domain: ", ah.Name).Base(err).WriteToLog()
newError("failed to parse AAAA record for domain: ", ah.Name).Base(err).WriteToLog()
break L
}
ipRecord.IP = append(ipRecord.IP, net.IPAddress(ans.AAAA[:]))
Expand Down
47 changes: 23 additions & 24 deletions app/dns/nameserver_doh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
// thus most of the DOH implementation is copied from udpns.go
type DoHNameServer struct {
sync.RWMutex
ips map[string]record
ips map[string]*record
pub *pubsub.Service
cleanup *task.Periodic
reqID uint32
Expand Down Expand Up @@ -112,7 +112,7 @@ func NewDoHLocalNameServer(url *url.URL) *DoHNameServer {

func baseDOHNameServer(url *url.URL, prefix string) *DoHNameServer {
s := &DoHNameServer{
ips: make(map[string]record),
ips: make(map[string]*record),
pub: pubsub.NewService(),
name: prefix + "//" + url.Host,
dohURL: url.String(),
Expand Down Expand Up @@ -156,7 +156,7 @@ func (s *DoHNameServer) Cleanup() error {
}

if len(s.ips) == 0 {
s.ips = make(map[string]record)
s.ips = make(map[string]*record)
}

return nil
Expand All @@ -166,7 +166,10 @@ func (s *DoHNameServer) updateIP(req *dnsRequest, ipRec *IPRecord) {
elapsed := time.Since(req.start)

s.Lock()
rec := s.ips[req.domain]
rec, found := s.ips[req.domain]
if !found {
rec = &record{}
}
updated := false

switch req.reqType {
Expand All @@ -176,7 +179,7 @@ func (s *DoHNameServer) updateIP(req *dnsRequest, ipRec *IPRecord) {
updated = true
}
case dnsmessage.TypeAAAA:
addr := make([]net.Address, 0)
addr := make([]net.Address, 0, len(ipRec.IP))
for _, ip := range ipRec.IP {
if len(ip.IP()) == net.IPv6len {
addr = append(addr, ip)
Expand Down Expand Up @@ -295,34 +298,30 @@ func (s *DoHNameServer) findIPsForDomain(domain string, option dns_feature.IPOpt
return nil, errRecordNotFound
}

var err4 error
var err6 error
var ips []net.Address
var lastErr error
if option.IPv6Enable && record.AAAA != nil && record.AAAA.RCode == dnsmessage.RCodeSuccess {
aaaa, err := record.AAAA.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, aaaa...)
}

if option.IPv4Enable && record.A != nil && record.A.RCode == dnsmessage.RCodeSuccess {
a, err := record.A.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, a...)
var ip6 []net.Address

switch {
case option.IPv4Enable:
ips, err4 = record.A.getIPs()
fallthrough
case option.IPv6Enable:
ip6, err6 = record.AAAA.getIPs()
ips = append(ips, ip6...)
}

if len(ips) > 0 {
return toNetIP(ips)
}

if lastErr != nil {
return nil, lastErr
if err4 != nil {
return nil, err4
}

if (option.IPv4Enable && record.A != nil) || (option.IPv6Enable && record.AAAA != nil) {
return nil, dns_feature.ErrEmptyResponse
Comment on lines -324 to -325
Copy link
Contributor

@AkinoKaede AkinoKaede Aug 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a bug that might caused by this changes. The program may wait until the context timeout if the records are empty, and return this error:

[Info] app/dns: failed to lookup ip for domain api.twitter.com at server DOH//9.9.9.9 > context deadline exceeded
[Info] proxy/dns: ip query > app/dns: returning nil for domain api.twitter.com > multierr: context deadline exceeded |

c.c. @Loyalsoldier

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's my mistake

if err6 != nil {
return nil, err6
}

return nil, errRecordNotFound
Expand Down
49 changes: 24 additions & 25 deletions app/dns/nameserver_quic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ const handshakeIdleTimeout = time.Second * 8
// QUICNameServer implemented DNS over QUIC
type QUICNameServer struct {
sync.RWMutex
ips map[string]record
ips map[string]*record
pub *pubsub.Service
cleanup *task.Periodic
reqID uint32
name string
destination net.Destination
destination *net.Destination
session quic.Session
}

Expand All @@ -57,10 +57,10 @@ func NewQUICNameServer(url *url.URL) (*QUICNameServer, error) {
dest := net.UDPDestination(net.DomainAddress(url.Hostname()), port)

s := &QUICNameServer{
ips: make(map[string]record),
ips: make(map[string]*record),
pub: pubsub.NewService(),
name: url.String(),
destination: dest,
destination: &dest,
}
s.cleanup = &task.Periodic{
Interval: time.Minute,
Expand Down Expand Up @@ -102,7 +102,7 @@ func (s *QUICNameServer) Cleanup() error {
}

if len(s.ips) == 0 {
s.ips = make(map[string]record)
s.ips = make(map[string]*record)
}

return nil
Expand All @@ -112,7 +112,10 @@ func (s *QUICNameServer) updateIP(req *dnsRequest, ipRec *IPRecord) {
elapsed := time.Since(req.start)

s.Lock()
rec := s.ips[req.domain]
rec, found := s.ips[req.domain]
if !found {
rec = &record{}
}
updated := false

switch req.reqType {
Expand Down Expand Up @@ -232,34 +235,30 @@ func (s *QUICNameServer) findIPsForDomain(domain string, option dns_feature.IPOp
return nil, errRecordNotFound
}

var err4 error
var err6 error
var ips []net.Address
var lastErr error
if option.IPv6Enable && record.AAAA != nil && record.AAAA.RCode == dnsmessage.RCodeSuccess {
aaaa, err := record.AAAA.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, aaaa...)
}

if option.IPv4Enable && record.A != nil && record.A.RCode == dnsmessage.RCodeSuccess {
a, err := record.A.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, a...)
var ip6 []net.Address

switch {
case option.IPv4Enable:
ips, err4 = record.A.getIPs()
fallthrough
case option.IPv6Enable:
ip6, err6 = record.AAAA.getIPs()
ips = append(ips, ip6...)
}

if len(ips) > 0 {
return toNetIP(ips)
}

if lastErr != nil {
return nil, lastErr
if err4 != nil {
return nil, err4
}

if (option.IPv4Enable && record.A != nil) || (option.IPv6Enable && record.AAAA != nil) {
return nil, dns_feature.ErrEmptyResponse
if err6 != nil {
return nil, err6
}

return nil, errRecordNotFound
Expand Down
53 changes: 28 additions & 25 deletions app/dns/nameserver_tcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import (
type TCPNameServer struct {
sync.RWMutex
name string
destination net.Destination
ips map[string]record
destination *net.Destination
ips map[string]*record
pub *pubsub.Service
cleanup *task.Periodic
reqID uint32
Expand All @@ -45,7 +45,7 @@ func NewTCPNameServer(url *url.URL, dispatcher routing.Dispatcher) (*TCPNameServ
}

s.dial = func(ctx context.Context) (net.Conn, error) {
link, err := dispatcher.Dispatch(ctx, s.destination)
link, err := dispatcher.Dispatch(ctx, *s.destination)
if err != nil {
return nil, err
}
Expand All @@ -67,7 +67,7 @@ func NewTCPLocalNameServer(url *url.URL) (*TCPNameServer, error) {
}

s.dial = func(ctx context.Context) (net.Conn, error) {
return internet.DialSystem(ctx, s.destination, nil)
return internet.DialSystem(ctx, *s.destination, nil)
}

return s, nil
Expand All @@ -85,8 +85,8 @@ func baseTCPNameServer(url *url.URL, prefix string) (*TCPNameServer, error) {
dest := net.TCPDestination(net.ParseAddress(url.Hostname()), port)

s := &TCPNameServer{
destination: dest,
ips: make(map[string]record),
destination: &dest,
ips: make(map[string]*record),
pub: pubsub.NewService(),
name: prefix + "//" + dest.NetAddr(),
}
Expand Down Expand Up @@ -130,7 +130,7 @@ func (s *TCPNameServer) Cleanup() error {
}

if len(s.ips) == 0 {
s.ips = make(map[string]record)
s.ips = make(map[string]*record)
}

return nil
Expand All @@ -140,7 +140,10 @@ func (s *TCPNameServer) updateIP(req *dnsRequest, ipRec *IPRecord) {
elapsed := time.Since(req.start)

s.Lock()
rec := s.ips[req.domain]
rec, found := s.ips[req.domain]
if !found {
rec = &record{}
}
updated := false

switch req.reqType {
Expand Down Expand Up @@ -274,30 +277,30 @@ func (s *TCPNameServer) findIPsForDomain(domain string, option dns_feature.IPOpt
return nil, errRecordNotFound
}

var err4 error
var err6 error
var ips []net.Address
var lastErr error
if option.IPv4Enable {
a, err := record.A.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, a...)
}

if option.IPv6Enable {
aaaa, err := record.AAAA.getIPs()
if err != nil {
lastErr = err
}
ips = append(ips, aaaa...)
var ip6 []net.Address

switch {
case option.IPv4Enable:
ips, err4 = record.A.getIPs()
fallthrough
case option.IPv6Enable:
ip6, err6 = record.AAAA.getIPs()
ips = append(ips, ip6...)
}

if len(ips) > 0 {
return toNetIP(ips)
}

if lastErr != nil {
return nil, lastErr
if err4 != nil {
return nil, err4
}

if err6 != nil {
return nil, err6
}

return nil, dns_feature.ErrEmptyResponse
Expand Down
Loading