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.

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

rfc: wip: switch to netip for go 1.18 and v0.109.0 #4759

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 21 additions & 19 deletions internal/aghnet/net.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"net"
"net/netip"
"syscall"

"github.com/AdguardTeam/AdGuardHome/internal/aghos"
Expand Down Expand Up @@ -78,9 +79,9 @@ func CanBindPrivilegedPorts() (can bool, err error) {
// NetInterface represents an entry of network interfaces map.
type NetInterface struct {
// Addresses are the network interface addresses.
Addresses []net.IP `json:"ip_addresses,omitempty"`
Addresses []netip.Addr `json:"ip_addresses,omitempty"`
// Subnets are the IP networks for this network interface.
Subnets []*net.IPNet `json:"-"`
Subnets []*netip.Prefix `json:"-"`
Name string `json:"name"`
HardwareAddr net.HardwareAddr `json:"hardware_address"`
Flags net.Flags `json:"flags"`
Expand Down Expand Up @@ -130,19 +131,19 @@ func GetValidNetInterfacesForWeb() (netIfaces []*NetInterface, err error) {

// Collect network interface addresses.
for _, addr := range addrs {
ipNet, ok := addr.(*net.IPNet)
if !ok {
// Should be net.IPNet, this is weird.
return nil, fmt.Errorf("got %s that is not net.IPNet, it is %T", addr, addr)
ip, err := netip.ParsePrefix(addr.String())
if err != nil {
// This should always work
return nil, err
}

// Ignore link-local.
if ipNet.IP.IsLinkLocalUnicast() {
if ip.Addr().IsLinkLocalUnicast() {
continue
}

netIface.Addresses = append(netIface.Addresses, ipNet.IP)
netIface.Subnets = append(netIface.Subnets, ipNet)
netIface.Addresses = append(netIface.Addresses, ip.Addr())
netIface.Subnets = append(netIface.Subnets, &ip)
}

// Discard interfaces with no addresses.
Expand All @@ -157,15 +158,15 @@ func GetValidNetInterfacesForWeb() (netIfaces []*NetInterface, err error) {
// GetInterfaceByIP returns the name of interface containing provided ip.
//
// TODO(e.burkov): See TODO on GetValidInterfacesForWeb.
func GetInterfaceByIP(ip net.IP) string {
func GetInterfaceByIP(ip netip.Addr) string {
ifaces, err := GetValidNetInterfacesForWeb()
if err != nil {
return ""
}

for _, iface := range ifaces {
for _, addr := range iface.Addresses {
if ip.Equal(addr) {
if ip == addr {
return iface.Name
}
}
Expand All @@ -178,7 +179,7 @@ func GetInterfaceByIP(ip net.IP) string {
// the search fails.
//
// TODO(e.burkov): See TODO on GetValidInterfacesForWeb.
func GetSubnet(ifaceName string) *net.IPNet {
func GetSubnet(ifaceName string) *netip.Prefix {
netIfaces, err := GetValidNetInterfacesForWeb()
if err != nil {
log.Error("Could not get network interfaces info: %v", err)
Expand All @@ -195,15 +196,16 @@ func GetSubnet(ifaceName string) *net.IPNet {
}

// CheckPort checks if the port is available for binding. network is expected
// to be one of "udp" and "tcp".
func CheckPort(network string, ip net.IP, port int) (err error) {
// to be one of "udp", "udp6", "tcp" or "tcp6".
func CheckPort(network string, ip netip.Addr, port int) (err error) {
var c io.Closer
addr := netutil.IPPort{IP: ip, Port: port}.String()
addr := netip.AddrPortFrom(ip, uint16(port))

switch network {
case "tcp":
c, err = net.Listen(network, addr)
case "udp":
c, err = net.ListenPacket(network, addr)
case "tcp", "tcp6":
c, err = net.Listen(network, addr.String())
case "udp", "udp6":
c, err = net.ListenPacket(network, addr.String())
default:
return nil
}
Expand Down
7 changes: 4 additions & 3 deletions internal/aghnet/net_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"net"
"net/netip"
"os"
"strings"

Expand Down Expand Up @@ -141,7 +142,7 @@ func findIfaceLine(s *bufio.Scanner, name string) (ok bool) {
// interface through dhcpcd.conf.
func ifaceSetStaticIP(ifaceName string) (err error) {
ipNet := GetSubnet(ifaceName)
if ipNet.IP == nil {
if !ipNet.IsValid() {
return errors.Error("can't get IP address")
}

Expand All @@ -164,7 +165,7 @@ func ifaceSetStaticIP(ifaceName string) (err error) {

// dhcpcdConfIface returns configuration lines for the dhcpdc.conf files that
// configure the interface to have a static IP.
func dhcpcdConfIface(ifaceName string, ipNet *net.IPNet, gwIP net.IP) (conf string) {
func dhcpcdConfIface(ifaceName string, ipNet *netip.Prefix, gwIP net.IP) (conf string) {
b := &strings.Builder{}
stringutil.WriteToBuilder(
b,
Expand All @@ -181,7 +182,7 @@ func dhcpcdConfIface(ifaceName string, ipNet *net.IPNet, gwIP net.IP) (conf stri
stringutil.WriteToBuilder(b, "static routers=", gwIP.String(), "\n")
}

stringutil.WriteToBuilder(b, "static domain_name_servers=", ipNet.IP.String(), "\n\n")
stringutil.WriteToBuilder(b, "static domain_name_servers=", ipNet.Addr().String(), "\n\n")

return b.String()
}
43 changes: 20 additions & 23 deletions internal/aghnet/net_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/fs"
"net"
"net/netip"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -200,18 +201,19 @@ func TestBroadcastFromIPNet(t *testing.T) {
func TestCheckPort(t *testing.T) {
t.Run("tcp_bound", func(t *testing.T) {
l, err := net.Listen("tcp", "127.0.0.1:")

require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, l.Close)

ipp := netutil.IPPortFromAddr(l.Addr())
require.NotNil(t, ipp)
require.NotNil(t, ipp.IP)
require.NotZero(t, ipp.Port)
ip, err := netip.ParseAddrPort(l.Addr().String())
require.Nil(t, err)
require.NotNil(t, ip)
require.True(t, ip.IsValid())
require.NotZero(t, ip.Port())

err = CheckPort("tcp", ipp.IP, ipp.Port)
err = CheckPort("tcp", ip.Addr(), int(ip.Port()))
target := &net.OpError{}
require.ErrorAs(t, err, &target)

assert.Equal(t, "listen", target.Op)
})

Expand All @@ -220,25 +222,26 @@ func TestCheckPort(t *testing.T) {
require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, conn.Close)

ipp := netutil.IPPortFromAddr(conn.LocalAddr())
require.NotNil(t, ipp)
require.NotNil(t, ipp.IP)
require.NotZero(t, ipp.Port)
ip, err := netip.ParseAddrPort(conn.LocalAddr().String())
require.Nil(t, err)
require.NotNil(t, ip)
require.True(t, ip.IsValid())
require.NotZero(t, ip.Port())

err = CheckPort("udp", ipp.IP, ipp.Port)
err = CheckPort("udp", ip.Addr(), int(ip.Port()))
target := &net.OpError{}
require.ErrorAs(t, err, &target)

assert.Equal(t, "listen", target.Op)
})

t.Run("bad_network", func(t *testing.T) {
err := CheckPort("bad_network", nil, 0)
err := CheckPort("bad_network", netip.Addr{}, 0)
assert.NoError(t, err)
})

t.Run("can_bind", func(t *testing.T) {
err := CheckPort("udp", net.IP{0, 0, 0, 0}, 0)
err := CheckPort("udp", netip.IPv4Unspecified(), 0)
assert.NoError(t, err)
})
}
Expand Down Expand Up @@ -322,18 +325,12 @@ func TestNetInterface_MarshalJSON(t *testing.T) {
`"mtu":1500` +
`}` + "\n"

ip4, ip6 := net.IP{1, 2, 3, 4}, net.IP{0xAA, 0xAA, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}
mask4, mask6 := net.CIDRMask(24, netutil.IPv4BitLen), net.CIDRMask(8, netutil.IPv6BitLen)
ip4, ip6 := netip.AddrFrom4([4]byte{1, 2, 3, 4}), netip.AddrFrom16([16]byte{0xAA, 0xAA, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})
v4Subnet, v6Subnet := netip.PrefixFrom(ip4, 24), netip.PrefixFrom(ip6, 8)

iface := &NetInterface{
Addresses: []net.IP{ip4, ip6},
Subnets: []*net.IPNet{{
IP: ip4.Mask(mask4),
Mask: mask4,
}, {
IP: ip6.Mask(mask6),
Mask: mask6,
}},
Addresses: []netip.Addr{ip4, ip6},
Subnets: []*netip.Prefix{&v4Subnet, &v6Subnet},
Name: "iface0",
HardwareAddr: net.HardwareAddr{0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF},
Flags: net.FlagUp | net.FlagMulticast,
Expand Down
18 changes: 9 additions & 9 deletions internal/home/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package home

import (
"fmt"
"net"
"net/netip"
"os"
"path/filepath"
"sync"
Expand Down Expand Up @@ -65,10 +65,10 @@ type configuration struct {
// It's reset after config is parsed
fileData []byte

BindHost net.IP `yaml:"bind_host"` // BindHost is the IP address of the HTTP server to bind to
BindPort int `yaml:"bind_port"` // BindPort is the port the HTTP server
BetaBindPort int `yaml:"beta_bind_port"` // BetaBindPort is the port for new client
Users []User `yaml:"users"` // Users that can access HTTP server
BindHost netip.Addr `yaml:"bind_host"` // BindHost is the IP address of the HTTP server to bind to
BindPort int `yaml:"bind_port"` // BindPort is the port the HTTP server
BetaBindPort int `yaml:"beta_bind_port"` // BetaBindPort is the port for new client
Users []User `yaml:"users"` // Users that can access HTTP server
// AuthAttempts is the maximum number of failed login attempts a user
// can do before being blocked.
AuthAttempts uint `yaml:"auth_attempts"`
Expand Down Expand Up @@ -108,8 +108,8 @@ type configuration struct {

// field ordering is important -- yaml fields will mirror ordering from here
type dnsConfig struct {
BindHosts []net.IP `yaml:"bind_hosts"`
Port int `yaml:"port"`
BindHosts []netip.Addr `yaml:"bind_hosts"`
Port int `yaml:"port"`

// time interval for statistics (in days)
StatsInterval uint32 `yaml:"statistics_interval"`
Expand Down Expand Up @@ -173,11 +173,11 @@ type tlsConfigSettings struct {
var config = &configuration{
BindPort: 3000,
BetaBindPort: 0,
BindHost: net.IP{0, 0, 0, 0},
BindHost: netip.IPv4Unspecified(),
AuthAttempts: 5,
AuthBlockMin: 15,
DNS: dnsConfig{
BindHosts: []net.IP{{0, 0, 0, 0}},
BindHosts: []netip.Addr{netip.IPv4Unspecified()},
Port: defaultPortDNS,
StatsInterval: 1,
FilteringConfig: dnsforward.FilteringConfig{
Expand Down
8 changes: 4 additions & 4 deletions internal/home/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package home
import (
"encoding/json"
"fmt"
"net"
"net/http"
"net/netip"
"net/url"
"runtime"
"strings"
Expand All @@ -20,7 +20,7 @@ import (

// appendDNSAddrs is a convenient helper for appending a formatted form of DNS
// addresses to a slice of strings.
func appendDNSAddrs(dst []string, addrs ...net.IP) (res []string) {
func appendDNSAddrs(dst []string, addrs ...netip.Addr) (res []string) {
for _, addr := range addrs {
var hostport string
if config.DNS.Port != defaultPortDNS {
Expand All @@ -38,7 +38,7 @@ func appendDNSAddrs(dst []string, addrs ...net.IP) (res []string) {
// appendDNSAddrsWithIfaces formats and appends all DNS addresses from src to
// dst. It also adds the IP addresses of all network interfaces if src contains
// an unspecified IP address.
func appendDNSAddrsWithIfaces(dst []string, src []net.IP) (res []string, err error) {
func appendDNSAddrsWithIfaces(dst []string, src []netip.Addr) (res []string, err error) {
ifacesAdded := false
for _, h := range src {
if !h.IsUnspecified() {
Expand Down Expand Up @@ -71,7 +71,7 @@ func appendDNSAddrsWithIfaces(dst []string, src []net.IP) (res []string, err err
// on, including the addresses on all interfaces in cases of unspecified IPs.
func collectDNSAddresses() (addrs []string, err error) {
if hosts := config.DNS.BindHosts; len(hosts) == 0 {
addrs = appendDNSAddrs(addrs, net.IP{127, 0, 0, 1})
addrs = appendDNSAddrs(addrs, netip.AddrFrom4([4]byte{127, 0, 0, 1}))
} else {
addrs, err = appendDNSAddrsWithIfaces(addrs, hosts)
if err != nil {
Expand Down
28 changes: 14 additions & 14 deletions internal/home/controlinstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"encoding/json"
"fmt"
"io"
"net"
"net/http"
"net/netip"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -75,9 +75,9 @@ func (web *Web) handleInstallGetAddresses(w http.ResponseWriter, r *http.Request
}

type checkConfReqEnt struct {
IP net.IP `json:"ip"`
Port int `json:"port"`
Autofix bool `json:"autofix"`
IP netip.Addr `json:"ip"`
Port int `json:"port"`
Autofix bool `json:"autofix"`
}

type checkConfReq struct {
Expand Down Expand Up @@ -209,7 +209,7 @@ func (web *Web) handleInstallCheckConfig(w http.ResponseWriter, r *http.Request)
// handleStaticIP - handles static IP request
// It either checks if we have a static IP
// Or if set=true, it tries to set it
func handleStaticIP(ip net.IP, set bool) staticIPJSON {
func handleStaticIP(ip netip.Addr, set bool) staticIPJSON {
resp := staticIPJSON{}

interfaceName := aghnet.GetInterfaceByIP(ip)
Expand Down Expand Up @@ -317,8 +317,8 @@ func disableDNSStubListener() error {
}

type applyConfigReqEnt struct {
IP net.IP `json:"ip"`
Port int `json:"port"`
IP netip.Addr `json:"ip"`
Port int `json:"port"`
}

type applyConfigReq struct {
Expand Down Expand Up @@ -404,7 +404,7 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) {
Context.firstRun = false
config.BindHost = req.Web.IP
config.BindPort = req.Web.Port
config.DNS.BindHosts = []net.IP{req.DNS.IP}
config.DNS.BindHosts = []netip.Addr{req.DNS.IP}
config.DNS.Port = req.DNS.Port

// TODO(e.burkov): StartMods() should be put in a separate goroutine at the
Expand Down Expand Up @@ -477,7 +477,7 @@ func decodeApplyConfigReq(r io.Reader) (req *applyConfigReq, restartHTTP bool, e
return nil, false, errors.Error("ports cannot be 0")
}

restartHTTP = !config.BindHost.Equal(req.Web.IP) || config.BindPort != req.Web.Port
restartHTTP = config.BindHost != req.Web.IP || config.BindPort != req.Web.Port
if restartHTTP {
err = aghnet.CheckPort("tcp", req.Web.IP, req.Web.Port)
if err != nil {
Expand Down Expand Up @@ -505,9 +505,9 @@ func (web *Web) registerInstallHandlers() {
// TODO(e.burkov): This should removed with the API v1 when the appropriate
// functionality will appear in default checkConfigReqEnt.
type checkConfigReqEntBeta struct {
IP []net.IP `json:"ip"`
Port int `json:"port"`
Autofix bool `json:"autofix"`
IP []netip.Addr `json:"ip"`
Port int `json:"port"`
Autofix bool `json:"autofix"`
}

// checkConfigReqBeta is a struct representing new client's config check request
Expand Down Expand Up @@ -582,8 +582,8 @@ func (web *Web) handleInstallCheckConfigBeta(w http.ResponseWriter, r *http.Requ
// TODO(e.burkov): This should removed with the API v1 when the appropriate
// functionality will appear in default applyConfigReqEnt.
type applyConfigReqEntBeta struct {
IP []net.IP `json:"ip"`
Port int `json:"port"`
IP []netip.Addr `json:"ip"`
Port int `json:"port"`
}

// applyConfigReqBeta is a struct representing new client's config setting
Expand Down
Loading