From 43fee4ef8a2e7270383e401809f64cbd0c6fb755 Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Tue, 30 Jan 2024 16:24:42 +0100 Subject: [PATCH] bridge: Use random for test service to avoid clashes Go test runs tests from different packages in different binaries and these run in parallel. It's possible that the test bridge service in different tests execute at the same time, which would cause failures ("bind address already in use") if the port was not random. --- bridge/net/config.go | 5 +---- bridge/net/config_internal_test.go | 5 ----- bridge/net/network.go | 28 +++++++++++++++++++++++++--- bridge/test/test.go | 4 ++-- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/bridge/net/config.go b/bridge/net/config.go index 0fda6f54..76e91177 100644 --- a/bridge/net/config.go +++ b/bridge/net/config.go @@ -75,13 +75,10 @@ func validateExternalAddress(addr string) (externalAddressPort, error) { if host == "" { return externalAddressPort{}, fmt.Errorf("invalid externalAddress: host cannot be empty") } - port, err := strconv.ParseUint(portStr, 10, 16) + _, err = strconv.ParseUint(portStr, 10, 16) if err != nil { return externalAddressPort{}, fmt.Errorf("invalid externalAddress: %w", err) } - if port == 0 { - return externalAddressPort{}, fmt.Errorf("invalid externalAddress: port cannot be 0") - } _, errIpv4 := gonet.ResolveUDPAddr(UDP4, addr) _, errIpv6 := gonet.ResolveUDPAddr(UDP6, addr) diff --git a/bridge/net/config_internal_test.go b/bridge/net/config_internal_test.go index 9a321777..50812fec 100644 --- a/bridge/net/config_internal_test.go +++ b/bridge/net/config_internal_test.go @@ -105,11 +105,6 @@ func TestConfigValidate(t *testing.T) { config: &Config{ExternalAddresses: []string{":12345"}}, wantErr: true, }, - { - name: "ZeroPortInExternalAddress", - config: &Config{ExternalAddresses: []string{"localhost:0"}}, - wantErr: true, - }, { name: "InvalidPortInExternalAddress", config: &Config{ExternalAddresses: []string{"localhost:invalid"}}, diff --git a/bridge/net/network.go b/bridge/net/network.go index 542a4e59..2e19e16f 100644 --- a/bridge/net/network.go +++ b/bridge/net/network.go @@ -242,11 +242,24 @@ func (s coAPServers) Close() error { return errors.ErrorOrNil() } -func newServers(cfg Config, m *mux.Router) (coAPServers, bool, bool, error) { +func getPortFromAddress(addr gonet.Addr) (string, error) { + udpAddr, ok := addr.(*gonet.UDPAddr) + if ok { + return fmt.Sprintf("%d", udpAddr.Port), nil + } + addrStr := addr.String() + _, port, err := gonet.SplitHostPort(addrStr) + if err != nil { + return "", err + } + return port, nil +} + +func newServers(cfg *Config, m *mux.Router) (coAPServers, bool, bool, error) { servers := make(coAPServers, 0, len(cfg.externalAddressesPort)) hasIPv4 := false hasIPv6 := false - for _, addr := range cfg.externalAddressesPort { + for i, addr := range cfg.externalAddressesPort { var conn *net.UDPConn var err error if addr.network == UDP4 { @@ -261,6 +274,15 @@ func newServers(cfg Config, m *mux.Router) (coAPServers, bool, bool, error) { _ = servers.Close() return nil, false, false, err } + if addr.port == "0" { + port, err := getPortFromAddress(conn.LocalAddr()) + if err != nil { + _ = servers.Close() + return nil, false, false, err + } + cfg.externalAddressesPort[i].port = port + } + if conn != nil { servers = append(servers, coAPServer{ s: udp.NewServer( @@ -304,7 +326,7 @@ func New(cfg Config, handler RequestHandler) (*Net, error) { return nil, err } m := mux.NewRouter() - servers, hasIPv4, hasIPv6, err := newServers(cfg, m) + servers, hasIPv4, hasIPv6, err := newServers(&cfg, m) if err != nil { return nil, err } diff --git a/bridge/test/test.go b/bridge/test/test.go index 5131dac0..491495b4 100644 --- a/bridge/test/test.go +++ b/bridge/test/test.go @@ -29,8 +29,8 @@ import ( const ( BRIDGE_SERVICE_PIID = "f47ac10b-58cc-4372-a567-0e02b2c3d479" - BRIDGE_DEVICE_HOST = "127.0.0.1:15000" - BRIDGE_DEVICE_HOST_IPv6 = "[::1]:15001" + BRIDGE_DEVICE_HOST = "127.0.0.1:0" // 0 means random port + BRIDGE_DEVICE_HOST_IPv6 = "[::1]:0" // 0 means random port ) func MakeConfig(t *testing.T) service.Config {