feature: xtransport: Expire CachedIPs, split resolve function from Fetch

I selected default ttl when resolving using system to be 86400 / 24h.

As the program can run long time, I think it is relevant to honor TTL
when resolving and caching results.

Change cache internal format from string to net.IP. This should ensure
there is no need to further check validity of value later when using.

Resolve part was too big and had only one purpose. So it is fine
candidate to be own function.

dnscrypt-proxy/serversInfo.go

@@ -328,9 +328,9 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp
 func fetchDoHServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp, isNew bool) (ServerInfo, error) {
 	if len(stamp.ServerAddrStr) > 0 {
 		ipOnly, _ := ExtractHostAndPort(stamp.ServerAddrStr, -1)
-		proxy.xTransport.cachedIPs.Lock()
+		if ip := ParseIP(ipOnly); ip != nil {
-		proxy.xTransport.cachedIPs.cache[stamp.ProviderName] = ipOnly
+			proxy.xTransport.saveCachedIP(stamp.ProviderName, ip, -1*time.Second)
-		proxy.xTransport.cachedIPs.Unlock()
+		}
 	}
 	url := &url.URL{
 		Scheme: "https",

dnscrypt-proxy/xtransport.go

@@ -8,8 +8,8 @@ import (
 	"encoding/base64"
 	"encoding/hex"
 	"errors"
-	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/url"
@@ -29,11 +29,17 @@ const (
 	DefaultFallbackResolver = "9.9.9.9:53"
 	DefaultKeepAlive        = 5 * time.Second
 	DefaultTimeout          = 30 * time.Second
+	SystemResolverTTL       = 24 * time.Hour
 )
 
+type CachedIPItem struct {
+	ip  net.IP
+	ttl time.Time
+}
+
 type CachedIPs struct {
 	sync.RWMutex
-	cache map[string]string
+	cache map[string]*CachedIPItem
 }
 
 type XTransport struct {
@@ -57,7 +63,7 @@ func NewXTransport() *XTransport {
 		panic("DefaultFallbackResolver does not parse")
 	}
 	xTransport := XTransport{
-		cachedIPs:                CachedIPs{cache: make(map[string]string)},
+		cachedIPs:                CachedIPs{cache: make(map[string]*CachedIPItem)},
 		keepAlive:                DefaultKeepAlive,
 		timeout:                  DefaultTimeout,
 		fallbackResolver:         DefaultFallbackResolver,
@@ -75,11 +81,34 @@ func ParseIP(ipStr string) net.IP {
 	return net.ParseIP(strings.TrimRight(strings.TrimLeft(ipStr, "["), "]"))
 }
 
-func (xTransport *XTransport) clearCache() {
+// If ttl < 0, never expire
+// ttl is set always at least xTransport.timeout otherwise
+func (xTransport *XTransport) saveCachedIP(host string, ip net.IP, ttl time.Duration) {
 	xTransport.cachedIPs.Lock()
-	xTransport.cachedIPs.cache = make(map[string]string)
+	item := &CachedIPItem{ip: ip, ttl: time.Time{}}
+	if ttl >= 0 {
+		if ttl < xTransport.timeout {
+			ttl = xTransport.timeout
+		}
+		item.ttl = time.Now().Add(ttl)
+	}
+	xTransport.cachedIPs.cache[host] = item
 	xTransport.cachedIPs.Unlock()
-	dlog.Info("IP cache cleared")
+}
+
+// If expire is true, remove data if expired
+func (xTransport *XTransport) loadCachedIP(host string, expire bool) (net.IP, bool) {
+	xTransport.cachedIPs.Lock()
+	defer xTransport.cachedIPs.Unlock()
+	item, ok := xTransport.cachedIPs.cache[host]
+	if !ok {
+		return nil, false
+	}
+	if expire && !item.ttl.IsZero() && time.Until(item.ttl) < 0 {
+		delete(xTransport.cachedIPs.cache, host)
+		return nil, false
+	}
+	return item.ip, ok
 }
 
 func (xTransport *XTransport) rebuildTransport() {
@@ -99,11 +128,9 @@ func (xTransport *XTransport) rebuildTransport() {
 		DialContext: func(ctx context.Context, network, addrStr string) (net.Conn, error) {
 			host, port := ExtractHostAndPort(addrStr, stamps.DefaultPort)
 			ipOnly := host
-			xTransport.cachedIPs.RLock()
+			cachedIP, ok := xTransport.loadCachedIP(host, false)
-			cachedIP := xTransport.cachedIPs.cache[host]
+			if ok {
-			xTransport.cachedIPs.RUnlock()
+				ipOnly = cachedIP.String()
-			if len(cachedIP) > 0 {
-				ipOnly = cachedIP
 			} else {
 				dlog.Debugf("[%s] IP address was not cached", host)
 			}
@@ -135,47 +162,54 @@ func (xTransport *XTransport) rebuildTransport() {
 	xTransport.transport = transport
 }
 
-func (xTransport *XTransport) resolveUsingSystem(host string) (*string, error) {
+func (xTransport *XTransport) resolveUsingSystem(host string) (ip net.IP, ttl time.Duration, err error) {
-	foundIPs, err := net.LookupHost(host)
+	ttl = SystemResolverTTL
+	var foundIPs []string
+	foundIPs, err = net.LookupHost(host)
 	if err != nil {
-		return nil, err
+		return
 	}
+	ips := make([]net.IP, 0)
 	for _, ip := range foundIPs {
-		foundIP := net.ParseIP(ip)
+		if foundIP := net.ParseIP(ip); foundIP != nil {
-		if foundIP == nil {
-			continue
-		}
 			if xTransport.useIPv4 {
 				if ipv4 := foundIP.To4(); ipv4 != nil {
-				foundIPx := foundIP.String()
+					ips = append(ips, foundIP)
-				return &foundIPx, nil
 				}
 			}
 			if xTransport.useIPv6 {
 				if ipv6 := foundIP.To16(); ipv6 != nil {
-				foundIPx := "[" + foundIP.String() + "]"
+					ips = append(ips, foundIP)
-				return &foundIPx, nil
 				}
 			}
 		}
-	return nil, err
+	}
+	if len(ips) > 0 {
+		ip = ips[rand.Intn(len(ips))]
+	}
+	return
 }
 
-func (xTransport *XTransport) resolveUsingResolver(dnsClient *dns.Client, host string, resolver string) (*string, error) {
+func (xTransport *XTransport) resolveUsingResolver(proto, host string, resolver string) (ip net.IP, ttl time.Duration, err error) {
-	var err error
+	dnsClient := dns.Client{Net: proto}
 	if xTransport.useIPv4 {
 		msg := new(dns.Msg)
 		msg.SetQuestion(dns.Fqdn(host), dns.TypeA)
 		msg.SetEdns0(uint16(MaxDNSPacketSize), true)
 		var in *dns.Msg
-		in, _, err = dnsClient.Exchange(msg, resolver)
+		if in, _, err = dnsClient.Exchange(msg, resolver); err == nil {
-		if err == nil {
+			answers := make([]dns.RR, 0)
 			for _, answer := range in.Answer {
 				if answer.Header().Rrtype == dns.TypeA {
-					foundIP := answer.(*dns.A).A.String()
+					answers = append(answers, answer)
-					return &foundIP, nil
 				}
 			}
+			if len(answers) > 0 {
+				answer := answers[rand.Intn(len(answers))]
+				ip = answer.(*dns.A).A
+				ttl = time.Duration(answer.Header().Ttl) * time.Second
+				return
+			}
 		}
 	}
 	if xTransport.useIPv6 {
@@ -183,17 +217,62 @@ func (xTransport *XTransport) resolveUsingResolver(dnsClient *dns.Client, host s
 		msg.SetQuestion(dns.Fqdn(host), dns.TypeAAAA)
 		msg.SetEdns0(uint16(MaxDNSPacketSize), true)
 		var in *dns.Msg
-		in, _, err = dnsClient.Exchange(msg, resolver)
+		if in, _, err = dnsClient.Exchange(msg, resolver); err == nil {
-		if err == nil {
+			answers := make([]dns.RR, 0)
 			for _, answer := range in.Answer {
 				if answer.Header().Rrtype == dns.TypeAAAA {
-					foundIP := "[" + answer.(*dns.AAAA).AAAA.String() + "]"
+					answers = append(answers, answer)
-					return &foundIP, nil
+				}
+			}
+			if len(answers) > 0 {
+				answer := answers[rand.Intn(len(answers))]
+				ip = answer.(*dns.AAAA).AAAA
+				ttl = time.Duration(answer.Header().Ttl) * time.Second
+				return
 			}
 		}
 	}
+	return
+}
+
+func (xTransport *XTransport) resolveHost(host string) (err error) {
+	if xTransport.proxyDialer != nil || xTransport.httpProxyFunction != nil {
+		return
 	}
-	return nil, err
+	if ParseIP(host) != nil {
+		return
+	}
+	if _, ok := xTransport.loadCachedIP(host, true); ok {
+		return
+	}
+	var foundIP net.IP
+	var ttl time.Duration
+	if !xTransport.ignoreSystemDNS {
+		foundIP, ttl, err = xTransport.resolveUsingSystem(host)
+	}
+	if xTransport.ignoreSystemDNS || err != nil {
+		protos := []string{"udp", "tcp"}
+		if xTransport.mainProto == "tcp" {
+			protos = []string{"tcp", "udp"}
+		}
+		for _, proto := range protos {
+			if err != nil {
+				dlog.Noticef("System DNS configuration not usable yet, exceptionally resolving [%s] using resolver %s[%s]", host, proto, xTransport.fallbackResolver)
+			} else {
+				dlog.Debugf("Resolving [%s] using resolver %s[%s]", host, proto, xTransport.fallbackResolver)
+			}
+			foundIP, ttl, err = xTransport.resolveUsingResolver(proto, host, xTransport.fallbackResolver)
+			if err == nil {
+				break
+			}
+		}
+	}
+	if err != nil {
+		return
+	}
+	xTransport.saveCachedIP(host, foundIP, ttl)
+	dlog.Debugf("[%s] IP address [%s] added to the cache, valid until %v", host, foundIP, ttl)
+	return
 }
 
 func (xTransport *XTransport) Fetch(method string, url *url.URL, accept string, contentType string, body *[]byte, timeout time.Duration, padding *string) (*http.Response, time.Duration, error) {
@@ -223,6 +302,9 @@ func (xTransport *XTransport) Fetch(method string, url *url.URL, accept string,
 	if xTransport.proxyDialer == nil && strings.HasSuffix(host, ".onion") {
 		return nil, 0, errors.New("Onion service is not reachable without Tor")
 	}
+	if err := xTransport.resolveHost(host); err != nil {
+		return nil, 0, err
+	}
 	req := &http.Request{
 		Method: method,
 		URL:    url,
@@ -233,45 +315,6 @@ func (xTransport *XTransport) Fetch(method string, url *url.URL, accept string,
 		req.ContentLength = int64(len(*body))
 		req.Body = ioutil.NopCloser(bytes.NewReader(*body))
 	}
-	var err error
-	resolveByProxy := false
-	if xTransport.proxyDialer != nil || xTransport.httpProxyFunction != nil {
-		resolveByProxy = true
-	}
-	var foundIP *string
-	if !resolveByProxy && ParseIP(host) == nil {
-		xTransport.cachedIPs.RLock()
-		cachedIP := xTransport.cachedIPs.cache[host]
-		xTransport.cachedIPs.RUnlock()
-		if len(cachedIP) > 0 {
-			foundIP = &cachedIP
-		} else {
-			if !xTransport.ignoreSystemDNS {
-				foundIP, err = xTransport.resolveUsingSystem(host)
-			} else {
-				dlog.Debug("Ignoring system DNS")
-			}
-			if xTransport.ignoreSystemDNS || err != nil {
-				if xTransport.ignoreSystemDNS {
-					dlog.Debugf("Resolving [%s] using fallback resolver [%s]", host, xTransport.fallbackResolver)
-				} else {
-					dlog.Noticef("System DNS configuration not usable yet, exceptionally resolving [%s] using fallback resolver [%s]", host, xTransport.fallbackResolver)
-				}
-				dnsClient := dns.Client{Net: xTransport.mainProto}
-				foundIP, err = xTransport.resolveUsingResolver(&dnsClient, host, xTransport.fallbackResolver)
-			}
-			if foundIP == nil {
-				return nil, 0, fmt.Errorf("No IP found for [%s]", host)
-			}
-			if err != nil {
-				return nil, 0, err
-			}
-			xTransport.cachedIPs.Lock()
-			xTransport.cachedIPs.cache[host] = *foundIP
-			xTransport.cachedIPs.Unlock()
-			dlog.Debugf("[%s] IP address [%s] added to the cache", host, *foundIP)
-		}
-	}
 	start := time.Now()
 	resp, err := client.Do(req)
 	rtt := time.Since(start)