80 lines
4.8 KiB
Markdown
80 lines
4.8 KiB
Markdown
# 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.
|