diff --git a/udp_mux.go b/udp_mux.go index b4beba0f..aa628aee 100644 --- a/udp_mux.go +++ b/udp_mux.go @@ -44,8 +44,8 @@ type UDPMuxDefault struct { mu sync.Mutex - // For UDP connection listen at unspecified address - localAddrsForUnspecified []net.Addr + // whether the UDP connection listens on an unspecified address + isUnspecified bool } // UDPMuxParams are parameters for UDPMux. @@ -61,50 +61,24 @@ type UDPMuxParams struct { } // NewUDPMuxDefault creates an implementation of UDPMux. -func NewUDPMuxDefault(params UDPMuxParams) *UDPMuxDefault { //nolint:cyclop +func NewUDPMuxDefault(params UDPMuxParams) *UDPMuxDefault { if params.Logger == nil { params.Logger = logging.NewDefaultLoggerFactory().NewLogger("ice") } - var localAddrsForUnspecified []net.Addr - if udpAddr, ok := params.UDPConn.LocalAddr().(*net.UDPAddr); !ok { //nolint:nestif + var isUnspecified bool + if udpAddr, ok := params.UDPConn.LocalAddr().(*net.UDPAddr); !ok { params.Logger.Errorf("LocalAddr is not a net.UDPAddr, got %T", params.UDPConn.LocalAddr()) } else if ok && udpAddr.IP.IsUnspecified() { // For unspecified addresses, the correct behavior is to return errListenUnspecified, but // it will break the applications that are already using unspecified UDP connection - // with UDPMuxDefault, so print a warn log and create a local address list for mux. + // with UDPMuxDefault, so print a warn log. params.Logger.Warn("UDPMuxDefault should not listening on unspecified address, use NewMultiUDPMuxFromPort instead") - var networks []NetworkType - switch { - case udpAddr.IP.To4() != nil: - networks = []NetworkType{NetworkTypeUDP4} - - case udpAddr.IP.To16() != nil: - networks = []NetworkType{NetworkTypeUDP4, NetworkTypeUDP6} - - default: - params.Logger.Errorf("LocalAddr expected IPV4 or IPV6, got %T", params.UDPConn.LocalAddr()) - } - if len(networks) > 0 { - if params.Net == nil { - var err error - if params.Net, err = stdnet.NewNet(); err != nil { - params.Logger.Errorf("Failed to get create network: %v", err) - } - } - - _, addrs, err := localInterfaces(params.Net, nil, nil, networks, true) - if err == nil { - localAddrsForUnspecified = make([]net.Addr, len(addrs)) - for i, addr := range addrs { - localAddrsForUnspecified[i] = &net.UDPAddr{ - IP: addr.addr.AsSlice(), - Port: udpAddr.Port, - Zone: addr.addr.Zone(), - } - } - } else { - params.Logger.Errorf("Failed to get local interfaces for unspecified addr: %v", err) + isUnspecified = true + if params.Net == nil { + var err error + if params.Net, err = stdnet.NewNet(); err != nil { + params.Logger.Errorf("Failed to create network: %v", err) } } } @@ -122,7 +96,7 @@ func NewUDPMuxDefault(params UDPMuxParams) *UDPMuxDefault { //nolint:cyclop return newBufferHolder(receiveMTU) }, }, - localAddrsForUnspecified: localAddrsForUnspecified, + isUnspecified: isUnspecified, } go mux.connWorker() @@ -137,8 +111,41 @@ func (m *UDPMuxDefault) LocalAddr() net.Addr { // GetListenAddresses returns the list of addresses that this mux is listening on. func (m *UDPMuxDefault) GetListenAddresses() []net.Addr { - if len(m.localAddrsForUnspecified) > 0 { - return m.localAddrsForUnspecified + if m.isUnspecified { + udpAddr, ok := m.params.UDPConn.LocalAddr().(*net.UDPAddr) + if !ok { + m.params.Logger.Errorf("Failed to get local UDP address") + + return []net.Addr{m.LocalAddr()} + } + + var networks []NetworkType + switch { + case udpAddr.IP.To4() != nil: + networks = []NetworkType{NetworkTypeUDP4} + case udpAddr.IP.To16() != nil: + networks = []NetworkType{NetworkTypeUDP4, NetworkTypeUDP6} + default: + return []net.Addr{m.LocalAddr()} + } + + _, addrs, err := localInterfaces(m.params.Net, nil, nil, networks, true) + if err != nil { + m.params.Logger.Errorf("Failed to get local interfaces: %v", err) + + return []net.Addr{m.LocalAddr()} + } + + result := make([]net.Addr, len(addrs)) + for i, addr := range addrs { + result[i] = &net.UDPAddr{ + IP: addr.addr.AsSlice(), + Port: udpAddr.Port, + Zone: addr.addr.Zone(), + } + } + + return result } return []net.Addr{m.LocalAddr()} @@ -150,7 +157,7 @@ func (m *UDPMuxDefault) GetListenAddresses() []net.Addr { // the connection's Close method should be called for each GetConn call to avoid leaks. func (m *UDPMuxDefault) GetConn(ufrag string, addr net.Addr) (net.PacketConn, error) { // don't check addr for mux using unspecified address - if len(m.localAddrsForUnspecified) == 0 && m.params.UDPConnString != addr.String() { + if !m.isUnspecified && m.params.UDPConnString != addr.String() { return nil, errInvalidAddress } diff --git a/udp_mux_multi.go b/udp_mux_multi.go index 04a90b1f..ef62f2fe 100644 --- a/udp_mux_multi.go +++ b/udp_mux_multi.go @@ -90,7 +90,7 @@ func NewMultiUDPMuxFromPort(port int, opts ...UDPMuxFromPortOption) (*MultiUDPMu if params.net == nil { var err error if params.net, err = stdnet.NewNet(); err != nil { - return nil, fmt.Errorf("failed to get create network: %w", err) + return nil, fmt.Errorf("failed to create network: %w", err) } } diff --git a/udp_mux_test.go b/udp_mux_test.go index 92cac496..d01c4615 100644 --- a/udp_mux_test.go +++ b/udp_mux_test.go @@ -19,6 +19,7 @@ import ( "github.com/pion/ice/v4/internal/fakenet" "github.com/pion/logging" "github.com/pion/stun/v3" + "github.com/pion/transport/v4" "github.com/pion/transport/v4/test" "github.com/stretchr/testify/require" ) @@ -906,6 +907,81 @@ func TestUDPMux_GetConn_SharedRefcount(t *testing.T) { } } +type mutableNet struct { + transport.Net + mu sync.Mutex + ifaces []*transport.Interface +} + +func (m *mutableNet) Interfaces() ([]*transport.Interface, error) { + m.mu.Lock() + defer m.mu.Unlock() + + return m.ifaces, nil +} + +func (m *mutableNet) setInterfaces(ifaces []*transport.Interface) { + m.mu.Lock() + defer m.mu.Unlock() + m.ifaces = ifaces +} + +// Verify that that GetListenAddresses is able to reflect interface changes. +func TestUDPMuxDefault_GetListenAddresses_ReflectsInterfaceChanges(t *testing.T) { + defer test.CheckRoutines(t)() + + conn, err := net.ListenUDP(udp, &net.UDPAddr{IP: net.IPv4zero}) + require.NoError(t, err) + defer func() { _ = conn.Close() }() + + udpAddr, ok := conn.LocalAddr().(*net.UDPAddr) + require.True(t, ok) + wantPort := udpAddr.Port + + makeIface := func(name string, ip net.IP) *transport.Interface { + ifc := transport.NewInterface(net.Interface{ + Index: 1, + MTU: 1500, + Name: name, + Flags: net.FlagUp | net.FlagMulticast, + }) + ifc.AddAddress(&net.IPNet{IP: ip, Mask: net.CIDRMask(24, 32)}) + + return ifc + } + + ip1 := net.IPv4(10, 0, 0, 1).To4() + mn := &mutableNet{ifaces: []*transport.Interface{makeIface("eth0", ip1)}} + + mux := NewUDPMuxDefault(UDPMuxParams{ + Logger: nil, + UDPConn: conn, + Net: mn, + }) + require.NotNil(t, mux) + defer func() { _ = mux.Close() }() + + // first call: should expose ip1. + addrs := mux.GetListenAddresses() + require.Len(t, addrs, 1, "expected exactly one listen address for the initial interface") + ua, ok := addrs[0].(*net.UDPAddr) + require.True(t, ok) + require.Equal(t, wantPort, ua.Port) + require.True(t, ua.IP.Equal(ip1), "expected %s, got %s", ip1, ua.IP) + + // simulate a network change: replace ip1 with ip2. + ip2 := net.IPv4(10, 0, 0, 2).To4() + mn.setInterfaces([]*transport.Interface{makeIface("eth1", ip2)}) + + // second call: must reflect the new interface without requiring a restart. + addrs = mux.GetListenAddresses() + require.Len(t, addrs, 1, "expected exactly one listen address after interface change") + ua, ok = addrs[0].(*net.UDPAddr) + require.True(t, ok) + require.Equal(t, wantPort, ua.Port) + require.True(t, ua.IP.Equal(ip2), "GetListenAddresses must not cache: expected %s, got %s", ip2, ua.IP) +} + func TestNewUDPMuxDefault_UnspecifiedAddr_AutoInitNet(t *testing.T) { defer test.CheckRoutines(t)()