From 0fb27297cd602882795ea4dd31df0f51ffa8629b Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Mon, 20 Jul 2020 17:39:46 +0200 Subject: Sanity checks, thread safety, remove unused grouplist in AvailableClient --- .../de/bwlehrpool/bwlp_guac/AvailableClient.java | 2 - .../bwlp_guac/BwlpAuthenticationProvider.java | 4 +- .../de/bwlehrpool/bwlp_guac/ConnectionManager.java | 61 ++++++++++++++-------- .../java/de/bwlehrpool/bwlp_guac/GroupField.java | 2 +- .../java/de/bwlehrpool/bwlp_guac/JsonGroup.java | 6 +-- 5 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java index 75e93f3..30cd951 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java @@ -22,8 +22,6 @@ public class AvailableClient implements Cloneable { private static final AtomicLong CON_ID = new AtomicLong(); - public ArrayList groupList = new ArrayList(); - private final String clientip; private String password; diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java b/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java index 524ce8d..c737c4f 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java @@ -117,10 +117,10 @@ public class BwlpAuthenticationProvider implements AuthenticationProvider { response.groupid = Integer.parseInt(group.get("id").asText()); if (response.groupid != 0) { password = group.get("password").asText(); - correctPassword = ConnectionManager.getGroupPool().get(response.groupid).password; + correctPassword = ConnectionManager.getGroup(response.groupid).password; } } catch (Exception e) { - LOGGER.info("Error reading group", e); + LOGGER.info("Error reading group choice by user, asking again...", e); tryAgain = true; } diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java index 4bc653b..01bf2fc 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java @@ -10,7 +10,14 @@ import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; @@ -37,14 +44,30 @@ public class ConnectionManager { private static final Logger LOGGER = LoggerFactory.getLogger(ConnectionManager.class); - // TODO: Config private static final String SOURCE_URL = SlxConfig.clientListUrl(); + /** + * synchronize on clientPool when accessing + */ private static final LinkedHashMap clientPool = new LinkedHashMap(); + /** + * synchronize on clientPool when accessing + */ private static final LinkedHashMap groupPool = new LinkedHashMap(); - public static LinkedHashMap getGroupPool() { return groupPool; } + + public static JsonGroup getGroup(int id) { + synchronized (clientPool) { + return groupPool.get(id); + } + } + + public static Collection getGroupList() { + synchronized (clientPool) { + return new ArrayList<>(groupPool.values()); + } + } public static WrappedConnection getExistingConnection(String user) { AvailableClient freeClient = null; @@ -198,15 +221,18 @@ public class ConnectionManager { return; } - // Temporary - JsonGroup[] groups = root.locations; - - if (groups == null) { - LOGGER.info("Group list null"); + if (root.locations == null) { + LOGGER.info("META DATA ERROR: Group list null"); + return; + } + if (root.clients == null) { + LOGGER.info("META DATA ERROR: Client list null"); + return; } - synchronized (groupPool) { + + synchronized (clientPool) { HashSet processedGroups = new HashSet(); - for (JsonGroup gnew : groups) { + for (JsonGroup gnew : root.locations) { if (gnew.locationids == null) continue; JsonGroup existing = groupPool.get(gnew.id); @@ -215,6 +241,7 @@ public class ConnectionManager { groupPool.put(gnew.id, gnew); existing = gnew; redoClientMapping = true; + LOGGER.info("Group " + gnew.name + " with pw " + gnew.password); } else { if (!Arrays.equals(existing.locationids, gnew.locationids)) { redoClientMapping = true; @@ -224,7 +251,7 @@ public class ConnectionManager { existing.password = gnew.password; } if (redoClientMapping) { - existing.clientList = new ArrayList(); + existing.clientList = new ConcurrentLinkedQueue(); for (AvailableClient client : clientPool.values()) { int locationid = client.getLocationid(); for (int id : existing.locationids) { @@ -236,22 +263,15 @@ public class ConnectionManager { } } processedGroups.add(existing); - LOGGER.info("Group " + gnew.name + " with pw " + gnew.password); } for (Iterator it = groupPool.values().iterator(); it.hasNext();) { JsonGroup g = it.next(); if (!processedGroups.contains(g)) { - for (AvailableClient client : g.clientList) { - client.groupList.remove(g); - } it.remove(); } } } - if (root.clients == null) { - LOGGER.info("Client list null"); - } synchronized (clientPool) { for (JsonClient cnew : root.clients) { if (cnew.password == null || cnew.clientip == null) @@ -266,7 +286,6 @@ public class ConnectionManager { for (int id : group.locationids) { if (id == cnew.locationid) { group.clientList.add(existing); - existing.groupList.add(group); break; } } @@ -278,11 +297,9 @@ public class ConnectionManager { for (int id : group.locationids) { if (id == existing.getLocationid()) { group.clientList.remove(existing); - existing.groupList.remove(group); } if (id == cnew.locationid) { group.clientList.add(existing); - existing.groupList.add(group); } } } @@ -295,7 +312,7 @@ public class ConnectionManager { AvailableClient c = it.next(); if (c.isTimeout(NOW)) { LOGGER.info("Removing client " + c + " from list"); - for (JsonGroup group : c.groupList) { + for (JsonGroup group : groupPool.values()) { group.clientList.remove(c); } it.remove(); diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java b/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java index 0e9c27b..ddc57e8 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java @@ -16,6 +16,6 @@ public class GroupField extends Field { @JsonProperty("groups") public Collection getGroups() throws GuacamoleException { - return ConnectionManager.getGroupPool().values(); + return ConnectionManager.getGroupList(); } } \ No newline at end of file diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java index c5adf9e..e1aef81 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java @@ -1,10 +1,10 @@ package de.bwlehrpool.bwlp_guac; +import java.util.concurrent.ConcurrentLinkedQueue; + import org.codehaus.jackson.annotate.JsonIgnore; import org.codehaus.jackson.annotate.JsonProperty; -import java.util.ArrayList; - public class JsonGroup { public int id; @@ -37,6 +37,6 @@ public class JsonGroup { } @JsonIgnore - public ArrayList clientList = new ArrayList(); + public ConcurrentLinkedQueue clientList = new ConcurrentLinkedQueue(); } -- cgit v1.2.3-55-g7522