From 9084510e1b12a3ed93abfe933da9a3a828b509f0 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Fri, 18 Oct 2019 10:50:19 +0200
Subject: [PATCH] cmd/stdiscosrv: Sort addresses before replication (fixes
 #6093) (#6094)

This makes sure addresses are sorted when coming in from the API. The
database merge operation still checks for correct ordering (which is
quick) and sorts if it isn't correct (legacy database record or
replication peer), but then does a copy first.

Tested with -race in production...
---
 cmd/stdiscosrv/apisrv.go   |  5 +++++
 cmd/stdiscosrv/database.go | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/cmd/stdiscosrv/apisrv.go b/cmd/stdiscosrv/apisrv.go
index 5ac4622a1..a848ebc93 100644
--- a/cmd/stdiscosrv/apisrv.go
+++ b/cmd/stdiscosrv/apisrv.go
@@ -18,6 +18,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -279,6 +280,10 @@ func (s *apiSrv) handleAnnounce(remote net.IP, deviceID protocol.DeviceID, addre
 		dbAddrs[i].Expires = expire
 	}
 
+	// The address slice must always be sorted for database merges to work
+	// properly.
+	sort.Sort(databaseAddressOrder(dbAddrs))
+
 	seen := now.UnixNano()
 	if s.repl != nil {
 		s.repl.send(key, dbAddrs, seen)
diff --git a/cmd/stdiscosrv/database.go b/cmd/stdiscosrv/database.go
index d4d02536f..84a28b7ae 100644
--- a/cmd/stdiscosrv/database.go
+++ b/cmd/stdiscosrv/database.go
@@ -10,6 +10,7 @@
 package main
 
 import (
+	"log"
 	"sort"
 	"time"
 
@@ -263,12 +264,15 @@ func (s *levelDBStore) Stop() {
 // chosen for any duplicates.
 func merge(a, b DatabaseRecord) DatabaseRecord {
 	// Both lists must be sorted for this to work.
-	sort.Slice(a.Addresses, func(i, j int) bool {
-		return a.Addresses[i].Address < a.Addresses[j].Address
-	})
-	sort.Slice(b.Addresses, func(i, j int) bool {
-		return b.Addresses[i].Address < b.Addresses[j].Address
-	})
+	if !sort.IsSorted(databaseAddressOrder(a.Addresses)) {
+		log.Println("Warning: bug: addresses not correctly sorted in merge")
+		a.Addresses = sortedAddressCopy(a.Addresses)
+	}
+	if !sort.IsSorted(databaseAddressOrder(b.Addresses)) {
+		// no warning because this is the side we read from disk and it may
+		// legitimately predate correct sorting.
+		b.Addresses = sortedAddressCopy(b.Addresses)
+	}
 
 	res := DatabaseRecord{
 		Addresses: make([]DatabaseAddress, 0, len(a.Addresses)+len(b.Addresses)),
@@ -352,3 +356,24 @@ func expire(addrs []DatabaseAddress, now int64) []DatabaseAddress {
 	}
 	return addrs
 }
+
+func sortedAddressCopy(addrs []DatabaseAddress) []DatabaseAddress {
+	sorted := make([]DatabaseAddress, len(addrs))
+	copy(sorted, addrs)
+	sort.Sort(databaseAddressOrder(sorted))
+	return sorted
+}
+
+type databaseAddressOrder []DatabaseAddress
+
+func (s databaseAddressOrder) Less(a, b int) bool {
+	return s[a].Address < s[b].Address
+}
+
+func (s databaseAddressOrder) Swap(a, b int) {
+	s[a], s[b] = s[b], s[a]
+}
+
+func (s databaseAddressOrder) Len() int {
+	return len(s)
+}