Record what came of the codebase audit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-09-25 11:25:39 +00:00
parent 51f19c2d03
commit 5d67b0c1e3

79
ts3-client/AUDIT.md Normal file
View File

@@ -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.