From 5d67b0c1e3e562c8f5d711e8617e5f37e995a93b Mon Sep 17 00:00:00 2001 From: ericek111 Date: Fri, 25 Sep 2026 11:25:39 +0000 Subject: [PATCH] Record what came of the codebase audit Co-Authored-By: Claude Opus 5.5 --- ts3-client/AUDIT.md | 79 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 ts3-client/AUDIT.md diff --git a/ts3-client/AUDIT.md b/ts3-client/AUDIT.md new file mode 100644 index 0000000..d9c8d06 --- /dev/null +++ b/ts3-client/AUDIT.md @@ -0,0 +1,79 @@ +# Codebase audit — ts3-client + +First audit: 2026-09-11. Revisited and worked through: 2026-09-25. Every finding below was +either fixed (commit given) or closed with a reason. Logging (§3) was left out on purpose. + +State after the fixes: `mvn -o test` passes (177 tests: 25 in ts3j, 135 core, 11 desktop, +6 swing), on Java 25 and 26. The Android release build (R8 over core and ts3j) passes. The +Swing client was run against the test server on Java 25: it connected, loaded the tree and +chat logs, and wrote its profile owner-only. + +--- + +## 1. Fixed + +### Tests and build +| Finding | Commit | +|---|---| +| The importer test failed on every run after the first. The cause was not the real profile: the POM already points `user.home` at `target/test-home`, but that folder outlives a run, so the first run's identity was still there | `ba5de5a` | +| The ts3j DNS test resolved `voice.teamspeak.com` for real and failed when DNS did not answer | ts3j `Test SRV lookup…`, `acb3efa` | +| README architecture and features out of date | `51f19c2` | +| Java 26 required for no reason; it now needs 25 (LTS), and builds, tests and runs on it | `85c3ae6` | +| Missing tests: channel tree, event handler, settings round-trip and migration | `71ac2c8` | + +### Connection lifecycle and threading +| Finding | Commit | +|---|---| +| An action racing a disconnect showed `NullPointerException` as the error; 20 raw threads. Actions now capture the socket and run on a pool that lives as long as the connection | `f350b19` | +| `connect()` on a live instance leaked socket and audio; fields shared across threads were not volatile | `d28bb57` | +| Permission errors were recognised by substring; now by the server's error id | `d074ffa` | +| `HotkeyEngine` locked on another class's list; `Hotkeys.all()` now returns a snapshot | `5b1809c` | +| Two quick tab switches could interleave the microphone hand-over | `665316d` | +| `MainFrame` was also the app controller: microphone ownership, push-to-talk, away and nickname fan-out moved to `core/session/Sessions` | `bf07f0b` | + +### Model and events +| Finding | Commit | +|---|---| +| `buildTree()` refilled lists shared with the UI; node fields not volatile. Lists are now published whole and unmodifiable. Whether a channel is full is counted from the model | `67f8f46` | +| `selfPermissionValue` scanned every permission; `primaryServerGroupName` unused | `8c735a0` | +| A cleared avatar, description or away message was ignored | `7cb4f47` | + +### Voice +| Finding | Commit | +|---|---| +| Fixed 60 ms jitter delay instead of TS3's adaptive buffer. The stashed libspeex port is now in `VoiceStream`, driven as TS3 drives it. A simulated 0–60 ms jitter stops concealing after about 3 s | `0083dc8` | +| A playback line kept open for every client that ever spoke | `6234ec6` | +| A dead playback line silently muted its speaker for the whole session | `12ca585` | +| `CaptureVoiceInput.stop()` tore down while the capture loop was still running | `8e9c461` | + +### Chat logs +| Finding | Commit | +|---|---| +| Multi-line messages lost their continuation lines in history | `f9b2f4d` | +| History read whole, ever-growing logs; now the last 4 MiB | `a5c4ba1` | +| `appendPrivateMessage` duplicated `appendMessage` | `1a3a3e3` | +| Logs could not be turned off or moved (new Options → Chat page) | `d7261c9` | +| Chat logs only worked with admin rights: the server id came from `serverinfo`, which guests may not use. It turned out to be base64(SHA-1(the server's handshake key)), as the official client derives it. Found while checking the "`serverUniqueId` fallbacks can go" finding | ts3j `Derive the server's unique id…`, `eeccb49` | + +### Persistence, file transfers, UI +| Finding | Commit | +|---|---| +| Profile files (private keys, server passwords) readable by everyone, and truncated by a crash mid-write. Now written privately and atomically, and the profile folder is owner-only | `3cdade9` | +| Three copies of the enum parser in `Settings` | `48a0032` | +| A stalled file server hung a download forever | `ae8d1f7` | +| Collapsed channels reopened on every tree rebuild | `f30cecf` | + +## 2. Closed without a change, or still open + +- **Chat-log folders "may contain `/`"**: they cannot. The value encoded is itself Base64 text, + and no 6-bit group of such bytes reaches 62 or 63, the codes for `+` and `/`. +- **`rootMessage()` hides the command**: every caller names the action in front of it. +- **Not tested:** `TeamspeakConnection`'s lifecycle against a fake socket (the smoke test + above covers connect only); the idle release and dead-line replacement in + `PerSpeakerPlayout`; the jitter buffer with real voice from another client. +- **`git stash@{0}`** ("jitter buffer test") is now fully ported, except FEC decoding (TS3 + does not use FEC either). It can be dropped. + +## 3. Left out on purpose + +- **Logging.** There are still 60 swallowed exceptions and no log output.