Manager: improve the "my own URLs" construction

Improve the "my own URLs" construction, such that:
- IPv6 link-local addresses are always skipped. They require a "zone index"
  string, typically the interface name, so something like
  `[fe80::cafe:f00d%eth0]`. This is not supported by web browsers, so the
  URLs would be of limited use. Furthermore, they require the interface
  name of the side initiating the connection, whereas this code is used to
  answer the question "how can this machine be reached as a server?"
- IPv4 addresses are sorted before IPv6 addresses. Even though I like IPv6
  a lot, IPv4 is still more familiar to people.
- Loopback addresses (::1, 127.0.0.1) are sorted last, so that the First-
  Time Wizard is most likely to use the bigger-scoped address.

internal/own_url/interfaces.go

@@ -23,9 +23,11 @@ package own_url
  */
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"net"
+	"sort"
 
 	"github.com/rs/zerolog/log"
 )
@@ -83,7 +85,7 @@ func networkInterfaces() ([]net.IP, error) {
 			case ip.IsUnspecified():
 				logger.Trace().Msg("    - skipping unspecified")
 			default:
-				logger.Trace().Msg("    - usable")
+				logger.Trace().Msg("    - potentially usable")
 				ifaceAddresses = append(ifaceAddresses, ip)
 			}
 		}
@@ -95,35 +97,56 @@ func networkInterfaces() ([]net.IP, error) {
 		return usableAddresses, ErrNoInterface
 	}
 
+	sort.Slice(usableAddresses, func(i, j int) bool {
+		// Sort loopback addresses after others.
+		if usableAddresses[i].IsLoopback() != usableAddresses[j].IsLoopback() {
+			return usableAddresses[j].IsLoopback()
+		}
+		// Sort IPv4 before IPv6, because people are likely to be more familiar with
+		// them.
+		if isIPv4(usableAddresses[i]) != isIPv4(usableAddresses[j]) {
+			return isIPv4(usableAddresses[i])
+		}
+		// Otherwise just order lexicographically.
+		return bytes.Compare(usableAddresses[i], usableAddresses[j]) < 0
+	})
+
 	return usableAddresses, nil
 }
 
-// filterAddresses reduces the number of IPv6 addresses.
-// It prefers link-local addresses; if these are in the list, all the other IPv6
-// addresses will be removed. Link-local addresses are stable and meant for
-// same-network connections, which is exactly what Flamenco needs.
-// Loopback addresses (localhost) are always filtered out, unless they're the only addresses available.
+// filterAddresses reduces the number of IP addresses.
+//
+// The function prefers non-link-local addresses over link-local ones.
+// Link-local addresses are stable and meant for same-network connections, but
+// they require a "zone index", typically the interface name, so something like
+// `[fe80::cafe:f00d%eth0]`. This is not supported by webbrowsers. Furthermore,
+// they require the interface name of the side initiating the connection,
+// whereas this code is used to answer the question "how can this machine be
+// reached?".
+//
+// Source: https://stackoverflow.com/a/52972417/875379
+//
+// Loopback addresses (localhost) are always filtered out, unless they're the
+// only addresses available.
 func filterAddresses(addrs []net.IP) []net.IP {
 	keepAddrs := make([]net.IP, 0)
+	keepLinkLocalv4 := hasOnlyLinkLocalv4(addrs)
 
-	if hasOnlyLoopback(addrs) {
-		return addrs
-	}
-
-	var keepLinkLocalv6 = hasLinkLocalv6(addrs)
-	var keepLinkLocalv4 = hasLinkLocalv4(addrs)
-
-	var keep bool
 	for _, addr := range addrs {
 		if addr.IsLoopback() {
 			continue
 		}
 
 		isv4 := isIPv4(addr)
+
+		var keep bool
 		if isv4 {
 			keep = keepLinkLocalv4 == addr.IsLinkLocalUnicast()
 		} else {
-			keep = keepLinkLocalv6 == addr.IsLinkLocalUnicast()
+			// Never keep IPv6 link-local addresses. They need a "zone index" to work,
+			// and those can only be determined on the connecting side. Furthermore,
+			// they're incompatible with most webbrowsers.
+			keep = !addr.IsLinkLocalUnicast()
 		}
 
 		if keep {
@@ -131,6 +154,19 @@ func filterAddresses(addrs []net.IP) []net.IP {
 		}
 	}
 
+	// Only when after the filtering there is nothing left, add the loopback
+	// addresses. This is likely a bit of a strange test, because either this is a
+	// loopback device (and should only have loopback addresses) or it is not (and
+	// should only have non-loopback addresses). It does make the code reliable
+	// even when things are mixed, which is nice.
+	if len(keepAddrs) == 0 {
+		for _, addr := range addrs {
+			if addr.IsLoopback() {
+				keepAddrs = append(keepAddrs, addr)
+			}
+		}
+	}
+
 	return keepAddrs
 }
 
@@ -138,29 +174,17 @@ func isIPv4(addr net.IP) bool {
 	return addr.To4() != nil
 }
 
-func hasLinkLocalv6(addrs []net.IP) bool {
+func hasOnlyLinkLocalv4(addrs []net.IP) bool {
+	hasLinkLocalv4 := false
 	for _, addr := range addrs {
-		if !isIPv4(addr) && addr.IsLinkLocalUnicast() {
-			return true
+		// Only consider non-loopback IPv4 addresses.
+		if addr.IsLoopback() || !isIPv4(addr) {
+			continue
 		}
-	}
-	return false
-}
-
-func hasLinkLocalv4(addrs []net.IP) bool {
-	for _, addr := range addrs {
-		if isIPv4(addr) && addr.IsLinkLocalUnicast() {
-			return true
-		}
-	}
-	return false
-}
-
-func hasOnlyLoopback(addrs []net.IP) bool {
-	for _, addr := range addrs {
-		if !addr.IsLoopback() {
+		if !addr.IsLinkLocalUnicast() {
 			return false
 		}
+		hasLinkLocalv4 = true
 	}
-	return true
+	return hasLinkLocalv4
 }

internal/own_url/interfaces_test.go

@@ -32,10 +32,14 @@ func Test_filterAddresses(t *testing.T) {
 		{"IPv6 without link-local",
 			[]net.IP{globalIPv6, lanIPv6},
 			[]net.IP{globalIPv6, lanIPv6, localhostIPv6}},
-		// Link-local address present, just use that one.
+		// In a mix, only the global address should be used.
 		{"IPv6 with link-local",
-			[]net.IP{linkLocalIPv6},
-			[]net.IP{linkLocalIPv6, lanIPv6, localhostIPv6}},
+			[]net.IP{globalIPv6},
+			[]net.IP{linkLocalIPv6, globalIPv6, localhostIPv6}},
+		// Only loopback and link-local.
+		{"IPv6 with link-local + loopback",
+			[]net.IP{localhostIPv6},
+			[]net.IP{localhostIPv6, linkLocalIPv6}},
 		// Only loopback
 		{"IPv6 with only loopback",
 			[]net.IP{localhostIPv6},
@@ -46,10 +50,14 @@ func Test_filterAddresses(t *testing.T) {
 		{"IPv4 without link-local",
 			[]net.IP{globalIPv4, lanIPv4},
 			[]net.IP{globalIPv4, lanIPv4, localhostIPv4}},
-		// Link-local address present, just use that one.
+		// In a mix, only the global and lan addresses should be used.
 		{"IPv4 with link-local",
+			[]net.IP{globalIPv4, lanIPv4},
+			[]net.IP{globalIPv4, linkLocalIPv4, lanIPv4, localhostIPv4}},
+		// Only loopback and link-local.
+		{"IPv4 with link-local + loopback",
 			[]net.IP{linkLocalIPv4},
-			[]net.IP{linkLocalIPv4, lanIPv4, localhostIPv4}},
+			[]net.IP{localhostIPv4, linkLocalIPv4}},
 		// Only loopback
 		{"IPv4 with only loopback",
 			[]net.IP{localhostIPv4},
@@ -58,11 +66,11 @@ func Test_filterAddresses(t *testing.T) {
 		// Mixed IPv4/IPv6 tests:
 		// IPv4 no link-local, but IPv6 with link-local:
 		{"IPv4 w/o, IPv6 w/ link-local",
-			[]net.IP{lanIPv4, linkLocalIPv6},
+			[]net.IP{lanIPv4, lanIPv6},
 			[]net.IP{lanIPv4, localhostIPv4, lanIPv6, linkLocalIPv6}},
 		// IPv4 link-local, IPv6 without:
 		{"IPv4 w/, IPv4 w/o link-local",
-			[]net.IP{linkLocalIPv4, lanIPv6},
+			[]net.IP{lanIPv4, lanIPv6},
 			[]net.IP{linkLocalIPv4, lanIPv4, lanIPv6}},
 		// Only loopback
 		{"IPv4 + IPv6 with only loopback",