all: fix or remove remaining net.IP <-> string conversions

heyxkhoa · Jan 18, 2021 · e99595a · e99595a
1 parent 7fd0634
commit e99595a
Show file tree

Hide file tree

Showing 16 changed files with 42 additions and 45 deletions.
diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go
@@ -262,9 +262,6 @@ func parseOptionString(s string) (uint8, []byte) {
 			return 0, nil
 		}
 		val = ip
-		if ip.To4() != nil {
-			val = ip.To4()
-		}
 
 	default:
 		return 0, nil

diff --git a/internal/dhcpd/dhcphttp.go b/internal/dhcpd/dhcphttp.go
@@ -27,7 +27,8 @@ func convertLeases(inputLeases []Lease, includeExpires bool) []map[string]string
 	leases := []map[string]string{}
 	for _, l := range inputLeases {
 		lease := map[string]string{
-			"mac":      l.HWAddr.String(),
+			"mac": l.HWAddr.String(),
+			// TODO(e.burkov): remove conversion while #2509.
 			"ip":       l.IP.String(),
 			"hostname": l.Hostname,
 		}
@@ -70,7 +71,7 @@ func v4JSONToServerConf(j v4ServerConfJSON) V4ServerConf {
 }
 
 type v6ServerConfJSON struct {
-	RangeStart    string `json:"range_start"`
+	RangeStart    net.IP `json:"range_start"`
 	LeaseDuration uint32 `json:"lease_duration"`
 }
 

diff --git a/internal/dhcpd/server.go b/internal/dhcpd/server.go
@@ -79,7 +79,7 @@ type V6ServerConf struct {
 
 	// The first IP address for dynamic leases
 	// The last allowed IP address ends with 0xff byte
-	RangeStart string `yaml:"range_start"`
+	RangeStart net.IP `yaml:"range_start"`
 
 	LeaseDuration uint32 `yaml:"lease_duration"` // in seconds
 

diff --git a/internal/dhcpd/v6.go b/internal/dhcpd/v6.go
@@ -657,7 +657,7 @@ func v6Create(conf V6ServerConf) (DHCPServer, error) {
 		return s, nil
 	}
 
-	s.conf.ipStart = net.ParseIP(conf.RangeStart)
+	s.conf.ipStart = conf.RangeStart
 	if s.conf.ipStart == nil || s.conf.ipStart.To16() == nil {
 		return s, fmt.Errorf("dhcpv6: invalid range-start IP: %s", conf.RangeStart)
 	}

diff --git a/internal/dhcpd/v6_test.go b/internal/dhcpd/v6_test.go
@@ -17,7 +17,7 @@ func notify6(flags uint32) {
 func TestV6StaticLeaseAddRemove(t *testing.T) {
 	conf := V6ServerConf{
 		Enabled:    true,
-		RangeStart: "2001::1",
+		RangeStart: net.ParseIP("2001::1"),
 		notify:     notify6,
 	}
 	s, err := v6Create(conf)
@@ -60,7 +60,7 @@ func TestV6StaticLeaseAddRemove(t *testing.T) {
 func TestV6StaticLeaseAddReplaceDynamic(t *testing.T) {
 	conf := V6ServerConf{
 		Enabled:    true,
-		RangeStart: "2001::1",
+		RangeStart: net.ParseIP("2001::1"),
 		notify:     notify6,
 	}
 	sIface, err := v6Create(conf)
@@ -109,7 +109,7 @@ func TestV6StaticLeaseAddReplaceDynamic(t *testing.T) {
 func TestV6GetLease(t *testing.T) {
 	conf := V6ServerConf{
 		Enabled:    true,
-		RangeStart: "2001::1",
+		RangeStart: net.ParseIP("2001::1"),
 		notify:     notify6,
 	}
 	sIface, err := v6Create(conf)
@@ -169,7 +169,7 @@ func TestV6GetLease(t *testing.T) {
 func TestV6GetDynamicLease(t *testing.T) {
 	conf := V6ServerConf{
 		Enabled:    true,
-		RangeStart: "2001::2",
+		RangeStart: net.ParseIP("2001::2"),
 		notify:     notify6,
 	}
 	sIface, err := v6Create(conf)

diff --git a/internal/dnsforward/util.go b/internal/dnsforward/util.go
@@ -22,7 +22,7 @@ func IPFromAddr(addr net.Addr) (ip net.IP) {
 // IPStringFromAddr extracts IP address from net.Addr.
 // Note: we can't use net.SplitHostPort(a.String()) because of IPv6 zone:
 // https://github.com/AdguardTeam/AdGuardHome/internal/issues/1261
-func IPStringFromAddr(addr net.Addr) (ipstr string) {
+func IPStringFromAddr(addr net.Addr) (ipStr string) {
 	if ip := IPFromAddr(addr); ip != nil {
 		return ip.String()
 	}

diff --git a/internal/home/control.go b/internal/home/control.go
@@ -36,11 +36,12 @@ func httpError(w http.ResponseWriter, code int, format string, args ...interface
 // ---------------
 // dns run control
 // ---------------
-func addDNSAddress(dnsAddresses *[]string, addr string) {
+func addDNSAddress(dnsAddresses *[]string, addr net.IP) {
+	hostport := addr.String()
 	if config.DNS.Port != 53 {
-		addr = fmt.Sprintf("%s:%d", addr, config.DNS.Port)
+		hostport = net.JoinHostPort(hostport, strconv.Itoa(config.DNS.Port))
 	}
-	*dnsAddresses = append(*dnsAddresses, addr)
+	*dnsAddresses = append(*dnsAddresses, hostport)
 }
 
 func handleStatus(w http.ResponseWriter, _ *http.Request) {

diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go
@@ -107,14 +107,14 @@ func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request)
 	}
 
 	if reqData.Web.Port != 0 && reqData.Web.Port != config.BindPort && reqData.Web.Port != config.BetaBindPort {
-		err = util.CheckPortAvailable(reqData.Web.IP.String(), reqData.Web.Port)
+		err = util.CheckPortAvailable(reqData.Web.IP, reqData.Web.Port)
 		if err != nil {
 			respData.Web.Status = fmt.Sprintf("%v", err)
 		}
 	}
 
 	if reqData.DNS.Port != 0 {
-		err = util.CheckPacketPortAvailable(reqData.DNS.IP.String(), reqData.DNS.Port)
+		err = util.CheckPacketPortAvailable(reqData.DNS.IP, reqData.DNS.Port)
 
 		if util.ErrorIsAddrInUse(err) {
 			canAutofix := checkDNSStubListener()
@@ -125,15 +125,15 @@ func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request)
 					log.Error("Couldn't disable DNSStubListener: %s", err)
 				}
 
-				err = util.CheckPacketPortAvailable(reqData.DNS.IP.String(), reqData.DNS.Port)
+				err = util.CheckPacketPortAvailable(reqData.DNS.IP, reqData.DNS.Port)
 				canAutofix = false
 			}
 
 			respData.DNS.CanAutofix = canAutofix
 		}
 
 		if err == nil {
-			err = util.CheckPortAvailable(reqData.DNS.IP.String(), reqData.DNS.Port)
+			err = util.CheckPortAvailable(reqData.DNS.IP, reqData.DNS.Port)
 		}
 
 		if err != nil {
@@ -304,7 +304,7 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) {
 
 	// validate that hosts and ports are bindable
 	if restartHTTP {
-		err = util.CheckPortAvailable(newSettings.Web.IP.String(), newSettings.Web.Port)
+		err = util.CheckPortAvailable(newSettings.Web.IP, newSettings.Web.Port)
 		if err != nil {
 			httpError(w, http.StatusBadRequest, "Impossible to listen on IP:port %s due to %s",
 				net.JoinHostPort(newSettings.Web.IP.String(), strconv.Itoa(newSettings.Web.Port)), err)
@@ -313,13 +313,13 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) {
 
 	}
 
-	err = util.CheckPacketPortAvailable(newSettings.DNS.IP.String(), newSettings.DNS.Port)
+	err = util.CheckPacketPortAvailable(newSettings.DNS.IP, newSettings.DNS.Port)
 	if err != nil {
 		httpError(w, http.StatusBadRequest, "%s", err)
 		return
 	}
 
-	err = util.CheckPortAvailable(newSettings.DNS.IP.String(), newSettings.DNS.Port)
+	err = util.CheckPortAvailable(newSettings.DNS.IP, newSettings.DNS.Port)
 	if err != nil {
 		httpError(w, http.StatusBadRequest, "%s", err)
 		return

diff --git a/internal/home/dns.go b/internal/home/dns.go
@@ -252,11 +252,11 @@ func getDNSAddresses() []string {
 
 		for _, iface := range ifaces {
 			for _, addr := range iface.Addresses {
-				addDNSAddress(&dnsAddresses, addr.String())
+				addDNSAddress(&dnsAddresses, addr)
 			}
 		}
 	} else {
-		addDNSAddress(&dnsAddresses, config.DNS.BindHost.String())
+		addDNSAddress(&dnsAddresses, config.DNS.BindHost)
 	}
 
 	dnsEncryption := getDNSEncryption()
@@ -326,14 +326,12 @@ func startDNSServer() error {
 	Context.queryLog.Start()
 
 	const topClientsNumber = 100 // the number of clients to get
-	topClients := Context.stats.GetTopClientsIP(topClientsNumber)
-	for _, ip := range topClients {
-		ipAddr := net.ParseIP(ip)
-		if !ipAddr.IsLoopback() {
-			Context.rdns.Begin(ipAddr)
+	for _, ip := range Context.stats.GetTopClientsIP(topClientsNumber) {
+		if !ip.IsLoopback() {
+			Context.rdns.Begin(ip)
 		}
-		if !Context.ipDetector.detectSpecialNetwork(ipAddr) {
-			Context.whois.Begin(ipAddr)
+		if !Context.ipDetector.detectSpecialNetwork(ip) {
+			Context.whois.Begin(ip)
 		}
 	}
 

diff --git a/internal/home/rdns.go b/internal/home/rdns.go
@@ -44,7 +44,7 @@ func InitRDNS(dnsServer *dnsforward.Server, clients *clientsContainer) *RDNS {
 // Begin - add IP address to rDNS queue
 func (r *RDNS) Begin(ip net.IP) {
 	now := uint64(time.Now().Unix())
-	expire := r.ipAddrs.Get([]byte(ip))
+	expire := r.ipAddrs.Get(ip)
 	if len(expire) != 0 {
 		exp := binary.BigEndian.Uint64(expire)
 		if exp > now {
@@ -55,7 +55,7 @@ func (r *RDNS) Begin(ip net.IP) {
 	expire = make([]byte, 8)
 	const ttl = 1 * 60 * 60
 	binary.BigEndian.PutUint64(expire, now+ttl)
-	_ = r.ipAddrs.Set([]byte(ip), expire)
+	_ = r.ipAddrs.Set(ip, expire)
 
 	if r.clients.Exists(ip, ClientSourceRDNS) {
 		return

diff --git a/internal/home/web.go b/internal/home/web.go
@@ -114,7 +114,7 @@ func WebCheckPortAvailable(port int) bool {
 		alreadyRunning = true
 	}
 	if !alreadyRunning {
-		err := util.CheckPortAvailable(config.BindHost.String(), port)
+		err := util.CheckPortAvailable(config.BindHost, port)
 		if err != nil {
 			return false
 		}

diff --git a/internal/querylog/json.go b/internal/querylog/json.go
@@ -150,9 +150,9 @@ func answerToMap(a *dns.Msg) (answers []jobject) {
 		// try most common record types
 		switch v := k.(type) {
 		case *dns.A:
-			answer["value"] = v.A.String()
+			answer["value"] = v.A
 		case *dns.AAAA:
-			answer["value"] = v.AAAA.String()
+			answer["value"] = v.AAAA
 		case *dns.MX:
 			answer["value"] = fmt.Sprintf("%v %v", v.Preference, v.Mx)
 		case *dns.CNAME:

diff --git a/internal/stats/stats.go b/internal/stats/stats.go
@@ -48,7 +48,7 @@ type Stats interface {
 	Update(e Entry)
 
 	// Get IP addresses of the clients with the most number of requests
-	GetTopClientsIP(limit uint) []string
+	GetTopClientsIP(limit uint) []net.IP
 
 	// WriteDiskConfig - write configuration
 	WriteDiskConfig(dc *DiskConfig)

diff --git a/internal/stats/stats_test.go b/internal/stats/stats_test.go
@@ -80,7 +80,7 @@ func TestStats(t *testing.T) {
 	assert.EqualValues(t, 0.123456, d["avg_processing_time"].(float64))
 
 	topClients := s.GetTopClientsIP(2)
-	assert.Equal(t, "127.0.0.1", topClients[0])
+	assert.Equal(t, "127.0.0.1", topClients[0].String())
 
 	s.clear()
 	s.Close()

diff --git a/internal/stats/unit.go b/internal/stats/unit.go
@@ -655,7 +655,7 @@ func (s *statsCtx) getData() map[string]interface{} {
 	return d
 }
 
-func (s *statsCtx) GetTopClientsIP(maxCount uint) []string {
+func (s *statsCtx) GetTopClientsIP(maxCount uint) []net.IP {
 	units, _ := s.loadUnits(s.conf.limit)
 	if units == nil {
 		return nil
@@ -669,9 +669,9 @@ func (s *statsCtx) GetTopClientsIP(maxCount uint) []string {
 		}
 	}
 	a := convertMapToArray(m, int(maxCount))
-	d := []string{}
+	d := []net.IP{}
 	for _, it := range a {
-		d = append(d, it.Name)
+		d = append(d, net.ParseIP(it.Name))
 	}
 	return d
 }
diff --git a/internal/util/network.go b/internal/util/network.go
@@ -128,8 +128,8 @@ func GetSubnet(ifaceName string) net.IPNet {
 }
 
 // CheckPortAvailable - check if TCP port is available
-func CheckPortAvailable(host string, port int) error {
-	ln, err := net.Listen("tcp", net.JoinHostPort(host, strconv.Itoa(port)))
+func CheckPortAvailable(host net.IP, port int) error {
+	ln, err := net.Listen("tcp", net.JoinHostPort(host.String(), strconv.Itoa(port)))
 	if err != nil {
 		return err
 	}
@@ -142,8 +142,8 @@ func CheckPortAvailable(host string, port int) error {
 }
 
 // CheckPacketPortAvailable - check if UDP port is available
-func CheckPacketPortAvailable(host string, port int) error {
-	ln, err := net.ListenPacket("udp", net.JoinHostPort(host, strconv.Itoa(port)))
+func CheckPacketPortAvailable(host net.IP, port int) error {
+	ln, err := net.ListenPacket("udp", net.JoinHostPort(host.String(), strconv.Itoa(port)))
 	if err != nil {
 		return err
 	}