From 0bddff8bad958aac116eb82e460362bfa117ac9e Mon Sep 17 00:00:00 2001
From: Michael Jumper <mjumper@apache.org>
Date: Thu, 20 Aug 2020 19:17:45 -0700
Subject: [PATCH] GUACAMOLE-942: Correct race condition in retrieval of
 readable connection identifiers.

As activeTunnels is a live map which may change while
getActiveConnections() is running, it is possible for an initial call
to activeTunnels.isEmpty() to pass yet for the set of connection
identifiers produced to be empty.
---
 .../AbstractGuacamoleTunnelService.java       | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java
index 383ef3a9b..6687eb36d 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java
@@ -623,21 +623,26 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS
     public Collection<ActiveConnectionRecord> getActiveConnections(ModeledAuthenticatedUser user)
         throws GuacamoleException {
 
-        // Simply return empty list if there are no active tunnels
-        Collection<ActiveConnectionRecord> records = activeTunnels.values();
-        if (records.isEmpty())
-            return Collections.<ActiveConnectionRecord>emptyList();
-
         // Privileged users (such as system administrators) can view all
         // connections; no need to filter
+        Collection<ActiveConnectionRecord> records = activeTunnels.values();
         if (user.isPrivileged())
             return records;
 
         // Build set of all connection identifiers associated with active tunnels
-        Set<String> identifiers = new HashSet<String>(records.size());
+        Set<String> identifiers = new HashSet<>(records.size());
         for (ActiveConnectionRecord record : records)
             identifiers.add(record.getConnection().getIdentifier());
 
+        // Simply return empty list if there are no active tunnels (note that
+        // this check cannot be performed prior to building the set of
+        // identifiers, as activeTunnels may be non-empty at the beginning of
+        // the call to getActiveConnections() yet become empty before the
+        // set of identifiers is built, resulting in an error within
+        // selectReadable()
+        if (identifiers.isEmpty())
+            return Collections.<ActiveConnectionRecord>emptyList();
+
         // Produce collection of readable connection identifiers
         Collection<ConnectionModel> connections =
                 connectionMapper.selectReadable(user.getUser().getModel(),
@@ -649,7 +654,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS
             identifiers.add(connection.getIdentifier());
 
         // Produce readable subset of records
-        Collection<ActiveConnectionRecord> visibleRecords = new ArrayList<ActiveConnectionRecord>(records.size());
+        Collection<ActiveConnectionRecord> visibleRecords = new ArrayList<>(records.size());
         for (ActiveConnectionRecord record : records) {
             if (identifiers.contains(record.getConnection().getIdentifier()))
                 visibleRecords.add(record);