Repository: syncthing/syncthing (language: go)

Issue:
Properly indicate whether a connection is "LAN" or not in the GUI
IPv6 LAN connections currently show up as WAN connections because they use public IPv6 addresses. We should check if the other address is in the same subnet to determine if it's a LAN connection.

Apart from the wrong connection type in the UI, this might also affect bandwidth limitation logic.

https://forum.syncthing.net/t/towards-better-connection-type-icons/19333/23


Discussion / hints:
We should also treat ULA IPs as LAN:

https://github.com/syncthing/syncthing/issues/7456
We do compare against all directly connected networks already. If there is extra logic in the GUI that should probably be changed to just look at an `isLAN` flag from the backend, which already has that flag (but maybe doesn't expose it).

https://github.com/calmh/syncthing/blob/a7a87cd10ba63cc63a47cef3cb9733292f768746/lib/connections/service.go#L746-L751
I've been poking around in the code, trying to expose that flag to the GUI. Not sure I've got the cleanest possible solution, but at least it does fix the wrong classifications in my tests. Will send a PR soonish to discuss the implementation. 

Fix patch:
diff --git a/gui/default/index.html b/gui/default/index.html
index 81157d04d1d..7f26b0adf46 100644
--- a/gui/default/index.html
+++ b/gui/default/index.html
@@ -751,12 +751,8 @@ <h4 class="panel-title">
                       <span ng-switch-when="disconnected-inactive"><span class="hidden-xs" translate>Disconnected (Inactive)</span><span class="visible-xs" aria-label="{{'Disconnected (Inactive)' | translate}}"><i class="fas fa-fw fa-power-off"></i></span></span>
                       <span ng-switch-when="unused-disconnected"><span class="hidden-xs" translate>Disconnected (Unused)</span><span class="visible-xs" aria-label="{{'Disconnected (Unused)' | translate}}"><i class="fas fa-fw fa-unlink"></i></span></span>
                     </span>
-                    <span ng-switch="rdConnType(deviceCfg.deviceID)" class="remote-devices-panel">
-                      <span ng-switch-when="tcplan" class="reception reception-4 reception-theme"></span>
-                      <span ng-switch-when="tcpwan" class="reception reception-3 reception-theme"></span>
-                      <span ng-switch-when="quic" class="reception reception-2 reception-theme"></span>
-                      <span ng-switch-when="relay" class="reception reception-1 reception-theme"></span>
-                      <span ng-switch-when="disconnected" class="reception reception-0 reception-theme"></span>
+                    <span class="remote-devices-panel">
+                      <span ng-class="rdConnTypeIcon(rdConnType(deviceCfg.deviceID))" class="reception reception-theme"></span>
                     </span>
                   </span>
                   <div class="panel-title-text">{{deviceName(deviceCfg)}}</div>
@@ -849,7 +845,7 @@ <h4 class="panel-title">
                       </tr>
                       <tr ng-if="connections[deviceCfg.deviceID].connected">
                         <th><span class="reception reception-4 reception-theme"></span>&nbsp;<span translate>Connection Type</span></th>
-                        <td ng-if="connections[deviceCfg.deviceID].connected" class="text-right">
+                        <td class="text-right">
                           <span tooltip data-original-title="{{rdConnDetails(rdConnType(deviceCfg.deviceID))}}">
                             {{rdConnTypeString(rdConnType(deviceCfg.deviceID))}}
                           </span>
diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js
index aa301d081e2..f43bdea8e8d 100755
--- a/gui/default/syncthing/core/syncthingController.js
+++ b/gui/default/syncthing/core/syncthingController.js
@@ -1203,24 +1203,27 @@ angular.module('syncthing.core')
         $scope.rdConnType = function (deviceID) {
             var conn = $scope.connections[deviceID];
             if (!conn) return "-1";
-            if (conn.type.indexOf('relay') === 0) return "relay";
-            if (conn.type.indexOf('quic') === 0) return "quic";
-            if (conn.type.indexOf('tcp') === 0) return "tcp" + rdAddrType(conn.address);
-            return "disconnected";
-        }
-
-        function rdAddrType(address) {
-            var re = /(^(?:127\.|0?10\.|172\.0?1[6-9]\.|172\.0?2[0-9]\.|172\.0?3[01]\.|192\.168\.|169\.254\.|::1|[fF][cCdD][0-9a-fA-F]{2}:|[fF][eE][89aAbB][0-9a-fA-F]:))/
-            if (re.test(address)) return "lan";
-            return "wan";
+            var type = "disconnected";
+            if (conn.type.indexOf('relay') === 0) type = "relay";
+            else if (conn.type.indexOf('quic') === 0) type = "quic";
+            else if (conn.type.indexOf('tcp') === 0) type = "tcp";
+            else return type;
+
+            if (conn.isLocal) type += "lan";
+            else type += "wan";
+            return type;
         }
 
         $scope.rdConnTypeString = function (type) {
             switch (type) {
-                case "relay":
-                    return $translate.instant('Relay');
-                case "quic":
-                    return $translate.instant('QUIC');
+                case "relaywan":
+                    return $translate.instant('Relay WAN');
+                case "relaylan":
+                    return $translate.instant('Relay LAN');
+                case "quicwan":
+                    return $translate.instant('QUIC WAN');
+                case "quiclan":
+                    return $translate.instant('QUIC LAN');
                 case "tcpwan":
                     return $translate.instant('TCP WAN');
                 case "tcplan":
@@ -1230,11 +1233,30 @@ angular.module('syncthing.core')
             }
         }
 
+        $scope.rdConnTypeIcon = function (type) {
+            switch (type) {
+            case "tcplan":
+            case "quiclan":
+                return "reception-4";
+            case "tcpwan":
+            case "quicwan":
+                return "reception-3";
+            case "relaylan":
+                return "reception-2";
+            case "relaywan":
+                return "reception-1";
+            case "disconnected":
+                return "reception-0";
+            }
+        }
+
         $scope.rdConnDetails = function (type) {
             switch (type) {
-                case "relay":
+                case "relaylan":
+                case "relaywan":
                     return $translate.instant('Connections via relays might be rate limited by the relay');
-                case "quic":
+                case "quiclan":
+                case "quicwan":
                     return $translate.instant('QUIC connections are in most cases considered suboptimal');
                 case "tcpwan":
                     return $translate.instant('Using a direct TCP connection over WAN');
diff --git a/lib/connections/service.go b/lib/connections/service.go
index 127377d3b37..e504c8d2026 100644
--- a/lib/connections/service.go
+++ b/lib/connections/service.go
@@ -403,11 +403,13 @@ func (s *service) handleHellos(ctx context.Context) error {
 			continue
 		}
 
+		// Determine only once whether a connection is considered local
+		// according to our configuration, then cache the decision.
+		c.isLocal = s.isLAN(c.RemoteAddr())
 		// Wrap the connection in rate limiters. The limiter itself will
 		// keep up with config changes to the rate and whether or not LAN
 		// connections are limited.
-		isLAN := s.isLAN(c.RemoteAddr())
-		rd, wr := s.limiter.getLimiters(remoteID, c, isLAN)
+		rd, wr := s.limiter.getLimiters(remoteID, c, c.IsLocal())
 
 		protoConn := protocol.NewConnection(remoteID, rd, wr, c, s.model, c, deviceCfg.Compression, s.cfg.FolderPasswords(remoteID))
 		go func() {
diff --git a/lib/connections/structs.go b/lib/connections/structs.go
index 238ab7fb013..1a98fc72070 100644
--- a/lib/connections/structs.go
+++ b/lib/connections/structs.go
@@ -39,6 +39,7 @@ type tlsConn interface {
 type internalConn struct {
 	tlsConn
 	connType      connType
+	isLocal       bool
 	priority      int
 	establishedAt time.Time
 }
@@ -107,6 +108,10 @@ func (c internalConn) Type() string {
 	return c.connType.String()
 }
 
+func (c internalConn) IsLocal() bool {
+	return c.isLocal
+}
+
 func (c internalConn) Priority() int {
 	return c.priority
 }
diff --git a/lib/model/model.go b/lib/model/model.go
index 24b37ece83e..33f5e61fd3a 100644
--- a/lib/model/model.go
+++ b/lib/model/model.go
@@ -709,6 +709,7 @@ type ConnectionInfo struct {
 	Address       string `json:"address"`
 	ClientVersion string `json:"clientVersion"`
 	Type          string `json:"type"`
+	IsLocal       bool   `json:"isLocal"`
 	Crypto        string `json:"crypto"`
 }
 
@@ -739,6 +740,7 @@ func (m *model) ConnectionStats() map[string]interface{} {
 		}
 		if conn, ok := m.conn[device]; ok {
 			ci.Type = conn.Type()
+			ci.IsLocal = conn.IsLocal()
 			ci.Crypto = conn.Crypto()
 			ci.Connected = ok
 			ci.Statistics = conn.Statistics()
diff --git a/lib/protocol/mocks/connection.go b/lib/protocol/mocks/connection.go
index 3aab4b8b981..3fddab239a1 100644
--- a/lib/protocol/mocks/connection.go
+++ b/lib/protocol/mocks/connection.go
@@ -94,6 +94,16 @@ type Connection struct {
 	indexUpdateReturnsOnCall map[int]struct {
 		result1 error
 	}
+	IsLocalStub        func() bool
+	isLocalMutex       sync.RWMutex
+	isLocalArgsForCall []struct {
+	}
+	isLocalReturns struct {
+		result1 bool
+	}
+	isLocalReturnsOnCall map[int]struct {
+		result1 bool
+	}
 	PriorityStub        func() int
 	priorityMutex       sync.RWMutex
 	priorityArgsForCall []struct {
@@ -639,6 +649,59 @@ func (fake *Connection) IndexUpdateReturnsOnCall(i int, result1 error) {
 	}{result1}
 }
 
+func (fake *Connection) IsLocal() bool {
+	fake.isLocalMutex.Lock()
+	ret, specificReturn := fake.isLocalReturnsOnCall[len(fake.isLocalArgsForCall)]
+	fake.isLocalArgsForCall = append(fake.isLocalArgsForCall, struct {
+	}{})
+	stub := fake.IsLocalStub
+	fakeReturns := fake.isLocalReturns
+	fake.recordInvocation("IsLocal", []interface{}{})
+	fake.isLocalMutex.Unlock()
+	if stub != nil {
+		return stub()
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *Connection) IsLocalCallCount() int {
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
+	return len(fake.isLocalArgsForCall)
+}
+
+func (fake *Connection) IsLocalCalls(stub func() bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = stub
+}
+
+func (fake *Connection) IsLocalReturns(result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	fake.isLocalReturns = struct {
+		result1 bool
+	}{result1}
+}
+
+func (fake *Connection) IsLocalReturnsOnCall(i int, result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	if fake.isLocalReturnsOnCall == nil {
+		fake.isLocalReturnsOnCall = make(map[int]struct {
+			result1 bool
+		})
+	}
+	fake.isLocalReturnsOnCall[i] = struct {
+		result1 bool
+	}{result1}
+}
+
 func (fake *Connection) Priority() int {
 	fake.priorityMutex.Lock()
 	ret, specificReturn := fake.priorityReturnsOnCall[len(fake.priorityArgsForCall)]
@@ -1111,6 +1174,8 @@ func (fake *Connection) Invocations() map[string][][]interface{} {
 	defer fake.indexMutex.RUnlock()
 	fake.indexUpdateMutex.RLock()
 	defer fake.indexUpdateMutex.RUnlock()
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
 	fake.priorityMutex.RLock()
 	defer fake.priorityMutex.RUnlock()
 	fake.remoteAddrMutex.RLock()
diff --git a/lib/protocol/mocks/connection_info.go b/lib/protocol/mocks/connection_info.go
index 8612b638327..9e9dd472826 100644
--- a/lib/protocol/mocks/connection_info.go
+++ b/lib/protocol/mocks/connection_info.go
@@ -30,6 +30,16 @@ type ConnectionInfo struct {
 	establishedAtReturnsOnCall map[int]struct {
 		result1 time.Time
 	}
+	IsLocalStub        func() bool
+	isLocalMutex       sync.RWMutex
+	isLocalArgsForCall []struct {
+	}
+	isLocalReturns struct {
+		result1 bool
+	}
+	isLocalReturnsOnCall map[int]struct {
+		result1 bool
+	}
 	PriorityStub        func() int
 	priorityMutex       sync.RWMutex
 	priorityArgsForCall []struct {
@@ -190,6 +200,59 @@ func (fake *ConnectionInfo) EstablishedAtReturnsOnCall(i int, result1 time.Time)
 	}{result1}
 }
 
+func (fake *ConnectionInfo) IsLocal() bool {
+	fake.isLocalMutex.Lock()
+	ret, specificReturn := fake.isLocalReturnsOnCall[len(fake.isLocalArgsForCall)]
+	fake.isLocalArgsForCall = append(fake.isLocalArgsForCall, struct {
+	}{})
+	stub := fake.IsLocalStub
+	fakeReturns := fake.isLocalReturns
+	fake.recordInvocation("IsLocal", []interface{}{})
+	fake.isLocalMutex.Unlock()
+	if stub != nil {
+		return stub()
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *ConnectionInfo) IsLocalCallCount() int {
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
+	return len(fake.isLocalArgsForCall)
+}
+
+func (fake *ConnectionInfo) IsLocalCalls(stub func() bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = stub
+}
+
+func (fake *ConnectionInfo) IsLocalReturns(result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	fake.isLocalReturns = struct {
+		result1 bool
+	}{result1}
+}
+
+func (fake *ConnectionInfo) IsLocalReturnsOnCall(i int, result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	if fake.isLocalReturnsOnCall == nil {
+		fake.isLocalReturnsOnCall = make(map[int]struct {
+			result1 bool
+		})
+	}
+	fake.isLocalReturnsOnCall[i] = struct {
+		result1 bool
+	}{result1}
+}
+
 func (fake *ConnectionInfo) Priority() int {
 	fake.priorityMutex.Lock()
 	ret, specificReturn := fake.priorityReturnsOnCall[len(fake.priorityArgsForCall)]
@@ -462,6 +525,8 @@ func (fake *ConnectionInfo) Invocations() map[string][][]interface{} {
 	defer fake.cryptoMutex.RUnlock()
 	fake.establishedAtMutex.RLock()
 	defer fake.establishedAtMutex.RUnlock()
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
 	fake.priorityMutex.RLock()
 	defer fake.priorityMutex.RUnlock()
 	fake.remoteAddrMutex.RLock()
diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go
index 2fba29c6b8a..48dfde2d306 100644
--- a/lib/protocol/protocol.go
+++ b/lib/protocol/protocol.go
@@ -162,6 +162,7 @@ type Connection interface {
 type ConnectionInfo interface {
 	Type() string
 	Transport() string
+	IsLocal() bool
 	RemoteAddr() net.Addr
 	Priority() int
 	String() string


Test patch:
diff --git a/lib/protocol/mocked_connection_info_test.go b/lib/protocol/mocked_connection_info_test.go
index b336a7440bd..aab43306ff1 100644
--- a/lib/protocol/mocked_connection_info_test.go
+++ b/lib/protocol/mocked_connection_info_test.go
@@ -28,6 +28,16 @@ type mockedConnectionInfo struct {
 	establishedAtReturnsOnCall map[int]struct {
 		result1 time.Time
 	}
+	IsLocalStub        func() bool
+	isLocalMutex       sync.RWMutex
+	isLocalArgsForCall []struct {
+	}
+	isLocalReturns struct {
+		result1 bool
+	}
+	isLocalReturnsOnCall map[int]struct {
+		result1 bool
+	}
 	PriorityStub        func() int
 	priorityMutex       sync.RWMutex
 	priorityArgsForCall []struct {
@@ -188,6 +198,59 @@ func (fake *mockedConnectionInfo) EstablishedAtReturnsOnCall(i int, result1 time
 	}{result1}
 }
 
+func (fake *mockedConnectionInfo) IsLocal() bool {
+	fake.isLocalMutex.Lock()
+	ret, specificReturn := fake.isLocalReturnsOnCall[len(fake.isLocalArgsForCall)]
+	fake.isLocalArgsForCall = append(fake.isLocalArgsForCall, struct {
+	}{})
+	stub := fake.IsLocalStub
+	fakeReturns := fake.isLocalReturns
+	fake.recordInvocation("IsLocal", []interface{}{})
+	fake.isLocalMutex.Unlock()
+	if stub != nil {
+		return stub()
+	}
+	if specificReturn {
+		return ret.result1
+	}
+	return fakeReturns.result1
+}
+
+func (fake *mockedConnectionInfo) IsLocalCallCount() int {
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
+	return len(fake.isLocalArgsForCall)
+}
+
+func (fake *mockedConnectionInfo) IsLocalCalls(stub func() bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = stub
+}
+
+func (fake *mockedConnectionInfo) IsLocalReturns(result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	fake.isLocalReturns = struct {
+		result1 bool
+	}{result1}
+}
+
+func (fake *mockedConnectionInfo) IsLocalReturnsOnCall(i int, result1 bool) {
+	fake.isLocalMutex.Lock()
+	defer fake.isLocalMutex.Unlock()
+	fake.IsLocalStub = nil
+	if fake.isLocalReturnsOnCall == nil {
+		fake.isLocalReturnsOnCall = make(map[int]struct {
+			result1 bool
+		})
+	}
+	fake.isLocalReturnsOnCall[i] = struct {
+		result1 bool
+	}{result1}
+}
+
 func (fake *mockedConnectionInfo) Priority() int {
 	fake.priorityMutex.Lock()
 	ret, specificReturn := fake.priorityReturnsOnCall[len(fake.priorityArgsForCall)]
@@ -460,6 +523,8 @@ func (fake *mockedConnectionInfo) Invocations() map[string][][]interface{} {
 	defer fake.cryptoMutex.RUnlock()
 	fake.establishedAtMutex.RLock()
 	defer fake.establishedAtMutex.RUnlock()
+	fake.isLocalMutex.RLock()
+	defer fake.isLocalMutex.RUnlock()
 	fake.priorityMutex.RLock()
 	defer fake.priorityMutex.RUnlock()
 	fake.remoteAddrMutex.RLock()