First pass of fixes.

.gitignore

@@ -26,4 +26,12 @@ bin/
 .vscode/
 
 ### Mac OS ###
-.DS_Store
+.DS_Store
+
+# IDE
+.idea/
+*.iml
+
+# Build output
+*.class
+*.jar

Readme

@@ -0,0 +1,70 @@
+# TimekeeperEnforcement
+
+Local enforcement for the Timekeeper e107 plugin. Once a minute it looks for each
+monitored student's games, asks the server how many play minutes are left
+(`timer.php`), warns, and closes the game when there are none.
+
+Needs Java 11 or newer. Run it as root (it stops other users' processes and shows
+popups as them).
+
+## Build
+
+```bash
+javac -d out src/au/pfip/*.java
+jar --create --file TimekeeperEnforcement.jar --main-class au.pfip.Main -C out .
+```
+
+## Settings
+
+The first of these that exists is used. Use `/etc/timekeeper.inf`, owned by root and
+not writable by the students: a settings file they can edit defeats the monitoring.
+
+1. `/etc/timekeeper.inf`
+2. `~/timekeeper.inf`
+3. `~/.config/timekeeper/timekeeper.inf`
+4. `./timekeeper.inf`
+
+If none exists, a new one is written to `~/.config/timekeeper/timekeeper.inf` with the
+defaults below. It is Java XML properties format.
+
+| Key | Default | Meaning |
+|---|---|---|
+| `UsersToMonitor` | *(empty, must be set)* | Comma separated accounts to watch, e.g. `alice,bob` |
+| `TimekeeperActive` | `true` | `false` makes the program wait 10 minutes and exit |
+| `ExemptNetwork` | *(empty = none)* | Network name where enforcement is off (checked at start-up only; anyone can name a hotspot after a network) |
+| `OfflineChecksBeforeKill` | `5` | Failed server checks in a row before a game is closed (`1` = close at once) |
+| `GamesRefreshMinutes` | `60` | How often the games list is fetched again |
+| `GamesURL`, `PlaytimeURL` | the overlord URLs | Server endpoints |
+| `LogFilePath` | home folder | Where `timekeeper<day>.log` is written |
+
+The games list is cached in `games.cache` next to the settings file and used when the
+server can't be reached.
+
+## systemd
+
+```ini
+[Unit]
+Description=Timekeeper enforcement
+After=network-online.target
+Wants=network-online.target
+
+[Service]
+ExecStart=/usr/bin/java -jar /opt/timekeeper/TimekeeperEnforcement.jar
+Restart=always
+RestartSec=30
+User=root
+
+[Install]
+WantedBy=multi-user.target
+```
+
+## Tests
+
+`test/au/pfip/Harness.java` and `Harness2.java` run against a fake local server and
+start and stop throwaway `sleep` processes owned by the current user:
+
+```bash
+javac -d out src/au/pfip/*.java test/au/pfip/*.java
+java -cp out au.pfip.Harness
+java -cp out au.pfip.Harness2
+```

src/au/pfip/FileHandling.java

@@ -3,21 +3,35 @@ package au.pfip;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.time.Duration;
 
 import static java.nio.file.StandardOpenOption.APPEND;
 import static java.nio.file.StandardOpenOption.CREATE;
 
 public class FileHandling {
 
+    // Log files are named by day of month, so a file this old is last month's.
+    private static final long STALE_DAYS = 25;
+
     private String logFilePath;
 
+    /**
+     * Use the log file. It used to be deleted at every start, so a restart (or
+     * a reboot) wiped the day's log. It is now only removed when it is left
+     * over from last month.
+     */
     public void initialiseFileHandling(String path) throws IOException {
         logFilePath = path;
-        Files.deleteIfExists(Path.of(logFilePath));
+        Path file = Path.of(logFilePath);
+        if (Files.exists(file)) {
+            long ageMillis = System.currentTimeMillis() - Files.getLastModifiedTime(file).toMillis();
+            if (ageMillis > Duration.ofDays(STALE_DAYS).toMillis()) {
+                Files.deleteIfExists(file);
+            }
+        }
     }
 
     public void write(final String s) throws IOException {
         Files.writeString(Path.of(logFilePath), s + System.lineSeparator(), CREATE, APPEND);
     }
-
-}
+}

src/au/pfip/Harness.java

@@ -0,0 +1,100 @@
+package au.pfip;
+
+import com.sun.net.httpserver.HttpServer;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.*;
+
+public class Harness {
+    static int failures = 0;
+    static void check(String name, boolean ok) {
+        System.out.println((ok ? "PASS " : "FAIL ") + name);
+        if (!ok) failures++;
+    }
+
+    public static void main(String[] a) throws Exception {
+        String me = System.getProperty("user.name");
+
+        // --- user parsing
+        check("parseUsers trims/dedupes/filters",
+                Arrays.equals(Main.parseUsers(" alice, bob ,alice,,bad name;rm,$(x)"), new String[]{"alice","bob"}));
+        check("parseUsers empty", Main.parseUsers("").length == 0);
+
+        // --- games json parsing
+        List<String> g1 = communications.parseUniqueValues("[\"minecraft\",\"steam\",\"minecraft\",\"bad;name\"]");
+        check("games array deduped + bad name dropped", g1.equals(List.of("minecraft","steam")));
+        List<String> g2 = communications.parseUniqueValues("{\"a\":\"roblox\",\"b\":\"fortnite\"}");
+        check("games object values", g2.equals(List.of("roblox","fortnite")));
+
+        // --- Shell
+        List<String> echo = Shell.run(5, "echo", "hi $(whoami) \"q\"");
+        check("Shell.run passes text literally (no shell)", echo != null && echo.get(0).equals("hi $(whoami) \"q\""));
+        long t0 = System.currentTimeMillis();
+        check("Shell.run times out", Shell.run(1, "sleep", "30") == null && System.currentTimeMillis() - t0 < 5000);
+        check("Shell.run non-zero exit -> null", Shell.run(5, "false") == null);
+
+        // --- Processes
+        int n = 40000 + new Random().nextInt(9000);
+        Process p1 = new ProcessBuilder("sleep", String.valueOf(n)).start();
+        Process p2 = new ProcessBuilder("sleep", String.valueOf(n + 1)).start();
+        Thread.sleep(500);
+        List<Processes.Proc> snap = Processes.snapshot(me);
+        check("snapshot finds my process", Processes.matching(snap, "sleep " + n).size() == 1);
+        check("snapshot excludes other text", Processes.matching(snap, "sleep " + (n + 5000)).isEmpty());
+        check("terminate returns true", Processes.terminate(me, "sleep " + n));
+        p1.waitFor();
+        check("target process is dead", !p1.isAlive());
+        check("other process untouched", p2.isAlive());
+        p2.destroyForcibly();
+
+        // a process that ignores SIGTERM must be force-killed
+        Process stubborn = new ProcessBuilder("bash", "-c", "trap '' TERM; exec -a stubborn-game-" + (n + 7) + " sleep 600").start();
+        Thread.sleep(700);
+        String stubName = "stubborn-game-" + (n + 7);
+        check("stubborn process found", Processes.matching(Processes.snapshot(me), stubName).size() == 1);
+        check("stubborn (ignores SIGTERM) is force-killed", Processes.terminate(me, stubName) && !stubborn.isAlive());
+
+        // --- communications against a local fake server
+        HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+        final String[] body = {"12"}; final int[] status = {200};
+        server.createContext("/", ex -> {
+            byte[] b = body[0].getBytes(StandardCharsets.UTF_8);
+            ex.sendResponseHeaders(status[0], b.length);
+            ex.getResponseBody().write(b); ex.close();
+        });
+        server.start();
+        int port = server.getAddress().getPort();
+        Main.PlaytimeURL = "http://127.0.0.1:" + port + "/timer.php";
+        Main.GamesURL = "http://127.0.0.1:" + port + "/games.json";
+
+        body[0] = "12";   check("numeric reply", communications.getPlaytimeFromServer(me, "g") == 12);
+        body[0] = "0";    check("zero reply", communications.getPlaytimeFromServer(me, "g") == 0);
+        body[0] = "1440"; check("1440 reply", communications.getPlaytimeFromServer(me, "g") == 1440);
+        body[0] = "";     check("empty reply -> UNKNOWN", communications.getPlaytimeFromServer(me, "g") == communications.UNKNOWN);
+        body[0] = "<b>Fatal error</b> in \"x\" `id`"; check("html reply -> UNKNOWN", communications.getPlaytimeFromServer(me, "g") == communications.UNKNOWN);
+        status[0] = 500; body[0] = "5"; check("HTTP 500 -> UNKNOWN", communications.getPlaytimeFromServer(me, "g") == communications.UNKNOWN);
+        status[0] = 200; body[0] = "Max play time exceeded."; check("text reply -> 0", communications.getPlaytimeFromServer(me, "g") == 0);
+        body[0] = "x\"; touch /tmp/pwned; \""; check("hostile text reply -> 0, no command run", communications.getPlaytimeFromServer(me, "g") == 0 && !new java.io.File("/tmp/pwned").exists());
+
+        // games list: good, then server error keeps old list, empty answer keeps old list
+        status[0] = 200; body[0] = "[\"minecraft\",\"steam\"]";
+        check("games fetched", communications.getGamesListFromServer() && Main.GamesList.length == 2);
+        status[0] = 500;
+        check("server error keeps list", !communications.getGamesListFromServer() && Main.GamesList.length == 2);
+        status[0] = 200; body[0] = "[]";
+        check("empty answer keeps list", !communications.getGamesListFromServer() && Main.GamesList.length == 2);
+        Main.GamesList = new String[0];
+        status[0] = 500;
+        communications.getGamesListFromServer();
+        check("offline start uses cache", Main.GamesList.length == 2);
+
+        // unreachable server
+        server.stop(0);
+        long t1 = System.currentTimeMillis();
+        check("unreachable -> UNKNOWN fast", communications.getPlaytimeFromServer(me, "g") == communications.UNKNOWN && System.currentTimeMillis() - t1 < 15000);
+
+        stubborn.destroyForcibly();
+        System.out.println(failures == 0 ? "ALL PASSED" : failures + " FAILED");
+        System.exit(failures == 0 ? 0 : 1);
+    }
+}

src/au/pfip/Harness2.java

@@ -0,0 +1,86 @@
+package au.pfip;
+
+import com.sun.net.httpserver.HttpServer;
+import java.lang.reflect.Method;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+public class Harness2 {
+    static int failures = 0;
+    static void check(String name, boolean ok) {
+        System.out.println((ok ? "PASS " : "FAIL ") + name);
+        if (!ok) failures++;
+    }
+
+    public static void main(String[] a) throws Exception {
+        Method checkUser = Main.class.getDeclaredMethod("checkUser", String.class);
+        checkUser.setAccessible(true);
+
+        HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
+        final String[] body = {"0"}; final int[] status = {200};
+        final java.util.List<String> queries = new java.util.concurrent.CopyOnWriteArrayList<>();
+        server.createContext("/", ex -> {
+            queries.add(ex.getRequestURI().toString());
+            byte[] b = body[0].getBytes(StandardCharsets.UTF_8);
+            ex.sendResponseHeaders(status[0], b.length);
+            ex.getResponseBody().write(b); ex.close();
+        });
+        server.start();
+        Main.PlaytimeURL = "http://127.0.0.1:" + server.getAddress().getPort() + "/timer.php";
+
+        String me = System.getProperty("user.name");   // the student who is playing
+        String other = "nobodyxyz";                     // a monitored student who is not playing
+        int n = 40000 + new Random().nextInt(9000);
+        String game = "sleep " + n;
+        Main.GamesList = new String[]{game};
+
+        // 1. The old bug: the playing student is checked FIRST, a non-playing student LAST.
+        Process p1 = new ProcessBuilder("sleep", String.valueOf(n)).start();
+        Thread.sleep(300);
+        body[0] = "0";
+        checkUser.invoke(null, me);
+        checkUser.invoke(null, other);
+        check("0 minutes: playing student's game is closed even though the next student isn't playing",
+                p1.waitFor(15, TimeUnit.SECONDS));
+        check("server was asked about the playing student only once",
+                queries.size() == 1 && queries.get(0).contains("user=" + me) && queries.get(0).contains("game=sleep+" + n));
+
+        // 2. Minutes left: nothing is killed.
+        Process p2 = new ProcessBuilder("sleep", String.valueOf(n)).start();
+        Thread.sleep(300);
+        body[0] = "10";
+        checkUser.invoke(null, me);
+        check("10 minutes left: game keeps running", p2.isAlive());
+
+        // 3. Server problems: closed only after OfflineChecksBeforeKill failures in a row.
+        settings.addSetting("OfflineChecksBeforeKill", "2");
+        status[0] = 500;
+        checkUser.invoke(null, me);
+        check("1st failed check: game keeps running", p2.isAlive());
+        status[0] = 200; body[0] = "10";
+        checkUser.invoke(null, me);                     // a good answer resets the count
+        status[0] = 500;
+        checkUser.invoke(null, me);
+        check("failure count was reset by a good answer: still running", p2.isAlive());
+        checkUser.invoke(null, me);
+        check("2 failed checks in a row: game is closed", p2.waitFor(15, TimeUnit.SECONDS));
+
+        // 4. A text reason from the server closes the game.
+        Process p3 = new ProcessBuilder("sleep", String.valueOf(n)).start();
+        Thread.sleep(300);
+        status[0] = 200; body[0] = "You are not within allowed play times.";
+        checkUser.invoke(null, me);
+        check("text reason: game is closed", p3.waitFor(15, TimeUnit.SECONDS));
+
+        // 5. No game running: server isn't called.
+        queries.clear();
+        checkUser.invoke(null, me);
+        check("no game running: server not called", queries.isEmpty());
+
+        server.stop(0);
+        System.out.println(failures == 0 ? "ALL PASSED" : failures + " FAILED");
+        System.exit(failures == 0 ? 0 : 1);
+    }
+}

src/au/pfip/Main.java

@@ -1,31 +1,55 @@
-// Press Shift twice to open the Search Everywhere dialog and type `show whitespaces`,
-// then press Enter. You can now see whitespace characters in your code.
-
 package au.pfip;
 
-import java.util.Objects;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
 
 /**
- * The code below will print the words Hello World
- * to the screen, and it is amazing
+ * Timekeeper enforcement: once a minute, look for the monitored students'
+ * games, ask the server how many play minutes are left, warn, and close the
+ * game when there are none.
+ *
+ * Changes from 4.0:
+ *  - several students: the old loop kept ONE running game and ONE user, and
+ *    checking the next student cleared what the previous check had found, so
+ *    only the last student in UsersToMonitor was ever enforced (and only
+ *    while they were playing). Each student now has their own state, is
+ *    checked, charged and closed on their own
+ *  - no shell: games are found and stopped with Java's process API, and
+ *    pkill is restricted to the student's own processes (it used to stop
+ *    anyone's process whose command line contained the game name)
+ *  - an unreachable server or unusable reply is told apart from "0 minutes";
+ *    the game is closed after OfflineChecksBeforeKill failed checks in a row
+ *    (default 5, set 1 for the old close-at-once behaviour)
+ *  - the games list is refreshed every GamesRefreshMinutes and cached, so
+ *    starting offline doesn't leave nothing monitored
+ *  - missing or blank settings no longer crash the program or switch
+ *    enforcement off (ExemptNetwork empty used to match every network)
+ *  - the 30 minute warning is only for a real 30 minutes (the server used to
+ *    answer 30 for students with Timekeeper switched off; it answers 1440 now)
  */
 public class Main {
 
-    private static final double version = 4.0;
-    public static final String GamesURL = "https://overlord.pfip.au/e107_plugins/timekeeper/timekeeperClientServices/games.json";
-    public static final String PlaytimeURL = "https://overlord.pfip.au/e107_plugins/timekeeper/timekeeperClientServices/timer.php";
-    private static String runningGame = "";
-    private static String activeUser = "";
-    private static int playTimeRemaining = 0;
-    private static Boolean shutdownFlag = false;
-    public static String[] GamesList;
-    private static String[] Users;
+    public static final String VERSION = "4.1";
+    public static String GamesURL;
+    public static String PlaytimeURL;
+    public static String[] GamesList = new String[0];
+
+    private static final Pattern VALID_USER = Pattern.compile("[A-Za-z_][A-Za-z0-9_.-]{0,31}");
+
+    private static volatile boolean shutdownFlag = false;
+    private static String[] Users = new String[0];
+    private static final Map<String, String> runningGames = new HashMap<>();
+    private static final Map<String, Integer> failedChecks = new HashMap<>();
+    private static long lastGamesRefresh = 0;
 
     /**
      * @param args Commandline arguments
      */
     public static void main(String[] args) {
-
         // If we couldn't get/make settings just exit as no config available
         if(!settings.initialiseSettings())
         {
@@ -40,9 +64,12 @@ public class Main {
             System.out.println("Logging engine failed to initialise, we can live without this.");
         }
 
-        logging.information("Starting Timekeeper " + version);
-        boolean timekeeperActive = Boolean.parseBoolean(settings.getSetting("TimekeeperActive"));
+        logging.information("Starting Timekeeper " + VERSION);
 
+        GamesURL = settings.getSetting("GamesURL");
+        PlaytimeURL = settings.getSetting("PlaytimeURL");
+
+        boolean timekeeperActive = Boolean.parseBoolean(settings.getSetting("TimekeeperActive"));
         if(!timekeeperActive)
         {
             logging.information("Timekeeper is set to not active, bailing out after a long delay.");
@@ -51,132 +78,198 @@ public class Main {
             System.exit(0);
         }
 
-        Users = settings.getSetting("UsersToMonitor").split(",");
+        Users = parseUsers(settings.getSetting("UsersToMonitor"));
+        if(Users.length == 0)
+        {
+            logging.error("No valid UsersToMonitor in the settings file, bailing out after a long delay.");
+            System.out.println("No valid UsersToMonitor in the settings file, bailing out after a long delay.");
+            burnTime(600);
+            System.exit(0);
+        }
 
-        if(Shell.check("/usr/bin/nmcli | /usr/bin/grep \"connected to \"",settings.getSetting("ExemptNetwork")))
+        if(isExemptNetwork())
         {
-            logging.information("Network " + settings.getSetting("ExemptNetwork") + "detected.");
+            logging.information("Network " + settings.getSetting("ExemptNetwork") + " detected.");
             burnTime(600);
             System.exit(0);
         }
 
         // Get list of games to be monitored.
         communications.getGamesListFromServer();
+        lastGamesRefresh = System.currentTimeMillis();
 
         // Inform users that the Timekeeper is starting up.
         for (String user : Users) {
-            communications.msgPopup("Starting up version " + version + ".",user);
+            communications.msgPopup("Starting up version " + VERSION + ".", user);
         }
 
         registerShutdownHook();
-        while(!shutdownFlag) {
-
-            startChecks();
-
-            checkPlayTime();
-
-            killGames();
 
+        while(!shutdownFlag) {
+            refreshGamesListIfDue();
+            for (String user : Users) {
+                checkUser(user);
+            }
             burnTime(60);
         }
 
         // Inform users that the Timekeeper is shutting down.
         for (String user : Users) {
-            communications.msgPopup("Shutting down.",user);
+            communications.msgPopup("Shutting down.", user);
         }
-
         logging.information("Shutting down.");
     }
 
+    public static boolean isValidUser(String user)
+    {
+        return user != null && VALID_USER.matcher(user).matches();
+    }
+
     /**
-     * Kill running games if there is no playtime left.
+     * The monitored user names: trimmed, valid, no duplicates.
      */
-    private static void killGames()
+    static String[] parseUsers(String setting)
     {
-        if(!Objects.equals(runningGame, "") && playTimeRemaining <= 0) {
-            System.out.println("Killing " + runningGame + " for user " + activeUser);
-            communications.msgPopup("Killing: " + runningGame + " as you are out of play time.",activeUser);
-            killGame(runningGame,activeUser);
-            //Shell.check("ps aux | grep " + user + " | grep " + game + " | grep -v \"grep\"", game)
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 30){
-            communications.msgPopup("Notice: You have 30 minutes remaining.",activeUser);
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 5){
-            communications.msgPopup("Notice: You have 5 minutes remaining.",activeUser);
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 4){
-            communications.msgPopup("Notice: You have 4 minutes remaining.",activeUser);
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 3){
-            communications.msgPopup("Notice: You have 3 minutes remaining.",activeUser);
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 2){
-            communications.msgPopup("Notice: You have 2 minutes remaining.",activeUser);
-        }else if(!Objects.equals(runningGame, "") && playTimeRemaining == 1){
-            communications.msgPopup("Notice: You have 1 minute remaining.",activeUser);
-        }else if(!Objects.equals(activeUser, "")){
-            System.out.println(activeUser + " still has playtime remaining (" + playTimeRemaining + " minutes).");
+        Set<String> users = new LinkedHashSet<>();
+        for (String user : setting.split(",")) {
+            user = user.trim();
+            if (user.isEmpty()) {
+                continue;
+            }
+            if (isValidUser(user)) {
+                users.add(user);
+            } else {
+                logging.error("Ignoring invalid user name in UsersToMonitor: " + user);
+            }
         }
+        return users.toArray(new String[0]);
     }
 
     /**
-     * Get remaining playtime from the server
+     * Is the computer connected to the network where enforcement is switched
+     * off? (Checked once at start-up. Anyone can name a hotspot after a
+     * network, so choose this one with that in mind.)
      */
-    private static void checkPlayTime()
+    private static boolean isExemptNetwork()
     {
-        if(!Objects.equals(runningGame, "")) {
-            System.out.println(runningGame);
-            playTimeRemaining = communications.getPlaytimeFromServer(activeUser, runningGame);
-            System.out.println(playTimeRemaining);
-        }else{
-            System.out.println("No games running.");
+        String exempt = settings.getSetting("ExemptNetwork");
+        if (exempt.isEmpty()) {
+            return false;
         }
+        List<String> output = Shell.run(10, "/usr/bin/nmcli");
+        if (output == null) {
+            return false;
+        }
+        for (String line : output) {
+            if (line.contains("connected to ") && line.contains(exempt)) {
+                return true;
+            }
+        }
+        return false;
     }
 
-    private static void startChecks()
+    /**
+     * Fetch the games list again now and then, and retry every minute while
+     * there is none at all.
+     */
+    private static void refreshGamesListIfDue()
     {
-        for (String user : Users) {
-            System.out.println("Checking running games for " + user);
-            checkRunningGames(user);
+        long refreshMillis = Math.max(1, settings.getInt("GamesRefreshMinutes", 60)) * 60_000L;
+        boolean due = System.currentTimeMillis() - lastGamesRefresh >= refreshMillis;
+
+        if (GamesList.length == 0 || due) {
+            communications.getGamesListFromServer();
+            lastGamesRefresh = System.currentTimeMillis();
         }
     }
 
-    private static void checkRunningGames(String user)
+    /**
+     * Check one student: find their game, ask for their minutes, act on the answer.
+     */
+    private static void checkUser(String user)
     {
+        String game = findRunningGame(user);
+
+        if (game == null) {
+            if (runningGames.remove(user) != null) {
+                logging.information(user + " is no longer running a game.");
+            }
+            failedChecks.remove(user);
+            return;
+        }
+
+        String previous = runningGames.put(user, game);
+        if (!game.equals(previous)) {
+            logging.information(game + " detected running for " + user + ".");
+        }
+
+        int remaining = communications.getPlaytimeFromServer(user, game);
+
+        if (remaining == communications.UNKNOWN) {
+            int failures = failedChecks.merge(user, 1, Integer::sum);
+            int limit = Math.max(1, settings.getInt("OfflineChecksBeforeKill", 5));
+            logging.information("Could not check play time for " + user + " (" + failures + " of " + limit + ").");
+            if (failures >= limit) {
+                communications.msgPopup("The Timekeeper server can not be reached, so " + game + " will be closed.", user);
+                killGame(user, game);
+            }
+            return;
+        }
+
+        failedChecks.remove(user);
+
+        if (remaining <= 0) {
+            communications.msgPopup("Closing " + game + " as you are out of play time.", user);
+            killGame(user, game);
+        } else if (remaining == 30 || remaining <= 5) {
+            communications.msgPopup("Notice: You have " + remaining + (remaining == 1 ? " minute" : " minutes") + " remaining.", user);
+        } else {
+            System.out.println(user + " still has playtime remaining (" + remaining + " minutes).");
+        }
+    }
+
+    /**
+     * The first known game that is running for the user, or null.
+     */
+    private static String findRunningGame(String user)
+    {
+        List<Processes.Proc> processes = Processes.snapshot(user);
         for (String game : GamesList) {
-                               // ps aux | grep      gregory | grep    minecraft | grep -v grep
-            if (Shell.check("/usr/bin/ps aux | /usr/bin/grep " + user + " | /usr/bin/grep " + game + " | /usr/bin/grep -v grep", game)) {
-                runningGame = game;
-                activeUser = user;
-                //System.out.println("Dis true---------");
-                System.out.println(game + " detected running.");
-                return;
-            } else {
-                //System.out.println(game + " not running.");
-                runningGame = "";
-                activeUser = "";
+            if (!Processes.matching(processes, game).isEmpty()) {
+                return game;
             }
         }
+        return null;
+    }
+
+    public static void killGame(String user, String game)
+    {
+        if (Processes.terminate(user, game)) {
+            runningGames.remove(user);
+        } else {
+            logging.error("Could not stop " + game + " for " + user + ", will try again next time.");
+        }
     }
 
     /**
-     * Burn 60 seconds while watching for shutdown signal.
+     * Burn time while watching for the shutdown signal.
      */
     private static void burnTime(int time)
     {
-        System.out.println("burning 60s.");
         int timer = time;
-        while(!shutdownFlag && timer!=0) {
+        while(!shutdownFlag && timer != 0) {
             timer--;
             try {
-                //System.out.println("burning " + timer + "s.");
                 //noinspection BusyWait
                 Thread.sleep(1000);
             } catch (InterruptedException e) {
-                logging.error("RuntimeException(e) ~ " + e);
+                logging.error("InterruptedException(e) ~ " + e);
+                Thread.currentThread().interrupt();
+                return;
             }
         }
     }
 
-
-
-
     private static void registerShutdownHook() {
         final Thread mainThread = Thread.currentThread();
         Runtime.getRuntime().addShutdownHook(new Thread(() -> {
@@ -188,45 +281,10 @@ public class Main {
             } catch (InterruptedException ex) {
                 logging.error("RuntimeException(e) ~ " + ex);
             }
-
         }));
     }
 
     static public void setShutdownProcess() {
         shutdownFlag = true;
     }
-
-
-
-    public static void killGame(String game, String user)
-    {
-
-        for (int i = 0; i <= 10; i++) {
-            if (Shell.check("/usr/bin/ps aux | /usr/bin/grep " + user + " | /usr/bin/grep " + game + " | /usr/bin/grep -v grep", game)) {
-                try{
-                    if(i >= 8) {
-                        Shell.runCmd("/usr/bin/pkill -9 -f " + game);
-                        logging.information("/usr/bin/pkill -9 -f " + game);
-                        logging.information("Killing " + game + " forcefully.");
-                    }else{
-                        Shell.runCmd("/usr/bin/pkill -15 -f " + game);
-                        logging.information("/usr/bin/pkill -15 -f " + game);
-                        logging.information("Killing " + game + " gracefully.");
-                    }
-                    Thread.sleep(1000);
-                } catch (InterruptedException e) {
-                    logging.error("InterruptedException(e) ~ " + e);
-                }
-            } else {
-                //System.out.println(game + " not running.");
-                runningGame = "";
-                activeUser = "";
-            }
-        }
-    }
-
-
-}
-
-// check for users logged in now.
-//ps -eo user,uid | awk 'NR>1 && $2 >= 1000 && ++seen[$2]==1{print $1}'
+}

src/au/pfip/Processes.java

@@ -0,0 +1,143 @@
+package au.pfip;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Finds and stops a student's game processes without a shell.
+ *
+ * This replaces "ps aux | grep user | grep game | grep -v grep" and
+ * "pkill -f game". The old pkill had no user filter, so it also stopped
+ * anyone else's process whose command line contained the game name.
+ */
+public class Processes {
+
+    /** One of the user's running processes. */
+    public static class Proc {
+        public final ProcessHandle handle;
+        public final String commandLine;
+
+        Proc(ProcessHandle handle, String commandLine) {
+            this.handle = handle;
+            this.commandLine = commandLine;
+        }
+    }
+
+    /**
+     * All processes owned by the user, except this program and the helpers it
+     * starts (notification popups mention game names in their text).
+     */
+    public static List<Proc> snapshot(String user) {
+        List<Proc> result = new ArrayList<>();
+        long self = ProcessHandle.current().pid();
+
+        ProcessHandle.allProcesses().forEach(handle -> {
+            try {
+                if (handle.pid() == self) {
+                    return;
+                }
+                Optional<String> owner = handle.info().user();
+                if (owner.isEmpty() || !owner.get().equals(user)) {
+                    return;
+                }
+                String commandLine = commandLineOf(handle);
+                if (commandLine.isEmpty() || commandLine.contains("notify-send")) {
+                    return;
+                }
+                result.add(new Proc(handle, commandLine));
+            } catch (RuntimeException e) {
+                // The process ended while we were looking at it.
+            }
+        });
+        return result;
+    }
+
+    /**
+     * The processes in the list whose command line contains the game name.
+     */
+    public static List<Proc> matching(List<Proc> processes, String game) {
+        List<Proc> result = new ArrayList<>();
+        for (Proc proc : processes) {
+            if (proc.commandLine.contains(game)) {
+                result.add(proc);
+            }
+        }
+        return result;
+    }
+
+    /**
+     * Stop the user's processes for the game: politely first, then by force.
+     *
+     * @return True when nothing is left running.
+     */
+    public static boolean terminate(String user, String game) {
+        List<Proc> targets = matching(snapshot(user), game);
+        if (targets.isEmpty()) {
+            return true;
+        }
+
+        logging.information("Killing " + game + " for " + user + " gracefully (" + targets.size() + " processes).");
+        for (Proc proc : targets) {
+            proc.handle.destroy();
+        }
+        waitUntilGone(targets, 8000);
+
+        targets = matching(snapshot(user), game);
+        if (targets.isEmpty()) {
+            return true;
+        }
+
+        logging.information("Killing " + game + " for " + user + " forcefully (" + targets.size() + " processes).");
+        for (Proc proc : targets) {
+            proc.handle.destroyForcibly();
+        }
+        waitUntilGone(targets, 3000);
+
+        return matching(snapshot(user), game).isEmpty();
+    }
+
+    private static void waitUntilGone(List<Proc> targets, long millis) {
+        long deadline = System.currentTimeMillis() + millis;
+        while (System.currentTimeMillis() < deadline) {
+            boolean anyAlive = false;
+            for (Proc proc : targets) {
+                if (proc.handle.isAlive()) {
+                    anyAlive = true;
+                    break;
+                }
+            }
+            if (!anyAlive) {
+                return;
+            }
+            try {
+                Thread.sleep(250);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                return;
+            }
+        }
+    }
+
+    /**
+     * The command line as `ps` shows it. The raw /proc/<pid>/cmdline is used
+     * first, because ProcessHandle.Info.commandLine() leaves out a renamed
+     * argv[0] (which some launchers use); Info is the fallback.
+     */
+    private static String commandLineOf(ProcessHandle handle) {
+        try {
+            byte[] raw = Files.readAllBytes(Path.of("/proc/" + handle.pid() + "/cmdline"));
+            String commandLine = new String(raw, StandardCharsets.UTF_8).replace('\0', ' ').trim();
+            if (!commandLine.isEmpty()) {
+                return commandLine;
+            }
+        } catch (Exception e) {
+            // process ended, or /proc not readable: use the Info below
+        }
+        ProcessHandle.Info info = handle.info();
+        return info.commandLine().orElse(info.command().orElse(""));
+    }
+}

src/au/pfip/Shell.java

@@ -1,72 +1,71 @@
 package au.pfip;
 
-import java.io.BufferedReader;
-        import java.io.File;
-        import java.io.InputStreamReader;
-        import java.util.ArrayList;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
 
+/**
+ * Runs external commands.
+ *
+ * Commands are always given as an argument list and run directly: there is no
+ * "bash -c" any more, so nothing taken from the server (game names, replies)
+ * or from the settings file can be interpreted by a shell. Every command has
+ * a timeout, so a hung command can no longer freeze the enforcement loop.
+ */
 public class Shell {
 
-    /** Returns null if it failed for some reason.
-     */
-    private static ArrayList<String> command(final String cmdline) {
-        try {
-            Process process =
-                    new ProcessBuilder("/usr/bin/bash", "-c", cmdline)
-                            .redirectErrorStream(true)
-                            .directory(new File("."))
-                            .start();
-
-            ArrayList<String> output = new ArrayList<>();
-            BufferedReader br = new BufferedReader(
-                    new InputStreamReader(process.getInputStream()));
-            String line;
-            while ( (line = br.readLine()) != null )
-                output.add(line);
-
-            //There should really be a timeout here.
-            if (0 != process.waitFor())
-                return null;
-
-            return output;
+    private static final int DEFAULT_TIMEOUT_SECONDS = 15;
 
-        } catch (Exception e) {
-            logging.error("RuntimeException(e) ~ " + e);
-            return null;
-        }
+    /**
+     * Run a command with the default timeout.
+     *
+     * @return The output lines, or null if it failed, timed out or exited non-zero.
+     */
+    public static List<String> run(String... command) {
+        return run(DEFAULT_TIMEOUT_SECONDS, command);
     }
 
-    public static boolean check(String cmd, String needle)
-    {
-        String cmdOutput = test(cmd);
-        return cmdOutput.contains(needle);
-    }
+    /**
+     * Run a command.
+     *
+     * @param timeoutSeconds How long to wait before the command is killed.
+     * @param command        The program and its arguments, one element each.
+     * @return The output lines (stdout and stderr), or null if it failed, timed out or exited non-zero.
+     */
+    public static List<String> run(int timeoutSeconds, String... command) {
+        try {
+            Process process = new ProcessBuilder(command)
+                    .redirectErrorStream(true)
+                    .start();
+            process.getOutputStream().close();
 
-    public static void runCmd(String cmd)
-    {
-        test(cmd);
-    }
+            if (!process.waitFor(timeoutSeconds, TimeUnit.SECONDS)) {
+                process.destroyForcibly();
+                logging.error("Command timed out after " + timeoutSeconds + "s: " + command[0]);
+                return null;
+            }
 
-// --Commented out by Inspection START (4/2/25, 10:02 pm):
-//    public static boolean checkMulti(String cmd, String needle)
-//    {
-//        String cmdOutput = test(cmd);
-//        return cmdOutput.contains(needle);
-//    }
-// --Commented out by Inspection STOP (4/2/25, 10:02 pm)
+            String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
+            if (process.exitValue() != 0) {
+                return null;
+            }
 
-    private static String test(String cmdline) {
-        ArrayList<String> output = command(cmdline);
-        StringBuilder reply = new StringBuilder();
-        if (null == output) {
-            return "";
-        }
-        else {
-            for (String line : output) {
-                reply.append(line);
+            List<String> lines = new ArrayList<>();
+            for (String line : output.split("\\R")) {
+                if (!line.isEmpty()) {
+                    lines.add(line);
+                }
             }
+            return lines;
+        } catch (IOException e) {
+            logging.error("IOException running " + command[0] + " ~ " + e);
+            return null;
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            logging.error("Interrupted while running " + command[0]);
+            return null;
         }
-        return reply.toString();
     }
-
 }

src/au/pfip/communications.java

@@ -1,117 +1,277 @@
 package au.pfip;
 
-import org.apache.commons.io.IOUtils;
-
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.StringReader;
-import java.net.URL;
-import java.net.URLConnection;
+import java.net.URI;
+import java.net.URLEncoder;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.time.Duration;
 import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
 
+/**
+ * Talking to the Timekeeper server, and to the student's desktop (popups).
+ *
+ * Changes from the first version:
+ *  - every request has a 10 second timeout, and a non-200 answer (a PHP error
+ *    page, for example) is "unknown" rather than being read as a reply
+ *  - "unknown" is told apart from "0 minutes left": an unreachable server or
+ *    an empty/HTML reply no longer closes the game on the first failure
+ *  - the popup is started without a shell. It used to be built into a root
+ *    shell command containing the server's reply text in quotes, so a reply
+ *    with a quote or $( ) in it ran as a command
+ *  - user and game names are URL-encoded in the query
+ *  - the games list is kept in a cache file next to the settings, used when
+ *    the server can't be reached (starting while offline used to mean no
+ *    games were monitored at all), and an empty answer never replaces a good list
+ *  - no commons-io dependency
+ */
 public class communications {
 
-    public static void getGamesListFromServer()
-    {
-        String RawGamesList;
-        StringBuilder logGames = new StringBuilder();
-        try{
-            RawGamesList = getURLData(Main.GamesURL);
-        } catch (IOException e){
-            logging.information("Exception while trying to get game list");
-            RawGamesList = "";
-        }
-        Main.GamesList = parseUniqueValues(RawGamesList).toArray(new String[0]); // set array of games here
-        for (int i = 0; i <= (Main.GamesList.length-1); i++) {
-            logGames.append(Main.GamesList[i]).append(",");
-        }
-        logging.information("Games to monitor: " + logGames);
-    }
+    /** Returned by getPlaytimeFromServer() when the server's answer can't be used. */
+    public static final int UNKNOWN = -1;
+
+    private static final Pattern DIGITS = Pattern.compile("\\d{1,6}");
+    private static final Pattern GAME_NAME = Pattern.compile("[\\w.+: ()\\-]{1,100}");
+    private static final Map<String, String> uidCache = new HashMap<>();
 
-    public static int getPlaytimeFromServer(String user,String game)
+    private static final HttpClient client = HttpClient.newBuilder()
+            .connectTimeout(Duration.ofSeconds(10))
+            .build();
+
+    /**
+     * Fetch the list of games to watch for. On failure the list already held
+     * is kept; with none held the cached copy is loaded.
+     *
+     * @return True when the list came from the server.
+     */
+    public static boolean getGamesListFromServer()
     {
-        StringBuilder urlQuery = new StringBuilder();
-        String RawPlaytime;
-        int Playtime;
-
-        urlQuery.append(Main.PlaytimeURL)
-                .append("?user=")
-                .append(user)
-                .append("&game=")
-                .append(game);
-
-        try{
-            logging.information("Using query " + urlQuery.toString() + " to get playtime from server.");
-            RawPlaytime = getURLData(urlQuery.toString()).trim();
-            boolean isNumeric = RawPlaytime.codePoints().allMatch( Character::isDigit );
-            if(isNumeric){
-                Playtime = Integer.parseInt(RawPlaytime.trim());
-            }else{
-                Playtime =0;
-                msgPopup("Notice: " + RawPlaytime + ".",user);
+        String rawGamesList = fetch(Main.GamesURL);
+        List<String> games = (rawGamesList == null) ? new ArrayList<>() : parseUniqueValues(rawGamesList);
+
+        if (!games.isEmpty()) {
+            Main.GamesList = games.toArray(new String[0]);
+            saveGamesCache(games);
+            logging.information("Games to monitor: " + String.join(",", games));
+            return true;
+        }
+
+        logging.information("Could not get the games list from the server.");
+        if (Main.GamesList.length == 0) {
+            List<String> cached = loadGamesCache();
+            if (!cached.isEmpty()) {
+                Main.GamesList = cached.toArray(new String[0]);
+                logging.information("Using the cached games list: " + String.join(",", cached));
             }
-        } catch (NumberFormatException | IOException e){
-            logging.information("Exception while trying to get playtime");
-            Playtime = 0;
         }
-        return Playtime;
+        return false;
     }
 
-    private static String getURLData(String GamesURL)  throws IOException
+    /**
+     * Ask the server for the minutes left. The server uses up one minute for
+     * each call, so call this once a minute while a game is running.
+     *
+     * @return Minutes remaining (0 or less: not allowed to play), or UNKNOWN.
+     */
+    public static int getPlaytimeFromServer(String user, String game)
     {
-        URL url = new URL(GamesURL);
-        URLConnection con = url.openConnection();
-        InputStream in = con.getInputStream();
-        String encoding = con.getContentEncoding();  // ** WRONG: should use "con.getContentType()" instead, but it returns something like "text/html; charset=UTF-8" so this value must be parsed to extract the actual encoding
-        encoding = encoding == null ? "UTF-8" : encoding;
-        return IOUtils.toString(in, encoding);
+        String query = Main.PlaytimeURL
+                + "?user=" + URLEncoder.encode(user, StandardCharsets.UTF_8)
+                + "&game=" + URLEncoder.encode(game, StandardCharsets.UTF_8);
+
+        logging.information("Using query " + query + " to get playtime from server.");
+        String reply = fetch(query);
+
+        if (reply == null) {
+            return UNKNOWN;
+        }
+
+        reply = reply.trim();
+
+        if (reply.isEmpty()) {
+            logging.information("Empty reply from the server.");
+            return UNKNOWN;
+        }
+
+        if (DIGITS.matcher(reply).matches()) {
+            return Integer.parseInt(reply);
+        }
+
+        if (reply.length() > 200 || reply.contains("<")) {
+            logging.information("Unexpected reply from the server (ignored).");
+            return UNKNOWN;
+        }
+
+        // Any other short text is the server's reason for not allowing play.
+        msgPopup("Notice: " + reply, user);
+        return 0;
     }
 
-    public static String getOnlyJsonValues(String json) {
-        json = json.replaceAll("\\{", "");
-        json = json.replaceAll("}", "");
-        json = json.replaceAll("\\[", "");
-        json = json.replaceAll("]", "");
-        json = json.replaceAll("\".*?\":", "");
-        json = json.replaceAll("\"", "");
-        //json = json.replaceAll("\n", "");
-        return json;
+    /**
+     * GET a URL. Returns the body, or null on any failure or non-200 status.
+     */
+    private static String fetch(String url)
+    {
+        try {
+            HttpRequest request = HttpRequest.newBuilder(URI.create(url))
+                    .timeout(Duration.ofSeconds(10))
+                    .header("User-Agent", "TimekeeperEnforcement/" + Main.VERSION)
+                    .GET()
+                    .build();
+            HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8));
+
+            if (response.statusCode() != 200) {
+                logging.information("Server answered HTTP " + response.statusCode() + " for " + url);
+                return null;
+            }
+            return response.body();
+        } catch (IOException | IllegalArgumentException e) {
+            logging.information("Exception while contacting the server: " + e);
+            return null;
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            return null;
+        }
     }
 
+    /**
+     * The distinct, usable game names found in the games list. Works on a JSON
+     * array of strings or an object (its keys are ignored, its string values
+     * are used), however it is laid out. A plain text list (one name per line,
+     * or comma separated) is accepted too.
+     */
     public static List<String> parseUniqueValues(String json) {
-        List<String> values = new ArrayList<>();
-        String onlyValues = getOnlyJsonValues(json);
-
-        try {
-            StringReader stringReader = new StringReader(onlyValues);
-            BufferedReader bufReader = new BufferedReader(stringReader);
-            String line;
-            while ((line = bufReader.readLine()) != null) {
-                if (line.endsWith(",")) {
-                    line = line.substring(0, line.length() - 1);
-                }
-                if (line.startsWith("\"") && line.endsWith("\"")) {
-                    line = line.substring(1, line.length() - 1);
-                }
+        Set<String> values = new LinkedHashSet<>();
 
+        if (json.indexOf('"') < 0) {
+            for (String name : json.split("[\\r\\n,]")) {
+                addGameName(values, name);
+            }
+            return new ArrayList<>(values);
+        }
 
-                line = line.trim();
-                if (!line.isEmpty()) {
-                    values.add(line);
+        int i = 0;
+        int n = json.length();
+        while (i < n) {
+            if (json.charAt(i) != '"') {
+                i++;
+                continue;
+            }
+            StringBuilder text = new StringBuilder();
+            i++;
+            while (i < n && json.charAt(i) != '"') {
+                char c = json.charAt(i);
+                if (c == '\\' && i + 1 < n) {
+                    text.append(json.charAt(i + 1));
+                    i += 2;
+                    continue;
                 }
+                text.append(c);
+                i++;
+            }
+            i++; // the closing quote
 
+            int j = i;
+            while (j < n && Character.isWhitespace(json.charAt(j))) {
+                j++;
             }
-        } catch (IOException e) {
-            //LOG.warn("Unable to read lines in String:" + onlyValues);
+            if (j < n && json.charAt(j) == ':') {
+                continue; // that was an object key, not a value
+            }
+            addGameName(values, text.toString());
+        }
+        return new ArrayList<>(values);
+    }
+
+    private static void addGameName(Set<String> values, String name) {
+        name = name.trim();
+        if (name.isEmpty()) {
+            return;
+        }
+        if (!GAME_NAME.matcher(name).matches()) {
+            logging.information("Ignoring game name with unusual characters: " + name);
+            return;
+        }
+        values.add(name);
+    }
+
+    /**
+     * Show a desktop notification to the user. Runs notify-send as that user
+     * (no shell involved: the text is a single argument).
+     */
+    public static void msgPopup(String message, String user) {
+        if (!Main.isValidUser(user)) {
+            return;
+        }
+        String uid = uidOf(user);
+        if (uid == null) {
+            logging.error("Could not find the uid of " + user + " for a popup.");
+            return;
         }
 
-        return values;
+        String text = message.replaceAll("\\p{Cntrl}", " ");
+        if (text.length() > 300) {
+            text = text.substring(0, 300);
+        }
+
+        Shell.run(10, "sudo", "-n", "-u", user, "env",
+                "XDG_RUNTIME_DIR=/run/user/" + uid,
+                "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/" + uid + "/bus",
+                "notify-send", "-t", "10000", "--app-name", "TimekeeperEnforcer", text);
     }
 
-    public static void msgPopup(String message,String user) {
-        Shell.runCmd("sudo -u " + user + " XDG_RUNTIME_DIR=/run/user/$(id -u " + user + ") notify-send -t 10000 --app-name \"TimekeeperEnforcer\" \"" + message + "\"");
+    private static String uidOf(String user) {
+        String cached = uidCache.get(user);
+        if (cached != null) {
+            return cached;
+        }
+        List<String> output = Shell.run(5, "id", "-u", user);
+        if (output == null || output.isEmpty() || !DIGITS.matcher(output.get(0).trim()).matches()) {
+            return null;
+        }
+        uidCache.put(user, output.get(0).trim());
+        return output.get(0).trim();
     }
 
-}
+    private static Path gamesCacheFile() {
+        return settings.getSettingsDirectory().resolve("games.cache");
+    }
+
+    private static void saveGamesCache(List<String> games) {
+        try {
+            Files.write(gamesCacheFile(), games, StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            logging.information("Could not save the games cache: " + e);
+        }
+    }
+
+    private static List<String> loadGamesCache() {
+        try {
+            Path file = gamesCacheFile();
+            if (!Files.exists(file)) {
+                return new ArrayList<>();
+            }
+            List<String> games = new ArrayList<>();
+            for (String line : Files.readAllLines(file, StandardCharsets.UTF_8)) {
+                line = line.trim();
+                if (!line.isEmpty() && GAME_NAME.matcher(line).matches()) {
+                    games.add(line);
+                }
+            }
+            return games;
+        } catch (IOException e) {
+            logging.information("Could not read the games cache: " + e);
+            return new ArrayList<>();
+        }
+    }
+}

src/au/pfip/logging.java

@@ -14,7 +14,6 @@ public class logging {
         {
             String newLogFile = settings.getSetting("LogFilePath") + "/timekeeper" + LocalDate.now().getDayOfMonth() + ".log";
             logFile.initialiseFileHandling(newLogFile);
-
         } catch (Exception e)
         {
             //noinspection CallToPrintStackTrace
@@ -27,35 +26,28 @@ public class logging {
 
     public static void information(String message)
     {
-        if(!Initialised)
-        {
-            stdOut(message);
-        }else{
-            try
-            {
-                logFile.write(LocalDate.now() + " " + LocalTime.now() + "~info~" + message);
-            } catch (Exception e)
-            {
-                //noinspection CallToPrintStackTrace
-                e.printStackTrace();
-            }
-        }
+        write("info", message);
     }
 
     public static void error(String message)
+    {
+        write("Error", message);
+    }
+
+    private static void write(String level, String message)
     {
         if(!Initialised)
         {
             stdOut(message);
-        }else{
-            try
-            {
-                logFile.write(LocalDate.now() + "~Error~" + message);
-            } catch (Exception e)
-            {
-                //noinspection CallToPrintStackTrace
-                e.printStackTrace();
-            }
+            return;
+        }
+        try
+        {
+            logFile.write(LocalDate.now() + " " + LocalTime.now() + "~" + level + "~" + message);
+        } catch (Exception e)
+        {
+            //noinspection CallToPrintStackTrace
+            e.printStackTrace();
         }
     }
 
@@ -65,4 +57,4 @@ public class logging {
         System.out.print("An error occurred before logging was setup.~");
         System.out.println(message);
     }
-}
+}

src/au/pfip/settings.java

@@ -1,37 +1,68 @@
 package au.pfip;
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.Properties;
 
+/**
+ * Settings file (Java XML properties). The first of these that exists is used:
+ * /etc/timekeeper.inf, ~/timekeeper.inf, ~/.config/timekeeper/timekeeper.inf,
+ * ./timekeeper.inf. Run as root and keep the file in /etc: a settings file
+ * the monitored user can edit defeats the monitoring.
+ *
+ * Changes: missing keys fall back to defaults (a missing key used to throw a
+ * NullPointerException or silently disable the program), the settings file is
+ * closed after reading, and the defaults written into a new file no longer
+ * contain real user names or a network name.
+ */
 public class settings {
 
     private static final Properties settings = new Properties();
+    private static final Map<String, String> defaults = new LinkedHashMap<>();
     private static String settingsFile;
 
+    static {
+        defaults.put("TimekeeperActive", "true");
+        defaults.put("LogFilePath", System.getProperty("user.home"));
+        // Comma separated list of the accounts to watch, e.g. "alice,bob". Must be set.
+        defaults.put("UsersToMonitor", "");
+        // Name of a network where enforcement is switched off. Empty means none.
+        defaults.put("ExemptNetwork", "");
+        defaults.put("GamesURL", "https://overlord.pfip.au/e107_plugins/timekeeper/timekeeperClientServices/games.json");
+        defaults.put("PlaytimeURL", "https://overlord.pfip.au/e107_plugins/timekeeper/timekeeperClientServices/timer.php");
+        // How many checks in a row may fail to reach the server before the game is closed (1 = close at once).
+        defaults.put("OfflineChecksBeforeKill", "5");
+        defaults.put("GamesRefreshMinutes", "60");
+    }
+
     public static boolean initialiseSettings()
     {
         try {
-            String[] paths = {"/etc/timekeeper.inf",System.getProperty("user.home") + "/timekeeper.inf",System.getProperty("user.home") + "/.config/timekeeper/timekeeper.inf",System.getProperty("user.dir") + "/timekeeper.inf"};
-            Path path;
+            String[] paths = {"/etc/timekeeper.inf", System.getProperty("user.home") + "/timekeeper.inf", System.getProperty("user.home") + "/.config/timekeeper/timekeeper.inf", System.getProperty("user.dir") + "/timekeeper.inf"};
 
-            for (int i = 0; i <= (paths.length-1); i++) {
-                System.out.println("Checking for settings at " + paths[i]);
-                path = Path.of(paths[i]);
-                if(Files.exists(path))
-                {
+            for (String candidate : paths) {
+                System.out.println("Checking for settings at " + candidate);
+                Path path = Path.of(candidate);
+                if (Files.exists(path)) {
                     System.out.println("Found settings at " + path);
                     settingsFile = String.valueOf(path);
-                    settings.loadFromXML(Files.newInputStream(path));
+                    try (InputStream in = Files.newInputStream(path)) {
+                        settings.loadFromXML(in);
+                    }
                     return true;
                 }
             }
-            path = Path.of(paths[2]);
+
+            Path path = Path.of(paths[2]);
             settingsFile = paths[2];
             Files.createDirectories(path.getParent());
-            createSettingsFile(String.valueOf(path.getParent()));
-            System.out.println("Settings file created at " + settingsFile);
+            createSettingsFile();
+            System.out.println("Settings file created at " + settingsFile + ". Set UsersToMonitor in it.");
             return true;
         } catch (IOException e) {
             //noinspection CallToPrintStackTrace
@@ -40,32 +71,55 @@ public class settings {
         }
     }
 
-    private static void createSettingsFile(String logPath)
+    private static void createSettingsFile()
     {
-        addSetting("TimekeeperActive", String.valueOf(true));
-        addSetting("LogFilePath", System.getProperty("user.home"));
-        addSetting("UsersToMonitor", "elliot,silas,gregory");
-        addSetting("ExemptNetwork", "RMS_BYOD");
+        for (Map.Entry<String, String> entry : defaults.entrySet()) {
+            addSetting(entry.getKey(), entry.getValue());
+        }
         saveSettingsFile();
     }
 
     public static void saveSettingsFile()
     {
-        try {
-            settings.storeToXML(Files.newOutputStream(Path.of(settingsFile)), "Timekeeper configuration file.");
+        try (OutputStream out = Files.newOutputStream(Path.of(settingsFile))) {
+            settings.storeToXML(out, "Timekeeper configuration file.");
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
 
-    public static void addSetting(String key,String val)
+    public static void addSetting(String key, String val)
     {
         settings.setProperty(key, String.valueOf(val));
     }
 
+    /**
+     * A setting, or its default when the file doesn't have it. Never null.
+     */
     public static String getSetting(String key)
     {
-        return settings.getProperty(key);
+        String value = settings.getProperty(key);
+        if (value == null) {
+            value = defaults.get(key);
+        }
+        return value == null ? "" : value.trim();
     }
 
-}
+    public static int getInt(String key, int fallback)
+    {
+        try {
+            return Integer.parseInt(getSetting(key));
+        } catch (NumberFormatException e) {
+            return fallback;
+        }
+    }
+
+    /**
+     * The folder the settings file lives in (also used for the games list cache).
+     */
+    public static Path getSettingsDirectory()
+    {
+        Path parent = (settingsFile == null) ? null : Path.of(settingsFile).toAbsolutePath().getParent();
+        return parent != null ? parent : Path.of(System.getProperty("user.home"));
+    }
+}