Copilot commented on code in PR #1117:
URL: https://github.com/apache/guacamole-client/pull/1117#discussion_r2353416508
##########
guacamole/src/main/java/org/apache/guacamole/rest/auth/HashTokenSessionMap.java:
##########
@@ -85,10 +98,25 @@ public HashTokenSessionMap(Environment environment) {
logger.debug("Error while reading session timeout value.", e);
sessionTimeoutValue = 60;
}
+
+ // Read connection timeout from guacamole.properties
+ try {
+ connectionTimeoutValue =
environment.getProperty(CONNECTION_TIMEOUT, 0); // Disabled by default
+ }
+ catch (GuacamoleException e) {
+ logger.error("Unable to read guacamole.properties: {}",
e.getMessage());
+ logger.debug("Error while reading connection timeout value.", e);
+ connectionTimeoutValue = 0;
Review Comment:
Extra whitespace at the end of the line should be removed.
```suggestion
connectionTimeoutValue = 0;
```
##########
guacamole/src/main/java/org/apache/guacamole/GuacamoleSession.java:
##########
@@ -284,6 +293,94 @@ public long getLastAccessedTime() {
return lastAccessedTime;
}
+ /**
+ * Returns the creation time of the specified tunnel, as the number of
+ * milliseconds since midnight January 1, 1970 GMT. Invoking this function
+ * does not affect the last access time of this session.
+ *
+ * @param uuid
+ * The UUID of the tunnel to get the creation time for.
+ *
+ * @return
+ * The time the tunnel was created, or 0 if the tunnel doesn't exist.
+ */
+ public long getTunnelCreationTime(String uuid) {
+ Long creationTime = tunnelCreationTimes.get(uuid);
+ return creationTime != null ? creationTime : 0;
+ }
+
+ /**
+ * Returns the age of the oldest tunnel in this session, in milliseconds.
+ * If no tunnels exist, returns 0. Invoking this function does not affect
+ * the last access time of this session.
+ *
+ * @return
+ * The age of the oldest tunnel in milliseconds, or 0 if no tunnels
exist.
+ */
+ public long getOldestTunnelAge() {
+ if (tunnelCreationTimes.isEmpty()) {
+ return 0;
+ }
+
+ long currentTime = System.currentTimeMillis();
+ long oldestCreationTime = tunnelCreationTimes.values().stream()
+ .mapToLong(Long::longValue)
+ .min()
+ .orElse(currentTime);
+
+ return currentTime - oldestCreationTime;
+ }
+
+ /**
+ * Closes and removes any tunnels in this session that exceed the specified
+ * age limit. Invoking this function does not affect the last access time
+ * of this session.
+ *
+ * @param maxAge
+ * The maximum allowed age of tunnels in milliseconds. Tunnels older
+ * than this will be closed and removed.
+ *
+ * @return
+ * The number of tunnels that were closed and removed.
+ */
+ public int closeExpiredTunnels(long maxAge) {
+ if (maxAge <= 0 || tunnelCreationTimes.isEmpty()) {
+ return 0;
+ }
+
+ long currentTime = System.currentTimeMillis();
+ int closedCount = 0;
+
+ // Find tunnels that exceed the age limit
+ List<String> expiredTunnelIds = new ArrayList<>();
+ for (Map.Entry<String, Long> entry : tunnelCreationTimes.entrySet()) {
+ long tunnelAge = currentTime - entry.getValue();
+ if (tunnelAge >= maxAge) {
+ expiredTunnelIds.add(entry.getKey());
+ }
+ }
+
+ // Close and remove expired tunnels
+ for (String tunnelId : expiredTunnelIds) {
+ UserTunnel tunnel = tunnels.get(tunnelId);
+ if (tunnel != null) {
+ try {
+ tunnel.close();
+ logger.debug("Closed tunnel \"{}\" due to connection
timeout.", tunnelId);
+ }
+ catch (GuacamoleException e) {
+ logger.debug("Unable to close expired tunnel \"" +
tunnelId + "\".", e);
+ }
+ // Remove from both maps regardless of whether close succeeded
+ tunnels.remove(tunnelId);
+ tunnelCreationTimes.remove(tunnelId);
+ closedCount++;
+ }
+ }
Review Comment:
The tunnel removal logic is duplicated here and in the `removeTunnel`
method. Consider extracting this into a private helper method to avoid code
duplication.
```suggestion
if (removeTunnelById(tunnelId, "Closed tunnel \"{}\" due to
connection timeout.", "Unable to close expired tunnel \"{}\".")) {
closedCount++;
}
}
/**
* Removes the tunnel with the given ID from the session, closing it and
removing
* its creation time. Returns true if a tunnel was removed, false
otherwise.
*
* @param tunnelId
* The ID of the tunnel to remove.
* @param successMsg
* The message to log on successful close.
* @param errorMsg
* The message to log on error.
* @return
* true if a tunnel was removed, false otherwise.
*/
private boolean removeTunnelById(String tunnelId, String successMsg,
String errorMsg) {
UserTunnel tunnel = tunnels.get(tunnelId);
if (tunnel != null) {
try {
tunnel.close();
logger.debug(successMsg, tunnelId);
}
catch (GuacamoleException e) {
logger.debug(errorMsg, tunnelId, e);
}
tunnels.remove(tunnelId);
tunnelCreationTimes.remove(tunnelId);
return true;
}
return false;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
