CachingMux should return unique addresses only (fixes #2321)

lib/discover/cache.go

@@ -7,6 +7,7 @@
 package discover
 
 import (
+	"sort"
 	stdsync "sync"
 	"time"
 
@@ -110,7 +111,7 @@ func (m *CachingMux) Lookup(deviceID protocol.DeviceID) (direct []string, relays
 		l.Debugln("   ", relays)
 	}
 
-	return direct, relays, nil
+	return uniqueSortedStrings(direct), uniqueSortedRelays(relays), nil
 }
 
 func (m *CachingMux) String() string {
@@ -196,3 +197,49 @@ func (c *cache) Cache() map[protocol.DeviceID]CacheEntry {
 	c.mut.Unlock()
 	return m
 }
+
+func uniqueSortedStrings(ss []string) []string {
+	m := make(map[string]struct{}, len(ss))
+	for _, s := range ss {
+		m[s] = struct{}{}
+	}
+
+	var us = make([]string, 0, len(m))
+	for k := range m {
+		us = append(us, k)
+	}
+
+	sort.Strings(us)
+
+	return us
+}
+
+func uniqueSortedRelays(rs []Relay) []Relay {
+	m := make(map[string]Relay, len(rs))
+	for _, r := range rs {
+		m[r.URL] = r
+	}
+
+	var ur = make([]Relay, 0, len(m))
+	for _, r := range m {
+		ur = append(ur, r)
+	}
+
+	sort.Sort(relayList(ur))
+
+	return ur
+}
+
+type relayList []Relay
+
+func (l relayList) Len() int {
+	return len(l)
+}
+
+func (l relayList) Swap(a, b int) {
+	l[a], l[b] = l[b], l[a]
+}
+
+func (l relayList) Less(a, b int) bool {
+	return l[a].URL < l[b].URL
+}

lib/discover/cache_test.go

@@ -0,0 +1,73 @@
+package discover
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/syncthing/syncthing/lib/protocol"
+)
+
+func TestCacheUnique(t *testing.T) {
+	direct := []string{"tcp://192.0.2.42:22000", "tcp://192.0.2.43:22000"}
+	relays := []Relay{{URL: "relay://192.0.2.44:443"}, {URL: "tcp://192.0.2.45:443"}}
+
+	c := NewCachingMux()
+	c.ServeBackground()
+	defer c.Stop()
+
+	// Add a fake discovery service and verify we get it's answers through the
+	// cache.
+
+	f1 := &fakeDiscovery{direct, relays}
+	c.Add(f1, time.Minute, 0)
+
+	dir, rel, err := c.Lookup(protocol.LocalDeviceID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(dir, direct) {
+		t.Errorf("Incorrect direct; %+v != %+v", dir, direct)
+	}
+	if !reflect.DeepEqual(rel, relays) {
+		t.Errorf("Incorrect relays; %+v != %+v", rel, relays)
+	}
+
+	// Add one more that answers in the same way and check that we don't
+	// duplicate or otherwise mess up the responses now.
+
+	f2 := &fakeDiscovery{direct, relays}
+	c.Add(f2, time.Minute, 0)
+
+	dir, rel, err = c.Lookup(protocol.LocalDeviceID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(dir, direct) {
+		t.Errorf("Incorrect direct; %+v != %+v", dir, direct)
+	}
+	if !reflect.DeepEqual(rel, relays) {
+		t.Errorf("Incorrect relays; %+v != %+v", rel, relays)
+	}
+}
+
+type fakeDiscovery struct {
+	direct []string
+	relays []Relay
+}
+
+func (f *fakeDiscovery) Lookup(deviceID protocol.DeviceID) (direct []string, relays []Relay, err error) {
+	return f.direct, f.relays, nil
+}
+
+func (f *fakeDiscovery) Error() error {
+	return nil
+}
+
+func (f *fakeDiscovery) String() string {
+	return "fake"
+}
+
+func (f *fakeDiscovery) Cache() map[protocol.DeviceID]CacheEntry {
+	return nil
+}