UPnP/SSDP: Cleaner, easier to test (and actually tested) code

Convert "get own URLs" code into nicer chunks, and test those.

This minimises the code that actually depends on the available network
interfaces, and increases test coverage. Found a few bugs too.

cmd/ssdp_server_poc/main.go

@@ -24,7 +24,7 @@ func main() {
 		panic(err)
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 	defer cancel()
 
 	// Handle Ctrl+C
@@ -38,7 +38,10 @@ func main() {
 		}
 	}()
 
-	urls, err := own_url.AvailableURLs(ctx, "http", ":8080", false)
+	urls, err := own_url.AvailableURLs("http", ":8080")
+	if err != nil {
+		log.Fatal().Err(err).Msg("unable to construct list of URLs")
+	}
 	urlStrings := []string{}
 	for _, url := range urls {
 		urlStrings = append(urlStrings, url.String())

internal/own_url/interfaces.go

@@ -36,8 +36,8 @@ var (
 )
 
 // networkInterfaces returns a list of interface addresses.
-// Only those addresses that can be eached by a unicast TCP/IP connection are returned.
+// Only those addresses that can be reached by a unicast TCP/IP connection are returned.
-func networkInterfaces(includeLinkLocal, includeLocalhost bool) ([]net.IP, error) {
+func networkInterfaces() ([]net.IP, error) {
 	log.Debug().Msg("iterating over all network interfaces")
 
 	interfaces, err := net.Interfaces()

internal/own_url/own_url.go

@@ -2,7 +2,6 @@
 package own_url
 
 import (
-	"context"
 	"fmt"
 	"net"
 	"net/url"
@@ -10,69 +9,79 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-func AvailableURLs(ctx context.Context, schema, listen string, includeLocal bool) ([]*url.URL, error) {
+func AvailableURLs(schema, listen string) ([]url.URL, error) {
-	var (
-		host, port string
-		portnum    int
-		err        error
-	)
-
 	if listen == "" {
 		panic("empty 'listen' parameter")
 	}
 
-	// Figure out which port we're supposted to listen on.
+	hostURL := specificHostURL(schema, listen)
-	if host, port, err = net.SplitHostPort(listen); err != nil {
+	if hostURL != nil {
-		return nil, fmt.Errorf("unable to split host and port in address '%s': %w", listen, err)
+		return []url.URL{*hostURL}, nil
-	}
-	if portnum, err = net.DefaultResolver.LookupPort(ctx, "listen", port); err != nil {
-		return nil, fmt.Errorf("unable to look up port '%s': %w", port, err)
 	}
+	log.Debug().Str("listen", listen).Msg("not listening on any specific host")
 
-	// If the host is empty or ::0/0.0.0.0, show a list of URLs to connect to.
+	addrs, err := networkInterfaces()
-	listenSpecificHost := false
-	var ip net.IP
-	if host != "" {
-		ip = net.ParseIP(host)
-		if ip == nil {
-			addrs, erresolve := net.DefaultResolver.LookupHost(ctx, host)
-			if erresolve != nil {
-				return nil, fmt.Errorf("unable to resolve listen host '%v': %w", host, erresolve)
-			}
-			if len(addrs) > 0 {
-				ip = net.ParseIP(addrs[0])
-			}
-		}
-		if ip != nil && !ip.IsUnspecified() {
-			listenSpecificHost = true
-		}
-	}
-
-	if listenSpecificHost {
-		// We can just construct a URL here, since we know it's a specific host anyway.
-		log.Debug().Str("host", ip.String()).Msg("listening on host")
-
-		link := fmt.Sprintf("%s://%s:%d/", schema, host, portnum)
-		myURL, errparse := url.Parse(link)
-		if errparse != nil {
-			return nil, fmt.Errorf("unable to parse listen URL %s: %w", link, errparse)
-		}
-		return []*url.URL{myURL}, nil
-	}
-
-	log.Debug().Str("host", host).Msg("not listening on any specific host")
-
-	addrs, err := networkInterfaces(false, includeLocal)
-	if err == ErrNoInterface {
-		addrs, err = networkInterfaces(true, includeLocal)
-	}
 	if err != nil {
 		return nil, err
 	}
 
 	log.Debug().Msg("iterating network interfaces to find possible URLs for Flamenco Manager.")
+	return urlsForNetworkInterfaces(schema, listen, addrs)
+}
 
-	links := make([]*url.URL, 0)
+// specificHostURL returns the hosts's URL if the "listen" string is specific enough, otherwise nil.
+// Examples: "192.168.0.1:8080" is specific enough, "0.0.0.0:8080" and ":8080" are not.
+func specificHostURL(scheme, listen string) *url.URL {
+	var (
+		host string
+		err  error
+	)
+
+	// Figure out which port we're supposted to listen on.
+	if host, _, err = net.SplitHostPort(listen); err != nil {
+		// This is annoying. SplitHostPort() doesn't return specific errors, so we
+		// have to test on the error message to see what's the problem.
+		// A missing port is fine, but other errors are not.
+		addrErr := err.(*net.AddrError)
+		if addrErr.Err != "missing port in address" {
+			log.Warn().Str("address", listen).Err(err).Msg("unable to split host and port in address")
+			return nil
+		}
+
+		// 'listen' doesn't have a port number, so it's just the host.
+		host = listen
+	}
+	if host == "" {
+		// An empty host is never specific enough.
+		return nil
+	}
+
+	ip := net.ParseIP(host)
+	if ip != nil && ip.IsUnspecified() {
+		// The host is "::0" or "0.0.0.0"; not specific.
+		return nil
+	}
+
+	// We can just construct a URL here, since we know it's a specific host anyway.
+	return &url.URL{
+		Scheme: scheme,
+		Host:   listen,
+		Path:   "/",
+	}
+}
+
+func urlsForNetworkInterfaces(scheme, listen string, addrs []net.IP) ([]url.URL, error) {
+	// Find the port number in the 'listen' string.
+	var (
+		port string
+		err  error
+	)
+	// Get the port number as integer.
+	if _, port, err = net.SplitHostPort(listen); err != nil {
+		return nil, fmt.Errorf("unable to split host and port in address %q", listen)
+	}
+
+	links := make([]url.URL, 0)
 	for _, addr := range addrs {
 		var strAddr string
 		if ipv4 := addr.To4(); ipv4 != nil {
@@ -81,17 +90,12 @@ func AvailableURLs(ctx context.Context, schema, listen string, includeLocal bool
 			strAddr = fmt.Sprintf("[%s]", addr)
 		}
 
-		constructedURL := fmt.Sprintf("%s://%s:%d/", schema, strAddr, portnum)
+		urlForAddr := url.URL{
-		parsedURL, err := url.Parse(constructedURL)
+			Scheme: scheme,
-		if err != nil {
+			Host:   fmt.Sprintf("%s:%s", strAddr, port),
-			log.Warn().
+			Path:   "/",
-				Str("address", strAddr).
-				Str("url", constructedURL).
-				Err(err).
-				Msg("skipping address, as it results in an unparseable URL")
-			continue
 		}
-		links = append(links, parsedURL)
+		links = append(links, urlForAddr)
 	}
 
 	return links, nil

internal/own_url/own_url_test.go

@@ -2,25 +2,70 @@
 package own_url
 
 import (
-	"context"
+	"net"
 	"testing"
 	"time"
 
 	"github.com/mattn/go-colorable"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestAvailableURLs(t *testing.T) {
 	output := zerolog.ConsoleWriter{Out: colorable.NewColorableStdout(), TimeFormat: time.RFC3339}
 	log.Logger = log.Output(output)
 
-	ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Second)
+	// This should run without errors. It's hard to predict the returned URLs
-	defer ctxCancel()
+	// though, as they depend on the local network devices.
-
+	urls, err := AvailableURLs("http", ":9999")
-	_, err := AvailableURLs(ctx, "http", ":9999", true)
 	if err != nil {
 		t.Fatal(err)
 	}
-	// t.Fatalf("urls: %v", urls)
+	assert.NotEmpty(t, urls, "expected at least one URL to be returned")
+}
+
+func TestSpecificHostURL(t *testing.T) {
+	tests := []struct {
+		name   string
+		expect string // Empty string encodes "expect nil pointer"
+		listen string
+	}{
+		{"Specific IPv4 with port", "http://192.168.0.1:8080/", "192.168.0.1:8080"},
+		{"Specific IPv4 without port", "http://192.168.0.1/", "192.168.0.1"},
+		{"Specific IPv6 with port", "http://[fe80::5054:ff:fede:2ad7]:8080/", "[fe80::5054:ff:fede:2ad7]:8080"},
+		{"Specific IPv6 without port", "http://[fe80::5054:ff:fede:2ad7]/", "[fe80::5054:ff:fede:2ad7]"},
+
+		{"Wildcard IPv4", "", "0.0.0.0:8080"},
+		{"Wildcard IPv6", "", "[::0]:8080"},
+		{"No host, just port", "", ":8080"},
+
+		{"Invalid address", "http://this%20is%20not%20an%20address/", "this is not an address"},
+		{"Invalid port", "", "192.168.0.1::too-many-colons"},
+	}
+
+	for _, test := range tests {
+		actual := specificHostURL("http", test.listen)
+		if test.expect == "" {
+			assert.Nil(t, actual, "for input %q", test.listen)
+			continue
+		}
+		if actual == nil {
+			t.Errorf("returned URL is nil for input %q", test.listen)
+			continue
+		}
+		assert.Equal(t, test.expect, actual.String(), "for input %q", test.listen)
+	}
+}
+
+func TestURLsForNetworkInterfaces(t *testing.T) {
+	addrs := []net.IP{linkLocalIPv6, lanIPv4}
+	urls, err := urlsForNetworkInterfaces("http", ":9999", addrs)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Len(t, urls, 2)
+	assert.Equal(t, "http://[fe80::5054:ff:fede:2ad7]:9999/", urls[0].String())
+	assert.Equal(t, "http://192.168.0.1:9999/", urls[1].String())
 }