mirror of
https://github.com/brazilofmux/tinymux
synced 2026-08-13 00:23:11 -04:00
Retire ISSUES.md trackers; migrate open items to GitHub issues
The 22 per-tracker ISSUES.md files carried both their open items and a full FIXED / FALSE ALARM / NOT A BUG audit history. The 57 still-open items have been migrated to GitHub issues #706-#762 with a 2.14-aligned label taxonomy (area:* / type:* / priority:* / topic:*), so open work now lives in the issue tracker instead of in-tree Markdown. The closed/audit history of every tracker is preserved here in git history (this commit's parent); nothing is lost. Open items migrated by tracker: mux/src/ (1) -> #706 mux/lib/ (10) -> #707-#716 mux/modules/engine/ (18) -> #717-#734 mux/ganl/ (10) -> #735-#744 mux/modules/sqlslave/ (9) -> #745-#753 testcases/ (4) -> #754-#757 client/tf/ + client/ (5) -> #758-#762 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stephen Dennis
parent
c1f291178d
commit
6288c4da37
22 changed files with 0 additions and 1982 deletions
72
ISSUES.md
72
ISSUES.md
|
|
@ -1,72 +0,0 @@
|
|||
# TinyMUX Project Issues Tracker
|
||||
|
||||
This is the top-level index for TinyMUX issue trackers. Per-tracker
|
||||
files keep their full FIXED / FALSE ALARM / NOT A BUG history for
|
||||
context; this file only summarises what is still open. When a tracker
|
||||
lists zero open items, the file is kept as a historical record of the
|
||||
audit passes that closed it.
|
||||
|
||||
Last refreshed: 2026-05-22 (post Apple Silicon JIT enablement and April 2026 safety audits).
|
||||
|
||||
## Trackers With Open Items
|
||||
|
||||
| Tracker | Open | Summary |
|
||||
|---|---|---|
|
||||
| [Core Server (`mux/src/`)](mux/src/ISSUES.md) | 1 | Windows console `SetConsoleCtrlHandler` TODO (platform.cpp:142). All April 2026 buffer/UB/signal/WebSocket/netaddr/Stub_PipePump/PanicRestart criticals documented as FIXED in sub-tracker. |
|
||||
| [libmux (`mux/lib/`)](mux/lib/ISSUES.md) | 10 | Ragel `date_scan.rl`: 8 bugs (32-bit digit overflow, `int→short` year narrowing, negative time-of-day fields, "12:30 AM" rejection, `iWeekOfYear==0` week-date, sub-second narrowing loss, TZ sign fragility) + 2 opportunities (dedicated fuzz tests; ZWJ emoji clusters). |
|
||||
| [Engine Module](mux/modules/engine/ISSUES.md) | ~17 | `alloc_lbuf` RAII remainder (~90 sites); dynamic `cargs` for `ulambda` JIT; critical RV64 JAL 21-bit offset overflow + HIR `iv.value` bounds; attr-cache stale-code leak (unbounded heap growth); Lua `m_cRef` non-atomic + `s_next_key` + `LuaAlloc` drift + snprintf cast; 5+ SQLite hygiene (null blobs, reset ordering, WAL BUSY, ROLLBACK, LoadAllAttrNames); numeric 9-digit threshold TODO. |
|
||||
| [GANL Networking](mux/ganl/ISSUES.md) | 9 | OpenSSL session ptr deref outside lock + `sslObjectsToFree` drain race; `recv()==0` / EOF mishandled (no Closing state); setsockopt (REUSE*, IPV6_V6ONLY, nonblock) return codes ignored; telnet subnegotiation stall on missing IAC SE + IAC IAC drop on full buf; no CA trust logging for STARTTLS. (Plus minor CHARSET client-list TODO.) |
|
||||
| [SQLSlave Module](mux/modules/sqlslave/ISSUES.md) | 9 | Non-atomic `m_cRef`/`g_c*` counters; `ConnectionHelper` derefs `m_pServer` w/o nullcheck; `Connect()` delete[] ownership contract (ABI hazard for out-of-proc); MySQL errors silent (`real_connect`, `next_result`, options); reconnect hook empty; broad `catch(...)`. |
|
||||
| [Test Infrastructure](testcases/ISSUES.md) | 5 | `isjson({"a":1})` JIT (preserves brace-group) vs legacy `parse_to` (strips) divergence — open which is "correct" (non-JIT path is the semantic bug per investigation); 84% SHA1 hash snapshots (brittle); edge/unicode/permission/overflow coverage gaps; single happy-path case per function norm. (Apple Silicon JIT now enabled 2026-05-02; re-probe recommended.) |
|
||||
|
||||
## Fully Closed Trackers (history preserved)
|
||||
|
||||
These trackers have no open items left; they are kept for their audit
|
||||
history and FIXED entries.
|
||||
|
||||
- [Mail Module](mux/modules/mail/ISSUES.md)
|
||||
- [Comsys Module](mux/modules/comsys/ISSUES.md)
|
||||
- [Hydra Proxy](mux/proxy/ISSUES.md)
|
||||
- [Android Client](client/android/ISSUES.md)
|
||||
- [TinyFugue Client](client/tf/ISSUES.md)
|
||||
- [Docker](docker/ISSUES.md)
|
||||
- [Debian Packaging](debian/ISSUES.md)
|
||||
- [WorldBuilder](tools/worldbuilder/ISSUES.md)
|
||||
- [Parser Research Tools](parser/ISSUES.md)
|
||||
- [Console Client](client/console/ISSUES.md)
|
||||
- [iOS Client](client/ios/ISSUES.md)
|
||||
- [DB Backend Tests](tests/db/ISSUES.md)
|
||||
- [Web Client](client/web/ISSUES.md)
|
||||
- [Win32 GUI Client](client/win32gui/ISSUES.md)
|
||||
- [Hydra Clients (aggregate)](client/ISSUES.md)
|
||||
|
||||
## Build System (top-level)
|
||||
|
||||
### ~~Subshell array variable loss in `dowin32.sh`~~ FIXED
|
||||
|
||||
- Both `find` loops in `process_distribution()` already use process substitution (`done < <(find ...)`), so the arrays are populated correctly in the parent shell.
|
||||
|
||||
### ~~Weak SSL/crypto library detection in configure.ac~~ FIXED
|
||||
|
||||
- **File:** `mux/configure.ac:273-274`
|
||||
- `configure.ac` now probes `SSL_new` in `-lssl` and `EVP_sha256` in `-lcrypto`, so `HAVE_LIBSSL`/`HAVE_LIBCRYPTO` reflect real OpenSSL entry points instead of a meaningless `main` symbol. OpenSSL remains mandatory via the existing `PKG_CHECK_MODULES([OPENSSL], [openssl], ...)` failure path.
|
||||
|
||||
### ~~Unquoted variable expansions in `dowin32.sh`~~ FIXED
|
||||
|
||||
- All variable expansions in `[ -e ... ]` tests, `rm` commands, and `ls` display lines are now properly double-quoted. Glob suffixes kept outside the quotes to preserve expansion.
|
||||
|
||||
### ~~Insecure external downloads in `dowin32.sh`~~ FIXED
|
||||
|
||||
- Generated `get_xdelta3.bat` and `get_patch.bat` now verify SHA256 hashes of downloaded archives via PowerShell `Get-FileHash` before extracting. Hash mismatches abort with an error and delete the untrusted download. Expected hashes are defined as `XDELTA3_SHA256` and `PATCH_SHA256` variables in `dowin32.sh`.
|
||||
|
||||
### ~~`dowin32.sh` lacks error handling~~ NOT A BUG
|
||||
|
||||
- Survey was incorrect: `dowin32.sh` already has `set -e` (line 8) and `set -o pipefail` (line 9).
|
||||
|
||||
### ~~Shell injection risk in `dowin32.sh` path handling~~ FIXED
|
||||
|
||||
- `dirname $rel_path` now properly quoted as `dirname "$rel_path"` at all four call sites.
|
||||
|
||||
### ~~Inconsistent diff/patch error handling between scripts~~ FIXED
|
||||
|
||||
- `dowin32.sh` now checks diff exit code: 0/1 are normal (identical/different), exit code >= 2 aborts with error message.
|
||||
125
client/ISSUES.md
125
client/ISSUES.md
|
|
@ -1,125 +0,0 @@
|
|||
# Hydra Client—Open Issues & Feature Gaps
|
||||
|
||||
## Active Sub-Trackers
|
||||
|
||||
- **[Console](console/ISSUES.md):** Console-only Hydra reconnect and terminal-capability bugs.
|
||||
- **[Android](android/ISSUES.md):** Android trigger/runtime issues.
|
||||
- **[iOS](ios/ISSUES.md):** Swift Hydra transport correctness and resource-lifetime issues.
|
||||
- **[TinyFugue](tf/ISSUES.md):** TF-specific Hydra transport and scripting gaps.
|
||||
- **[Win32 GUI](win32gui/ISSUES.md):** Windows GUI Hydra prompt/rendering and platform-validation gaps.
|
||||
|
||||
## Recently Fixed
|
||||
|
||||
- ~~Console/Android gRPC plaintext~~ — Fixed there: TLS by default landed for those clients (1192607). TF still pending below.
|
||||
- ~~Android Opens GameSession Without Input Path~~ — Fixed: bidi input (572843e)
|
||||
- ~~gRPC Subscribers Steal Messages~~ — Fixed server-side: per-subscriber queues (d0d5d05)
|
||||
- ~~OutputQueue Pre-rendering~~ — Fixed server-side: deferred per-subscriber rendering (634ce31)
|
||||
- ~~ColorFormat Negotiation~~ — Fixed: SetPreferences on GameSession stream (6853a2e)
|
||||
- ~~Auto-Reconnect Inconsistent~~ — Fixed: all clients retry with backoff (c1b7264)
|
||||
- ~~Browser localStorage Session Tokens~~ — Fixed: moved to sessionStorage (bcdb9f8)
|
||||
|
||||
## Bugs & Security Risks
|
||||
|
||||
### ~~TF Hydra Transport Still Uses Plaintext gRPC~~
|
||||
|
||||
- **Fixed:** TF now uses SslCredentials when world has SSL flag. (3d496b9)
|
||||
|
||||
### ~~HTML5 Protobuf Encoder Edge Cases~~
|
||||
|
||||
- **Mitigated:** Proto enum shifted to COLOR_UNSPECIFIED=0 so proto3 zero-default is harmless. HTML5 Subscribe explicitly sends ANSI_TRUECOLOR=1. (3d496b9)
|
||||
|
||||
### ~~Android Reconnect `inputChannel` Lifecycle Bug~~
|
||||
|
||||
- **Fixed:** inputChannel is now @Volatile var, replaced with fresh Channel on reconnect. (ffb0026)
|
||||
|
||||
### ~~`fetchScrollBack()` Ignores `color_format`~~
|
||||
|
||||
- **Fixed:** Console and Android set color_format = ANSI_TRUECOLOR. (3096196)
|
||||
|
||||
### ~~TF `send_naws` Hardcodes `ColorFormat`~~
|
||||
|
||||
- **Fixed:** TF now tracks `currentColorFormat_` and uses it in send_naws. (3d496b9)
|
||||
|
||||
### ~~Web Client Uses Legacy Unary/Server-Streaming Instead of GameSession~~
|
||||
|
||||
- **Fixed:** WebSocket GameSession transport with `hydra-gamesession` subprotocol. Binary protobuf ClientMessage/ServerMessage frames over WS, first-message auth via SetPreferences.session_id. Legacy grpc-web path retained as fallback. (55e6bb832)
|
||||
|
||||
### ~~Reconnect Paths Reopen `GameSession` Without Re-Sending Preferences~~
|
||||
|
||||
- **Fixed:** Console and Android now resend SetPreferences on reconnect. TF still pending. (ffb0026)
|
||||
|
||||
## Feature Gaps
|
||||
|
||||
### ~~Clients Do Not Send Initial Capability Message Consistently~~
|
||||
|
||||
- **Fixed:** All three C++ and Android clients now send `SetPreferences` on stream open. (b8661cf)
|
||||
|
||||
### ~~`GetScrollBack` color_format Not Used~~
|
||||
|
||||
- **Fixed:** Console and Android now set color_format = ANSI_TRUECOLOR on scroll-back requests. (3096196)
|
||||
|
||||
### ~~Reported Terminal Size Is Still Hardcoded On Native Hydra Clients~~
|
||||
|
||||
- **Fixed:** All clients now send actual terminal dimensions. Console reports window size, Android estimates from screen dp, TF sends ncurses cols/rows after connect. (1d2a548, a9df518)
|
||||
|
||||
### ~~Browser grpc-web Path Still Cannot Choose Live Output Color Format~~
|
||||
|
||||
- **Fixed:** HTML5 Subscribe now sends `color_format=1` (ANSI_TRUECOLOR) explicitly. (3d496b9)
|
||||
|
||||
### ~~Scrollback Fetch on Reconnect~~
|
||||
|
||||
- **Fixed:** Console and Android now call GetScrollBack (200 lines) after successful reconnect. (46ba394)
|
||||
- **Console session persistence:** FIXED. The Console client now
|
||||
persists `hydra_session` to `worlds.txt` as an optional
|
||||
`session=<id>` token on `hydra` lines. `HydraConnection::connect()`
|
||||
tries `GetSession(saved_id)` before the normal
|
||||
`Authenticate(username, password)` path — if the server still has
|
||||
the session alive and the username matches, the client resumes
|
||||
without re-entering the password. On any probe failure the token
|
||||
is discarded and the full auth path runs. The new session_id is
|
||||
written back to `worlds.txt` after each successful connect.
|
||||
Win32GUI shares the same `hydra_connection.cpp` and compiles
|
||||
unchanged through the defaulted constructor parameter — wiring
|
||||
it through the GUI is a small follow-up.
|
||||
- **Android session persistence:** still pending — needs a matching
|
||||
change in an Android-capable environment.
|
||||
|
||||
### ~~Create-Account Flow Not Exposed~~
|
||||
|
||||
- **Fixed:** /hcreate command added to Console, Win32GUI, and Android. (098c2c2)
|
||||
|
||||
### ~~GMCP Support Is Raw JSON Only~~ FIXED
|
||||
|
||||
- Hydra clients now special-case `Char.Vitals` and emit a structured vitals summary when common fields are present, while preserving the raw `[GMCP ...]` fallback for unknown or malformed payloads. The shared C++ Hydra transport (`client/console/src/hydra_connection.cpp`) covers Console and Win32 GUI; TinyFugue, Android, iOS, and Web each gained the same `Char.Vitals` formatting path in their platform-specific Hydra readers. Web regression coverage now asserts the structured vitals rendering in `client/web/test_web.js`. Local verification on this host covered `g++ -std=c++17 -fsyntax-only` for the Console/TF C++ readers and `node client/web/test_web.js`; Android/iOS remain source-reviewed only in this environment.
|
||||
|
||||
## Cross-Client Issues
|
||||
|
||||
### ~~Credential storage in plaintext~~ FIXED
|
||||
|
||||
- **Console/Win32 Console:** `worlds.txt` now saved with mode 0600 on Unix; load warns if permissions are loose. `secure_zero()` wipes the password from memory after Hydra authentication succeeds.
|
||||
- **Win32 GUI:** Hydra passwords moved from plaintext `worlds.json` to Windows Credential Manager (`CredWriteW`/`CredReadW`). Transparent migration from existing JSON on first load; passwords stripped from JSON on next save.
|
||||
- **iOS:** Already used Apple Keychain (`SecItem*` APIs) — no change needed.
|
||||
- **Android:** Already used `EncryptedSharedPreferences` with AES-256-GCM — no change needed.
|
||||
|
||||
### ~~Spawn config regex errors silently discarded~~ FIXED
|
||||
|
||||
- **File:** `console/src/spawn.cpp:9-10, 24-25`
|
||||
- **Issue:** `try { compiled.push_back(std::regex(...)); } catch (...) {}` silently swallows regex compilation errors. Users get no feedback when a spawn rule is invalid.
|
||||
|
||||
## Newly Confirmed Regressions
|
||||
|
||||
### ~~Console reconnect path still hardcodes `80x24`~~ FIXED
|
||||
|
||||
- Reconnect path now uses cached `termWidth_`/`termHeight_` instead of hardcoded 80x24, matching the TF fix.
|
||||
|
||||
### ~~iOS Hydra client still does not send `SetPreferences`~~ FIXED
|
||||
|
||||
- `runGameSession()` now sends `SetPreferences` (ANSI_TRUECOLOR, 80x24, "Titan-iOS") as the first bidi stream message. Also fixed: `EventLoopGroup` leak on repeated connect/disconnect.
|
||||
|
||||
### ~~SwiftUI / iOS Hydra Support Missing~~
|
||||
|
||||
- **Fixed:** HydraConnection.swift written with full feature parity, guarded behind #if canImport(GRPC). Needs Mac build with grpc-swift package. (6858c4d)
|
||||
|
||||
### ~~Hydra Command Surface Inconsistent Across Clients~~
|
||||
|
||||
- **Fixed:** All clients now delegate /h* commands to HydraConnection:: handleCommand(). One place to add new commands per language. (43fb05b)
|
||||
|
|
@ -1,28 +0,0 @@
|
|||
# Titan Android Client — Open Issues
|
||||
|
||||
Updated: 2026-03-29
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~Trigger conditions never receive real idle time~~ FIXED
|
||||
|
||||
- Trigger evaluation now uses per-tab/transport idle tracking instead of a
|
||||
hardcoded `0`.
|
||||
|
||||
### ~~Potential race on `inputChannel` during reconnect~~ FIXED
|
||||
|
||||
- The Android Hydra client now swaps the active bidi input channel under a
|
||||
lock, so reconnect and send paths do not race on different stream channels.
|
||||
|
||||
## Newly Confirmed
|
||||
|
||||
### ~~Hydra reconnect resends stale `80x24` terminal geometry~~ FIXED
|
||||
|
||||
- Reconnect now resends the original computed terminal width/height instead of
|
||||
falling back to `80x24`.
|
||||
|
||||
### ~~Vendored Android `hydra.proto` lagged the proxy schema~~ FIXED
|
||||
|
||||
- Android now includes `GameOutput.end_of_record`, which lets the client
|
||||
preserve prompt boundaries instead of treating every `GameOutput.text` chunk
|
||||
as a complete display line.
|
||||
|
|
@ -1,40 +0,0 @@
|
|||
# TinyMUX Console Client — Open Issues
|
||||
|
||||
Updated: 2026-04-10
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~Hydra reconnect resends stale `80x24` terminal geometry~~ FIXED
|
||||
|
||||
- Reconnect path now uses cached `termWidth_`/`termHeight_` (set from constructor) instead of hardcoded 80x24.
|
||||
|
||||
### ~~Pending overlapped write teardown is still unsafe~~ FIXED
|
||||
|
||||
- **File:** `client/console/src/app.h`, `connection.h`, `connection.cpp`, `command.cpp`
|
||||
- `app.connections` is now `unordered_map<string, shared_ptr<IConnection>>` and `Connection` inherits `std::enable_shared_from_this<Connection>`. Each heap-allocated write `IoContext` carries a `std::shared_ptr<Connection> owner` populated via `shared_from_this()` in `send_raw()`, so pending writes keep the `Connection` alive until their completion fires and deletes the ctx — even if `app.connections` already erased the map entry. Read and connect overlapped slots are embedded in `Connection`, so they now also pin the object via `pending_read_self_` / `pending_connect_self_` self-references that are set before `WSARecv` / `ConnectEx` and cleared on the matching completion (or on the synchronous error path). `on_completion()` takes a local `keepalive = shared_from_this()` at entry, which prevents `*this` from being destroyed mid-method when a write ctx owning the last reference is deleted. `memset` on the full `IoContext` struct was removed in favour of value-initialization (the `owner` field is a non-trivial `shared_ptr` now). Verification is deferred to a Windows build — the console client requires `<windows.h>`, `<winsock2.h>`, `<mswsock.h>`, etc. and does not compile on this Linux host.
|
||||
|
||||
### ~~Concurrent `grpc_` access lacks full synchronization~~ FIXED
|
||||
|
||||
- Hydra transport state now uses a mutex-protected shared snapshot, so
|
||||
reconnect/RPC/disconnect paths do not race on a raw `grpc_` pointer.
|
||||
|
||||
### ~~`TerminateThread()` used as fallback in cleanup~~ FIXED
|
||||
|
||||
- Console shutdown now cancels the blocking read and waits briefly, but no
|
||||
longer force-kills the input thread.
|
||||
|
||||
### ~~World file parsing has no field validation~~ FIXED
|
||||
|
||||
- Both `world` and `hydra` lines now check the `>>` extraction result. Malformed lines are skipped with a diagnostic to stderr.
|
||||
|
||||
## Bugs (New, 2026-04-04)
|
||||
|
||||
### ~~Telnet CHARSET negotiation validation gap~~ FIXED
|
||||
|
||||
- The CHARSET option handler now validates that the delimiter is printable, trims ASCII whitespace around offered names, and caps parsing to 50 offered charsets before replying. Local syntax-only verification is blocked in this environment because `client/console` requires Windows headers.
|
||||
|
||||
## Bugs (New, 2026-04-10)
|
||||
|
||||
### ~~`/def` accepts invalid regex triggers and substitutions without surfacing an error~~ FIXED
|
||||
|
||||
- `Macro::compile()` now reports `std::regex_error` details back to the caller instead of silently leaving a dead trigger behind. `parse_def()` validates both trigger regexes and substitution regexes up front, so `cmd_def()` now rejects malformed `/def -t...` and `/def -s...` rules with an explicit error instead of storing a broken macro and printing `Defined: ...`. Reverified with `g++ -std=c++17 -fsyntax-only -I client/console/src -I mux/include -I ragel client/console/src/macro.cpp`.
|
||||
|
|
@ -1,52 +0,0 @@
|
|||
# Titan iOS Client — Open Issues
|
||||
|
||||
Updated: 2026-04-10
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~Hydra `GameSession` never sends initial `SetPreferences`~~ FIXED
|
||||
|
||||
- `runGameSession()` now yields a `SetPreferences` message (ANSI_TRUECOLOR, 80x24, "Titan-iOS") as the first item on the bidi stream, before the ping task starts. Sent on both initial connect and reconnect since `runGameSession` is called from both paths.
|
||||
|
||||
### ~~`EventLoopGroup` lifetime is unmanaged across connects~~ FIXED
|
||||
|
||||
- `EventLoopGroup` is now stored as an instance variable, reused across reconnects, and shut down via `syncShutdownGracefully()` in `disconnect()`.
|
||||
|
||||
### ~~Hardcoded 80x24 viewport~~ FIXED
|
||||
|
||||
- Initial Hydra `SetPreferences` now uses an estimated terminal size derived from
|
||||
the current screen bounds and configured font size instead of a fixed `80x24`.
|
||||
|
||||
## Newly Confirmed
|
||||
|
||||
### ~~Hydra output ignored prompt/end-of-record boundaries~~ FIXED
|
||||
|
||||
- The iOS Hydra client now buffers `gameOutput.text` and flushes partial lines
|
||||
on `endOfRecord`, instead of assuming every chunk is a complete display line.
|
||||
|
||||
### ~~Trigger/timer automation was telnet-only~~ FIXED
|
||||
|
||||
- `ConditionContext` and trigger-command execution now use `WorldTab` transport
|
||||
state instead of only `tab.connection`, so Hydra tabs participate in idle,
|
||||
connected, timer, and trigger-command behavior.
|
||||
|
||||
## Remaining Enhancements
|
||||
|
||||
### ~~Hydra viewport sizing still uses a coarse screen-bounds estimate~~ FIXED
|
||||
|
||||
- Replaced `UIScreen.main.bounds` estimation with a `GeometryReader` on the
|
||||
output pane. Terminal columns/rows are now computed from the actual view
|
||||
dimensions and configured font size.
|
||||
|
||||
### ~~Hydra terminal size is not updated after connect~~ FIXED
|
||||
|
||||
- `ContentView.outputPane` now sends a fresh `SetPreferences` via
|
||||
`HydraConnection.updateTerminalSize()` whenever the output pane geometry
|
||||
changes (device rotation, split-screen, etc.). The update is suppressed
|
||||
when the computed dimensions haven't changed.
|
||||
|
||||
## Bugs (New, 2026-04-10)
|
||||
|
||||
### ~~Hydra reconnect path never fetches missed scroll-back~~ FIXED
|
||||
|
||||
- `runGameSession()` now accepts a reconnect-only `fetchScrollbackOnOpen` path. `attemptReconnect()` reopens `GameSession`, then immediately issues `GetScrollBack` with `ANSI_TRUECOLOR` and appends the returned lines before resuming live stream processing, matching the Android client's recovery behavior. Local verification on this host is limited because `swiftc` is not installed.
|
||||
|
|
@ -1,98 +0,0 @@
|
|||
# TitanFugue C++ Client — Open Issues
|
||||
|
||||
Updated: 2026-03-29
|
||||
|
||||
## Bugs
|
||||
|
||||
All previously reported bugs have been resolved:
|
||||
|
||||
- ~~`substitute()` only replaces last visual line~~ — Fixed: `replace_last_output_line` tracks `last_logical_line_count` and pops all visual lines of the logical line.
|
||||
- ~~`Ctrl-C` exits immediately~~ — Fixed: SIGINT handler sets flag, event loop prints "Interrupt: /quit to exit" instead of exiting.
|
||||
- ~~`compile_trigger` treats `simple` as `glob`~~ — Fixed: `simple` and `substr` both call `regex_escape()` for literal matching.
|
||||
|
||||
## ~~Newly Confirmed Bugs~~ FIXED
|
||||
|
||||
- ~~**Hydra reconnect drops capability negotiation**~~ FIXED — `sendPreferences()` is now called after stream reopen in `attemptReconnect()`, resending color format, terminal size, and terminal type.
|
||||
|
||||
- ~~**Initial Hydra viewport is still hardcoded to `80x24`**~~ FIXED — Terminal dimensions are cached in `termWidth_`/`termHeight_` instance variables, updated by `send_naws()` (called on SIGWINCH). Both `openStream()` and `attemptReconnect()` use the cached values. The initial 80x24 default still applies until the first `send_naws()` call, which happens immediately after connect from `main.cpp`.
|
||||
|
||||
- ~~**Hydra-enabled builds fail from source**~~ FIXED — `client/tf/CMakeLists.txt` now points protobuf generation at `mux/proxy/hydra.proto` instead of a nonexistent top-level `proxy/hydra.proto`. Verified with `cmake -S client/tf -B /tmp/tf-build-hydra -DHYDRA_GRPC=ON` and `cmake --build /tmp/tf-build-hydra -j2`.
|
||||
|
||||
- ~~**Hydra stream output ignored line framing and EOR prompts**~~ FIXED — `HydraConnection` now buffers `GameOutput.text`, emits only complete newline-terminated lines to the UI, preserves partial text, and treats `GameOutput.end_of_record` as a prompt boundary for `check_prompt()`/`current_prompt()`.
|
||||
|
||||
- ~~**`/hrestart` only matched when followed by a space**~~ FIXED — `HydraConnection::send_line()` now recognizes bare `/hrestart` and routes it to the existing usage/help path instead of forwarding it to the active game.
|
||||
|
||||
## Newly Identified Bugs (2026-04-04)
|
||||
|
||||
- ~~**Hydra reconnect path still races on `grpc_` ownership**~~ FIXED — TF Hydra transport now keeps gRPC/channel state in shared snapshots guarded by mutexes, uses a dedicated stream snapshot for the reader thread, and serializes stream writes so disconnect/reconnect no longer race on raw `grpc_` ownership.
|
||||
|
||||
- ~~**`/update` builds a shell command from unquoted user input**~~ FIXED — `/update` now runs `git pull` and `cmake --build` via direct `execvp` argument vectors in controlled working directories, removing the `sh -c` injection path for branch names and repo paths.
|
||||
|
||||
## Newly Identified Bugs (2026-04-04) — Continued
|
||||
|
||||
- ~~**Incomplete gRPC channel error handling**~~ FIXED — TF Hydra connect/reconnect now validates channel and stub creation explicitly and emits targeted diagnostics when transport state is missing during stream open or reconnect.
|
||||
|
||||
## Stubbed Or Partially Implemented Interfaces
|
||||
|
||||
- ~~**Lack of multi-key binding support**~~
|
||||
**Fixed:** `KeyBindings` uses a trie (`SeqTrieNode`) for multi-key
|
||||
sequence matching. `parse_key_sequence()` handles `^X^F`, `Meta-a`,
|
||||
`Esc a`, and mixed formats. Timeout-based disambiguation in event loop.
|
||||
|
||||
- **Nested keyboard read (`tfread()`) is missing**
|
||||
The `tfread()` scripting function only supports reading from file handles.
|
||||
Classic TF's `tfread()` can also read from the keyboard, allowing macros
|
||||
to pause for user input.
|
||||
|
||||
- **`read()` scripting function is still a stub**
|
||||
`client/tf/src/script_parse.cpp:890-892` always returns an empty string and explicitly labels the implementation as a stub. Scripts that expect interactive input currently fail silently.
|
||||
|
||||
- **`@read` depth counter only incremented by future `/read` command**
|
||||
The `status_read_depth` backing field is wired into the status bar
|
||||
but no command currently increments it. TitanFugue would need a
|
||||
`/read` command or nested `tfread()` to make `@read` useful.
|
||||
|
||||
- **Format variable evaluation does not cache compiled expressions**
|
||||
`status_int_*` and `status_var_*` variables are re-parsed on every
|
||||
status bar redraw. Unlikely to matter in practice since redraws are
|
||||
infrequent.
|
||||
|
||||
- ~~**`nlog` status/function support is still a stub**~~
|
||||
**Fixed:** `nlog()` now returns the active connection's successful per-world log-line count. The counter resets when logging starts or stops and increments only on actual writes.
|
||||
|
||||
- ~~**Redundant per-world logging overrides in `Connection`**~~
|
||||
**Fixed:** Removed the duplicate `start_log`, `stop_log`, and `log_line` overrides from `Connection`; TF now uses the shared `IConnection` logging implementation directly.
|
||||
|
||||
- ~~**Classic TF attribute codes: `E`, `W`, `I` meta-attrs not implemented**~~
|
||||
**Fixed:** `E`/`W`/`I` expand from `error_attr`/`warning_attr`/`info_attr`
|
||||
variables. Defaults: bold red, bold yellow, cyan. Word-token equivalents:
|
||||
`error`, `warning`, `info`. Users can override via `/set error_attr=...`.
|
||||
|
||||
## Previously Reported — Now Resolved
|
||||
|
||||
- ~~`SIGTSTP` (Ctrl-Z) support~~ — Implemented: proper shutdown/raise/reinit cycle with handler reinstall on resume.
|
||||
- ~~Shell process read loop EAGAIN~~ — Fixed: non-blocking read now distinguishes EAGAIN from real errors, closes fd on non-EAGAIN failures.
|
||||
|
||||
## Charset Support
|
||||
|
||||
Supported charsets (bidirectional, negotiated via telnet CHARSET option 42):
|
||||
|
||||
| Charset | Aliases | Use case |
|
||||
|---------|---------|----------|
|
||||
| UTF-8 | utf8 | Modern MUDs |
|
||||
| US-ASCII | ascii | 7-bit only |
|
||||
| ISO-8859-1 | latin1 | Western European MUDs |
|
||||
| CP437 | ibm437 | FANSI art, DOS-era MUDs |
|
||||
| Windows-1252 | cp1252, win1252 | Smart quotes, web text |
|
||||
| KOI8-R | | Russian Cyrillic MUDs |
|
||||
|
||||
## Notes on Hydra Reconnect
|
||||
|
||||
Both newly confirmed bugs above share a common fix pattern: cache the actual terminal dimensions and color format in the `HydraConnection` object, then use them in both the initial connect and reconnect paths.
|
||||
|
||||
## Notes
|
||||
|
||||
- Status bar is at parity with classic TF 5.0.
|
||||
- TrueColor (24-bit) rendering via ncurses with CIE97 perceptual fallback.
|
||||
- MCCP v2 (telnet option 86) zlib decompression.
|
||||
- Full telnet: ECHO, SGA, TTYPE, NAWS, BINARY, CHARSET negotiation.
|
||||
|
|
@ -1,64 +0,0 @@
|
|||
# Titan Web Client -- Open Issues
|
||||
|
||||
Updated: 2026-04-10
|
||||
|
||||
## Fixed In This Audit
|
||||
|
||||
- **Hydra stream framing and prompt handling** -- Fixed.
|
||||
`HydraConnection` now buffers `GameOutput.text`, emits complete newline-
|
||||
terminated lines to the UI, and treats `GameOutput.end_of_record` as a
|
||||
prompt boundary instead of rendering arbitrary stream chunks as full lines.
|
||||
This applies to both the WebSocket GameSession path and the grpc-web
|
||||
Subscribe fallback.
|
||||
|
||||
- **Hydra protobuf field-map drift** -- Fixed.
|
||||
The local decode maps now match current `mux/proxy/hydra.proto` for
|
||||
`GameOutput.end_of_record`, `ServerMessage.notice`, and `ServerMessage.link_event`.
|
||||
|
||||
- **`/hrestart` only matched with a trailing space** -- Fixed.
|
||||
Bare `/hrestart` now reaches the existing usage/error handling path instead
|
||||
of falling through as normal game input.
|
||||
|
||||
- **Quick-connect autosave dropped Hydra world settings** -- Fixed.
|
||||
Connecting a Hydra world now persists `transport`, `character`, `password`,
|
||||
and `game`, instead of collapsing the saved world back to a plain websocket
|
||||
entry.
|
||||
|
||||
## ~~Open — Bugs (New, 2026-04-04)~~ FIXED
|
||||
|
||||
- **ANSI 256-color/truecolor parser missing bounds checks** — **FIXED**
|
||||
- **File:** `client/web/js/terminal.js`
|
||||
- The `renderAnsiLine()` SGR parser now validates each palette index and RGB component against `Number.isInteger(v) && v >= 0 && v <= 255` before using it for `\e[38;5;Nm` (256-color fg), `\e[38;2;R;G;Bm` (truecolor fg), and the matching `48;...` background forms. Short/malformed sequences fall through to plain text instead of leaking `undefined` or `NaN` into CSS, and out-of-range component values are also rejected. Verified with `node --check` and a 15-case harness covering well-formed, short, and out-of-range inputs against the extracted `renderAnsiLine` function.
|
||||
|
||||
## Open
|
||||
|
||||
- **Saved world passwords live in browser localStorage** -- **FIXED**
|
||||
World metadata still lives in `localStorage`, but saved world passwords are
|
||||
now migrated out of the persisted settings blob into tab-scoped
|
||||
`sessionStorage`. `Settings.load()` strips any legacy `password` fields from
|
||||
stored worlds on first load, `getWorlds()` rehydrates passwords from the
|
||||
session store for the current tab, and world deletion / rename paths remove
|
||||
stale password entries instead of leaving them behind.
|
||||
|
||||
- **No automated browser-level regression coverage**
|
||||
**FIXED**
|
||||
A checked-in Node-based browser shim harness now exercises the real web
|
||||
scripts with fake `fetch`, `WebSocket`, `localStorage`, and
|
||||
`sessionStorage`. `client/web/test_web.js` covers settings password
|
||||
migration, Hydra fresh auth, session resume, WebSocket GameSession startup,
|
||||
prompt framing, grpc-web Subscribe fallback, and reconnect behavior.
|
||||
|
||||
## Verification
|
||||
|
||||
- `node client/web/test_web.js`
|
||||
- `node --check client/web/js/settings.js`
|
||||
- `node --check client/web/js/hydra_connection.js`
|
||||
- `node --check client/web/js/main.js`
|
||||
|
||||
## Notes
|
||||
|
||||
- `client/web` has no prior `ISSUES.md`; this file starts the current audited
|
||||
baseline.
|
||||
- The Hydra path is materially closer to usable than it looked from memory.
|
||||
The main problems found here were integration drift and framing bugs, not a
|
||||
missing client architecture.
|
||||
|
|
@ -1,45 +0,0 @@
|
|||
# TinyMUX Win32 GUI Client — Open Issues
|
||||
|
||||
Updated: 2026-03-29
|
||||
|
||||
## Audit Notes
|
||||
|
||||
- `client/win32gui` shares its Hydra transport implementation with
|
||||
`client/console/src/hydra_connection.*` via
|
||||
[win32gui.vcxproj](/home/sdennis/tinymux/client/win32gui/win32gui.vcxproj).
|
||||
- This audit re-verified the Win32 GUI Hydra path against the current
|
||||
`mux/proxy/hydra.proto` schema instead of assuming the older review still
|
||||
held.
|
||||
|
||||
## Fixed In This Pass
|
||||
|
||||
### Hydra `end_of_record` boundaries were ignored
|
||||
|
||||
- **Files:** `client/console/src/hydra_connection.cpp`,
|
||||
`client/console/src/app.cpp`, `client/win32gui/src/mainframe.cpp`,
|
||||
`client/win32gui/src/outputbuffer.cpp`
|
||||
- **Issue:** The shared Hydra reader treated all `GameOutput.text` as one raw
|
||||
stream and never surfaced `GameOutput.end_of_record`. In practice, prompts
|
||||
could merge into the next server line, and the console loop would clear Hydra
|
||||
partial lines immediately because `has_partial_line()` was hardcoded false.
|
||||
- **Fix:** The shared transport now tracks partial-line state and propagates an
|
||||
`end_of_record` flag to consumers. The console finalizes prompt records
|
||||
cleanly, and the Win32 GUI seals the open output line at record boundaries so
|
||||
subsequent output starts on a new line.
|
||||
|
||||
## Open Issues
|
||||
|
||||
### ~~No current Linux-side build validation for the Visual Studio target~~ FIXED
|
||||
|
||||
- Added `client/win32gui/validate_vcxproj.py`, a Linux-side validator for the
|
||||
Visual Studio project. It parses `win32gui.vcxproj`, verifies that all
|
||||
referenced compile/include/resource paths exist, checks that the expected
|
||||
`Debug|x64` and `Release|x64` configurations are present, and asserts that
|
||||
the key shared Hydra/console source files remain wired into the project.
|
||||
Reverified with `python3 client/win32gui/validate_vcxproj.py`.
|
||||
- This does not replace a real Windows build/run pass, but it closes the
|
||||
specific "no Linux-side validation at all" gap that the tracker called out.
|
||||
|
||||
### ~~Credentials remain plaintext in world storage~~ FIXED
|
||||
|
||||
- Hydra passwords moved from plaintext `worlds.json` to Windows Credential Manager via `CredWriteW`/`CredReadW` (`CRED_TYPE_GENERIC`, `CRED_PERSIST_LOCAL_MACHINE`). Existing JSON passwords are transparently migrated to the credential store on first load and stripped from JSON on next save.
|
||||
35
debian/ISSUES.md
vendored
35
debian/ISSUES.md
vendored
|
|
@ -1,35 +0,0 @@
|
|||
# Debian Packaging — Open Issues
|
||||
|
||||
Updated: 2026-03-27
|
||||
|
||||
## ~~High — Missing Build Dependencies~~ FIXED
|
||||
|
||||
### ~~`debian/control` does not declare all required build dependencies~~ FIXED
|
||||
|
||||
- Added `g++`, `ragel`, `pkg-config`, `libpcre2-dev`, `libssl-dev` to `Build-Depends`.
|
||||
|
||||
## Medium — Outdated Metadata
|
||||
|
||||
### ~~Documentation references TinyMUX 2.12~~ FIXED
|
||||
|
||||
- Updated all references in `README.Debian` from 2.12 to 2.14.
|
||||
|
||||
### ~~Standards-Version is from 2017~~ FIXED
|
||||
|
||||
- Updated from `4.0.1` to `4.6.2`.
|
||||
|
||||
## Medium — Packaging Quality
|
||||
|
||||
### ~~Hardcoded `--enable-stubslave` in rules~~ INTENTIONAL
|
||||
|
||||
- `--enable-stubslave` is correct for release/distribution packages per CLAUDE.md. Omitting it is only for local smoke testing.
|
||||
|
||||
### ~~Game data files installed world-readable, no postinst permissions~~ FIXED
|
||||
|
||||
- Added `debian/postinst` that restricts config files (`netmux.conf`, `mux.config`, `muxssl.conf`, `alias.conf`, `compat.conf`) to mode 640 on configure.
|
||||
|
||||
## Low — Convert Tool Build
|
||||
|
||||
### ~~`convert/configure.ac` has weak yacc/lex checking~~ FIXED
|
||||
|
||||
- Added `AC_MSG_ERROR` if yacc/bison or lex/flex are not found. Also fixed `AC_CHECK_LIB` to check for `yywrap` instead of `main`.
|
||||
|
|
@ -1,15 +0,0 @@
|
|||
# Docker — Open Issues
|
||||
|
||||
Updated: 2026-03-27
|
||||
|
||||
## ~~Critical — Outdated Images~~ FIXED
|
||||
|
||||
### ~~Dockerfile references MUX 2.12~~ FIXED
|
||||
|
||||
- Synced from `~/g/tinymux/Dockerfile` which targets MUX 2.13.0.11 with multi-stage build, pcre2, sqlite, and proper module installation. AnonymousMUX base image tag updated to match.
|
||||
|
||||
## ~~Medium — Build Configuration~~ FIXED
|
||||
|
||||
### ~~No multi-stage build~~ FIXED
|
||||
|
||||
- The current Dockerfile already uses multi-stage build (builder stage with build tools, runtime stage with only `libstdc++`, `libc6-compat`, `openssl`, `pcre2`, `sqlite`).
|
||||
|
|
@ -1,108 +0,0 @@
|
|||
# GANL (Global Adaptive Network Layer) — Open Issues
|
||||
|
||||
Updated: 2026-05-22 (post Apple Silicon JIT; CHARSET TODO noted)
|
||||
|
||||
## High — Implementation Gaps
|
||||
|
||||
### ~~Missing waiting logic in `connection.cpp`~~ FIXED
|
||||
- **File:** `mux/ganl/src/connection.cpp:558`
|
||||
- TLS shutdown now defers `networkEngine_.closeConnection()` until pending output drains. `close()` keeps the connection in `Closing`, allows `postWrite()` while draining queued shutdown bytes, and both readiness and IOCP write handlers finalize the socket close only after `encryptedOutput_` is empty.
|
||||
|
||||
### ~~Missing password callback for encrypted keys~~ FIXED
|
||||
- **File:** `mux/ganl/src/openssl_transport.cpp:75`
|
||||
- `OpenSSLTransport` now installs an `SSL_CTX` password callback backed by `TlsConfig::password`, so PEM private keys protected with passphrases can be loaded without pre-decrypting them on disk.
|
||||
|
||||
## Medium — Protocol Handling Gaps
|
||||
|
||||
### ~~Missing ANSI/MXP processing~~ FIXED
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:597`
|
||||
- `formatOutput()` now respects negotiated capabilities: ANSI-less clients have CSI/OSC escape sequences stripped before transmission, and non-MXP clients have obvious MXP tags suppressed instead of receiving raw markup.
|
||||
|
||||
### ~~Incomplete NEW-ENVIRON parsing~~ FIXED
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:1114, 1120`
|
||||
- `TelnetProtocolHandler` now sends `SB NEW-ENVIRON SEND` after `WILL NEW-ENVIRON`, parses `VAR/USERVAR/VALUE/ESC` sequences from `IS` and `INFO`, updates width/height plus ANSI/MXP capability hints from environment values, and answers `SEND` with a bounded `SB NEW-ENVIRON IS` reply.
|
||||
|
||||
## High — Memory Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Unbounded input buffer growth in telnet protocol handler~~ FIXED
|
||||
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:498, 505, 542, 557`
|
||||
- `TelnetProtocolHandler` now caps `inputBuffer` at 8 KB and `subnegotiationBuffer` at 4 KB. Overflow sets `lastError` and returns `false` from `processInput()`, which closes the connection as a protocol error.
|
||||
|
||||
### ~~Dangling pointer risk in select_network_engine.cpp~~ FIXED
|
||||
|
||||
- **File:** `mux/ganl/src/select_network_engine.cpp:480, 711-716, 773`
|
||||
- `SelectNetworkEngine` no longer stores a borrowed `IoBuffer*` from `postRead()`. Readiness events now carry `nullptr` for `IoEvent.buffer`, leaving buffer ownership entirely with `Connection`.
|
||||
|
||||
## Medium — Protocol Safety (New, 2026-04-04)
|
||||
|
||||
### ~~No NAWS dimension validation~~ FIXED
|
||||
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:1029-1034`
|
||||
- NAWS width/height are now validated to `1..1000`; out-of-range values are reset to the handler defaults of `80x24`.
|
||||
|
||||
### ~~Unbounded TTYPE response length~~ FIXED
|
||||
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:1062-1069`
|
||||
- Outbound TTYPE responses now clamp `clientTtype` to 256 bytes before appending it to the subnegotiation reply.
|
||||
|
||||
### ~~No negotiation timeout mechanism~~ FIXED
|
||||
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:341-374`, `mux/ganl/src/connection.cpp`, `mux/ganl/src/*_network_engine.cpp`
|
||||
- Idle `processEvents()` timeouts now sweep active connections and call `ConnectionBase::checkNegotiationTimeout()`, so stalled telnet handshakes age out even when the client goes silent after the initial negotiation bytes.
|
||||
|
||||
## Low — Technical Debt
|
||||
|
||||
### ~~Missing configuration for StartTLS~~ FIXED
|
||||
- **File:** `mux/ganl/include/telnet_protocol_handler.h:57`
|
||||
- `TelnetProtocolHandler` now stores an `offerStartTls_` policy with a constructor argument and setter, so integrations can explicitly disable STARTTLS offers instead of being locked to a hardcoded `true`.
|
||||
|
||||
## High — Concurrency & Lifetime (New, 2026-04-10)
|
||||
|
||||
### OpenSSL context pointer dereferenced outside session map lock
|
||||
- **File:** `mux/ganl/src/openssl_transport.cpp` (approximately line 307, `SSL_read`/`SSL_write` path)
|
||||
- **Issue:** The transport looks up the per-session `SSL *` by connection id under `sessionsMutex_`, extracts a borrowed pointer, drops the lock, and then dereferences the pointer. A concurrent `destroySessionContext()` call (e.g., triggered by a close readiness event on another thread, or by the engine from inside a write completion) can remove the map entry and free the `SSL *` between the two operations.
|
||||
- **Fix:** Hold the lock for the whole `SSL_*` call, or promote the map value to `std::shared_ptr<SslSession>` so lookup returns an owning handle.
|
||||
|
||||
### `sslObjectsToFree` drained outside the lock that protected it
|
||||
- **File:** `mux/ganl/src/openssl_transport.cpp:155-157, 218, 269`
|
||||
- **Issue:** `shutdown()` (and the shutdown paths driven from close-notify) iterate `sessions_` under a lock, copy `SSL *` pointers into a local `std::vector`, unlock, and then call `SSL_free()` on each. `SSL_free` in turn frees the attached `BIO *`. If another thread has just re-added a session with a colliding id (or if the read/write path is still holding the bare pointer from the audit above), the second access fires on freed memory.
|
||||
|
||||
### `recv()` / `read()` returning 0 treated like any other "no more data"
|
||||
- **File:** `mux/ganl/src/connection.cpp:1022-1024` (approximate)
|
||||
- **Issue:** The readiness loop breaks on `read() == 0` without marking the connection as peer-closed. Downstream code still attempts `postWrite()` and keeps the negotiation-timeout timer running on a half-closed socket. In the IOCP path the symmetric bug is that `CompletionConnection::handleRead()` silently ignores `postRead()` failures after a close, leaking the connection from the event loop.
|
||||
- **Fix:** Distinguish EOF from `EAGAIN` explicitly and transition into the `Closing` state from both readiness and IOCP paths.
|
||||
|
||||
## Medium — Socket Setup & Protocol Parsing (New, 2026-04-10)
|
||||
|
||||
### `SO_REUSEADDR` / `SO_REUSEPORT` failures silently ignored
|
||||
- **File:** `mux/ganl/src/select_network_engine.cpp:204`, `mux/ganl/src/epoll_network_engine.cpp:174`
|
||||
- **Issue:** `setsockopt` return values for SO_REUSEADDR are discarded. If the call fails (unusual, but possible on some kernels or in seccomp sandboxes), subsequent `bind()` can race with a TIME_WAIT leftover from the previous run, leaving the restart path stuck until the old endpoint times out.
|
||||
|
||||
### `IPV6_V6ONLY = 0` setsockopt not validated
|
||||
- **File:** `mux/ganl/src/select_network_engine.cpp:340`, `mux/ganl/src/epoll_network_engine.cpp:565`
|
||||
- **Issue:** Dual-stack mode is requested by setting `IPV6_V6ONLY` to 0, then `listen()` runs regardless of whether the setsockopt succeeded. On FreeBSD with `net.inet6.ip6.v6only=1` the listener silently becomes IPv6-only despite the config. Log a warning if the call fails.
|
||||
|
||||
### Accept-side FD not immediately set non-blocking
|
||||
- **File:** `mux/ganl/src/select_network_engine.cpp:875-915`
|
||||
- **Issue:** `acceptConnection()` calls `accept()` and then acquires the accept-handler lock before calling `setNonBlocking()`. Between the two a signal handler or concurrent operation could observe the fd in its default blocking state. Using `accept4(SOCK_NONBLOCK|SOCK_CLOEXEC)` on Linux closes the window in one syscall.
|
||||
|
||||
### Incomplete telnet subnegotiation stalls the state machine
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:678-720` (approximate)
|
||||
- **Issue:** If a client sends `IAC SB TTYPE IS foo` but never `IAC SE`, the handler stays in `Subnegotiation_IAC`/`Subnegotiation` indefinitely. The handshake timeout added in the previous pass only fires before negotiation completes, not mid-subnegotiation. Add a per-subnegotiation timeout (or cap the subnegotiation buffer plus deadline) so a half-open client cannot pin state.
|
||||
|
||||
### `IAC IAC` escape silently dropped when inputBuffer is full
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:639-647` (approximate)
|
||||
- **Issue:** The 8 KiB inputBuffer cap rejects any further `0xFF` byte — but the reject path also drops legitimate literal `0xFF` characters that were correctly escaped as `IAC IAC`. Client-sent binary data that happens to contain 0xFF becomes corrupted instead of triggering a protocol-error close.
|
||||
- **Fix:** Treat the buffer-full case the same as any other protocol overflow and close the connection.
|
||||
|
||||
## Low — Hardening
|
||||
|
||||
### No CA validation configured after `SSL_CTX_use_certificate_chain_file`
|
||||
- **File:** `mux/ganl/src/openssl_transport.cpp:93-113` (approximate)
|
||||
- **Issue:** The server certificate is loaded but no trust store is configured and no verify flags are set. STARTTLS peers are never validated. When `verifyPeer` is false this is silent — no log line indicates that peer validation was requested-but-bypassed. Log a warning when `verifyPeer` is requested but `SSL_CTX_set_verify()` defaults to `SSL_VERIFY_NONE`.
|
||||
|
||||
### CHARSET subnegotiation does not yet parse client's requested list
|
||||
- **File:** `mux/ganl/src/telnet_protocol_handler.cpp:1510`
|
||||
- **Issue:** On IAC SB CHARSET ... the handler currently hard-assumes UTF-8 support and never parses the client's offered list (the TODO notes "Add parsing of client's requested list if needed"). A client that offers only legacy charsets (or a preferred non-UTF8) will still be answered with UTF-8.
|
||||
- **Impact:** Low for modern clients; keeps the door open for explicit CHARSET negotiation policy later.
|
||||
|
|
@ -1,103 +0,0 @@
|
|||
# libmux (`mux/lib/`) — Open Issues
|
||||
|
||||
Created: 2026-04-10. Refreshed: 2026-05-22.
|
||||
|
||||
`mux/lib/` is the libmux shared library — stringutil, mathutil, alloc,
|
||||
hash, SHA1, UTF-8, color primitives, and the Ragel date scanner. Issues
|
||||
here affect every caller: driver, engine module, comsys, mail, and the
|
||||
standalone tests under `tests/libmux/`.
|
||||
|
||||
## Critical — Ragel Date Scanner Bugs (New, 2026-04-10)
|
||||
|
||||
The `date_scan.rl` Ragel source drives both the flexible softcode `ParseDate`
|
||||
path and the fixed-format `fun_convtime`/`fun_isodate` code. A full audit of
|
||||
the newly-unified Ragel -G2 scanner + recursive descent parser surfaced the
|
||||
following problems. Line numbers are against `date_scan.rl`; regenerated
|
||||
`date_scan.cpp` line numbers differ.
|
||||
|
||||
### 32-bit integer overflow in `digit+` token accumulator
|
||||
- **File:** `mux/lib/date_scan.rl:244-250`
|
||||
- **Issue:** `val = val * 10 + (*d - '0')` with no bound check. `val` is a
|
||||
plain `int` — 10+ digit inputs (e.g., `"9999999999-01-01"`) overflow and
|
||||
are stored as negative years, bypassing every downstream range check.
|
||||
- **Fix:** Clamp `ndig` (e.g., reject tokens with `ndig > 9`) or accumulate
|
||||
into `int64_t` and saturate before building the `DTT_NUM` token.
|
||||
|
||||
### Narrowing cast `int → short` on year without range validation
|
||||
- **File:** `mux/lib/date_scan.rl:1139`
|
||||
- **Issue:** `ft.iYear = static_cast<short>(dr.iYear)` truncates silently
|
||||
when `dr.iYear` falls outside `INT16_MIN..INT16_MAX`. Combined with the
|
||||
overflow above, parsing `"9999999999-01-01"` stores a nonsense year in
|
||||
the `FIELDEDTIME` record and then feeds it into
|
||||
`FieldedTimeToLinearTime`.
|
||||
- **Fix:** Validate `dr.iYear` against the tighter `[-27256, 30826]` range
|
||||
that `timeutil.cpp` already enforces elsewhere; reject before narrowing.
|
||||
|
||||
### Time-of-day fields not checked against zero
|
||||
- **File:** `mux/lib/date_scan.rl:625, 690` (approximate)
|
||||
- **Issue:** `iHour`, `iMinute`, `iSecond` are checked for the upper bound
|
||||
(`> 23`, `> 59`) but not for `>= 0`. A token like `"-5:30:00"` (the
|
||||
scanner does not reject the leading minus because of tokenization order)
|
||||
stores a negative hour and propagates into arithmetic that assumes
|
||||
unsigned.
|
||||
|
||||
### 12-hour "12:30 AM" is rejected
|
||||
- **File:** `mux/lib/date_scan.rl:635-643` (approximate)
|
||||
- **Issue:** For `iHour == 12` the code sets `iHour = 0` then *adds*
|
||||
`mer->iVal` (0 for AM, 12 for PM). But the upper-bound check fires before
|
||||
the AM/PM adjustment runs, so the parser rejects `"12:30:00 AM"` entirely.
|
||||
- **Fix:** Apply the meridian shift first, then validate.
|
||||
|
||||
### Month-specific day limits deferred to downstream
|
||||
- **File:** `mux/lib/date_scan.rl:842` (approximate)
|
||||
- **Issue:** The day-of-month check is a blanket `iDayOfMonth > 31`. The
|
||||
parser accepts `"Feb 31"` and `"Apr 31"` and relies on `isValidDate()` at
|
||||
~line 1211 to catch them. Fine in theory, but the delayed check means
|
||||
some callers that use the intermediate record (e.g., partial date
|
||||
completion) see an invalid state they don't expect.
|
||||
|
||||
### ISO week-date conversion runs with `iWeekOfYear == 0`
|
||||
- **File:** `mux/lib/date_scan.rl:1141-1148` (approximate)
|
||||
- **Issue:** If `parse_iso()` sets `bHasWeek` without ever assigning a
|
||||
week value, `ConvertWeekDateToLinearTime()` runs with `iWeekOfYear == 0`
|
||||
and produces a date seven days *before* the ISO-year start, silently
|
||||
wrapping into the previous year.
|
||||
- **Fix:** Require `dr.iWeekOfYear >= 1` as a precondition for the
|
||||
week-date branch.
|
||||
|
||||
### Sub-second hecto-nanosecond conversion loses precision silently
|
||||
- **File:** `mux/lib/date_scan.rl:1150-1162` (approximate)
|
||||
- **Issue:** `iFracHectoNano` (range 0..9,999,999) is split into
|
||||
millisecond / microsecond / nanosecond fields via narrowing casts into
|
||||
`unsigned short`. Values near the upper bound wrap around the field
|
||||
limits (e.g., `(frac / 10) % 1000` after narrowing). No test case
|
||||
exercises the edge.
|
||||
|
||||
### Timezone offset sign semantics are correct but fragile
|
||||
- **File:** `mux/lib/date_scan.rl:1222` (approximate)
|
||||
- **Issue:** `ltd.SetSeconds(60 * dr.iTzMinutes); lt -= ltd;` relies on the
|
||||
scanner having stored the offset with the right sign. A change to the
|
||||
tokenizer that inverts the sign (e.g., to align with `strftime`'s
|
||||
convention) would silently corrupt every parsed timestamp. Add a doc
|
||||
comment documenting the invariant, and consider switching to an explicit
|
||||
`SetSeconds(-60 * minutes)` / `lt += ltd` form so the sign is local to
|
||||
the assignment.
|
||||
|
||||
## Opportunities
|
||||
|
||||
### No dedicated date-parser fuzz/boundary tests in `tests/libmux/`
|
||||
- **File:** `tests/libmux/test_libmux.cpp`
|
||||
- **Issue:** The smoke tests at `testcases/convtime_fn.mux`,
|
||||
`testcases/parsedate_*.mux` cover happy paths, but there is no native
|
||||
harness hitting the scanner directly for overflow, narrowing, or
|
||||
negative-zero corner cases. Add a small unit-test driver under
|
||||
`tests/libmux/` that links the generated `date_scan.cpp` and runs
|
||||
adversarial inputs (large numbers, negative signs, empty strings,
|
||||
truncated ISO forms, century-rollover weeks).
|
||||
|
||||
### ZWJ emoji sequences still unimplemented (see memory)
|
||||
- **Issue:** `utf/` never finished GB11 ZWJ cluster support; the color
|
||||
width calculation and `strdistance()` treat ZWJ sequences as their
|
||||
constituent code points. Tracked in
|
||||
`memory/project_zwj_deferred.md`; kept here as a pointer so the
|
||||
libmux tracker lists it.
|
||||
|
|
@ -1,45 +0,0 @@
|
|||
# Comsys Module — Open Issues
|
||||
|
||||
Updated: 2026-03-27
|
||||
|
||||
## ~~High — Missing Error Handling in COM Interface Acquisition~~ FIXED
|
||||
|
||||
### ~~Unchecked `mux_CreateInstance()` calls in `FinalConstruct()`~~ FIXED
|
||||
|
||||
- All five core interface acquisitions now check `MUX_FAILED(mr)` and return early on failure, consistent with the `IID_ILog` pattern.
|
||||
|
||||
## Medium — Thread Safety
|
||||
|
||||
### ~~Global reference counters are not thread-safe~~ FIXED
|
||||
|
||||
- Changed `g_cComponents` and `g_cServerLocks` from `uint32_t` to `std::atomic<uint32_t>`.
|
||||
|
||||
## Medium — Data Integrity
|
||||
|
||||
### ~~Channel consistency checks unimplemented~~ FIXED
|
||||
|
||||
- `dbck()` now prunes channel users whose dbrefs are no longer valid players, reassigns channel ownership to GOD when the owner is destroyed, and removes comsys entries for destroyed players.
|
||||
|
||||
## ~~Medium — LBUF Truncation in Accessor Functions (GitHub #704)~~ FIXED
|
||||
|
||||
### ~~`cwho()` and `chanusers()` silently truncate on large channels~~ FIXED
|
||||
|
||||
- Added optional `offset`/`limit` pagination parameters to `cwho()`, `chanusers()`, and `channels()`. Softcode can now paginate through arbitrarily large result sets.
|
||||
|
||||
## High — Thread Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Non-atomic instance reference counting (`m_cRef`)~~ FIXED
|
||||
|
||||
- **File:** `comsys_mod.h:259, 276`, `comsys_mod.cpp:291-306, 3082-3097`
|
||||
- `CComsysMod::m_cRef` and `CComsysModFactory::m_cRef` are now `std::atomic<uint32_t>`. `AddRef()` uses `fetch_add` (relaxed) and `Release()` uses `fetch_sub` (acq_rel) with the previous-value check, closing the decrement/zero-check race window for double-delete.
|
||||
|
||||
## Low — Code Quality
|
||||
|
||||
### ~~`strncpy()` usage with casts~~ NOT A BUG
|
||||
|
||||
- **File:** `comsys_mod.cpp` `LoadChannels()` lambda (name/header copies)
|
||||
- Verified: `channel::name` is `UTF8[MAX_CHANNEL_LEN + 1]` and `channel::header` is `UTF8[MAX_HEADER_LEN + 1]` (`comsys_mod.h:113-114`). Each `strncpy()` copies at most `MAX_*_LEN` bytes and the next statement explicitly writes `'\0'` at index `MAX_*_LEN`, so the destination is always null-terminated even when the source exceeds the limit. The `reinterpret_cast<char *>` is only needed because `channel` uses the `UTF8` (unsigned char) alias. No fix required.
|
||||
|
||||
### ~~No parameter validation on PlayerNuke~~ FIXED
|
||||
|
||||
- Added `player < 0` guard returning `MUX_E_INVALIDARG`.
|
||||
|
|
@ -1,146 +0,0 @@
|
|||
# Core Engine (mux/modules/engine/) — Open Issues
|
||||
|
||||
Updated: 2026-05-22 (post Apple Silicon JIT)
|
||||
|
||||
## High — AST/JIT Safety & Performance
|
||||
|
||||
### ~~Stack overflow risk from AST recursion~~ FIXED
|
||||
- **File:** `mux/modules/engine/ast.cpp`
|
||||
- `ast_eval_node()` now carries an `AstEvalDepthGuard` RAII object backed by a `thread_local` counter capped at `AST_EVAL_MAX_DEPTH = 400`. Overflow sets `mudstate.bStackLimitReached` and returns, so adversarial deep ASTs (e.g., `[[[[...x]]]]` with thousands of layers) cannot blow the native C stack before the softcode limits (`func_nest_lim`, `nStackLimit`) trip on re-entry. `ast_dump()` also gained an `indent`-based cap so debug logging on pathological ASTs truncates rather than recurses unbounded.
|
||||
|
||||
### ~~O(n) lookup in `persistent_vm_t::attr_cache`~~ FIXED
|
||||
- **File:** `mux/modules/engine/jit_compiler.cpp:1056-1170`
|
||||
- The attribute cache is now a `std::unordered_map<uint64_t, attr_cache_entry>` keyed by a packed `(obj, attr_num)` word, replacing the linear `std::vector` scan on every `compile_attr()` lookup and update path. Reverified with `g++ -std=c++17 -fsyntax-only` on `jit_compiler.cpp`.
|
||||
|
||||
## Medium — Memory Management
|
||||
|
||||
### Continued use of manual `alloc_lbuf`/`free_lbuf` — MOSTLY RESOLVED
|
||||
- **File:** Multiple files in `mux/modules/engine/`
|
||||
- **Progress:** ~216 of 305 sites converted to `LBuf` RAII (`LBuf_Src` for fresh allocations, `LBuf_Adopt` for caller-owned `atr_get`/`atr_pget` returns) across 38 source files. Move semantics added to `LBuf` for adopt-by-value.
|
||||
- **Remaining:** ~90 sites that resist mechanical conversion: `fargs[]` array stores, `did_it()` charge/runout swap patterns, `unparse_object()` returns, ping-pong buffers, and cross-function lifetimes. These would need structural refactoring or a separate `LBufPtr` type.
|
||||
|
||||
## High — Buffer Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Unsafe `strcat()` in fun_rxlevel() and fun_txlevel()~~ FIXED
|
||||
|
||||
- Replaced the local `strcat()` list building with bounded `safe_str()`/`safe_chr()` writes into `levelbuff`, then copied the result out with `safe_str()`. Reverified with `g++ -std=c++17 -fsyntax-only -I mux/include -I mux/sqlite -I mux/modules/engine mux/modules/engine/functions.cpp`.
|
||||
|
||||
## Medium — SQLite Error Handling (New, 2026-04-04)
|
||||
|
||||
### ~~Missing `sqlite3_reset()` in CodeCachePut() error path~~ FIXED
|
||||
|
||||
- Added `sqlite3_reset(m_stmtCodeCachePut)` before the error return from `CodeCachePut()`. Reverified with `make -C tests/db test`.
|
||||
|
||||
### ~~No null pointer check for `sqlite3_column_blob()` results~~ FIXED
|
||||
|
||||
- `CodeCacheGet()` now rejects rows where any blob column reports `len > 0` but returns a null data pointer, resets the statement, and returns failure instead of passing invalid pointers to callers. Reverified with `make -C tests/db test`.
|
||||
|
||||
## Medium — JIT Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Unsafe global state in JIT compiler~~ FIXED
|
||||
|
||||
- **File:** `mux/modules/engine/jit_compiler.cpp:119-121`
|
||||
- All three `JITArena` static members (`s_next_id`, `s_current`, `s_arenas`) are now `thread_local`. Combined with the earlier `thread_local` conversion of `s_current_ecall_ctx`, no JIT compiler state is shared across threads. Arena IDs are per-thread (they only flow through the RV64 VM context, which is per-evaluation).
|
||||
|
||||
### ~~Unchecked `jit_alloc()` in `dbt_reset()`~~ FALSE ALARM
|
||||
|
||||
- Current `dbt_reset()` does not call `jit_alloc()`; the only `jit_alloc()` call in `dbt.cpp` is in `dbt_init()`, and that path already checks for null and returns an error.
|
||||
|
||||
## Low — Technical Debt
|
||||
|
||||
### Missing JIT support for dynamic `ulambda` args
|
||||
- **File:** `mux/modules/engine/ast.cpp:1150` (approx)
|
||||
- **Issue:** `ast_noeval_ulambda()` bypasses the JIT compiler because the JIT does not currently support dynamically-provided `cargs`.
|
||||
- **Impact:** Reduced performance for complex anonymous functions evaluated via `ulambda()`.
|
||||
|
||||
### ~~`MigrateSchema()` logs versions 8 and 9 as "upgraded" before the migration succeeds~~ FIXED
|
||||
- Removed the pre-success `fprintf()` calls from the v8/v9 branches so `RunMigration()` is the single source of success logging. Reverified with `make -C tests/db test`; schema versions 8 and 9 now log once per fresh database open.
|
||||
|
||||
## Critical — JIT Codegen Bugs (New, 2026-04-10)
|
||||
|
||||
### RV64 JAL offsets not range-checked against 21-bit signed immediate
|
||||
- **File:** `mux/modules/engine/hir_codegen.cpp:578, 1726, 1750, 1772`
|
||||
- **Issue:** Four Tier-2 call sites compute `int32_t offset = static_cast<int32_t>(target - pc);` and hand the raw value to `rv_JAL(rd, offset)`. RV64's JAL immediate is 21-bit signed (±1 MiB). The encoder silently drops the high bits; there is no check that the offset fits. Today the blob heap lives in the same JIT arena as the compiled code so offsets stay small, but if the arena grows past ~1 MiB (or the blob pool is relocated), every Tier-2 call — including the `rv64_strtod` fast path and all FP intrinsic stubs (`HIR_FCALL1`/`HIR_FCALL2`) — silently jumps to a wrong address.
|
||||
- **Fix:** Clip/assert on `offset`, or fall back to `AUIPC`+`JALR` (32-bit PC-relative) when `|offset| > 0xFFFFC`.
|
||||
|
||||
### Unchecked `iv.value` index into `result.addr[]` / `result.reg[]` / `result.spill_slot[]`
|
||||
- **File:** `mux/modules/engine/hir_codegen.cpp:696, 954, 968`
|
||||
- **Issue:** The linear-scan allocator writes `result.addr[iv.value]`, `result.reg[iv.value]`, and `result.spill_slot[iv.value]` without checking `iv.value < HIR_MAX_INSNS`. `iv.value` is a HIR instruction index and is normally bounded by the HIR builder, but any later pass that synthesizes extra virtuals (e.g., PHI rewrites, HIR_FCALL expansion) could push past `HIR_MAX_INSNS` and corrupt the adjacent allocator state. Add an explicit bounds assertion.
|
||||
|
||||
### ~~Warm-loop superblock corrupts registers when it over-commits the cache~~ FIXED (2026-06-04)
|
||||
- **File:** `mux/modules/engine/dbt_a64_sysv.cpp`, `dbt_x64_sysv.cpp`, `dbt_x64_win64.cpp` (self-loop detection / pre-load); shared helpers in `mux/include/dbt_internal.h`.
|
||||
- **Issue:** When the RV64→host translator detects a self-loop it forms a "warm-loop" superblock that keeps the loop body's registers resident across the back-edge (`warm_entry`). The register cache has only `RC_NUM_SLOTS - RC_NUM_PINNED` = 4 free slots (a0–a3 are pinned). A loop that references more than 4 non-pinned registers cannot hold a consistent mapping across `warm_entry`: a loop-invariant, never-dirty register (e.g. the ÷10 magic-reciprocal divisor in itoa) gets pre-loaded and read at the loop top with no reload, then evicted mid-body for a working register; `rc_flush` at the back-edge only saves *dirty* registers, so on later iterations `warm_entry` reads a stale host register. Surfaced as `strlen()`/`add()` etc. producing garbage for ≥4-digit values when the rv64 softlib was built with a newer GCC (15.2) whose codegen used >4 live registers in the itoa ÷10 loop. The interpreter (`dbt_interp`) ran the identical machine code correctly, proving a translator bug, not a GCC bug. The cache layout (8 slots / 4 pinned) and warm-loop codegen are identical across all three backends, so the bug was present in each.
|
||||
- **Fix:** Disable the warm-loop superblock when the loop's non-pinned referenced-register count exceeds the free slots; fall back to per-iteration dispatch (flush/reload through ctx), which is always correct. The scan, the referenced-register set, and the over-commit decision are now shared helpers (`rc_mark_used` / `rc_mark_referenced` / `rc_loop_overcommits` in `dbt_internal.h`) called identically by all three backends, so the guard can no longer be added to one backend and silently missed in the others (it originally landed only in a64). The over-commit count tracks **destinations as well as sources** — a write-only register still occupies a cache slot — while the warm-loop pre-load still keys off sources only (the registers read early). Regression test: `dbt_test.cpp::test_selfloop_register_pressure` (differential interpreter-vs-translator on a ÷10 self-loop), run against whichever backend matches the build host.
|
||||
|
||||
## Medium — JIT Memory & Offset Hygiene (New, 2026-04-10)
|
||||
|
||||
### Stale compiled entry leaked on `persistent_vm_t::attr_cache` replacement
|
||||
- **File:** `mux/modules/engine/jit_compiler.cpp:1139-1140`
|
||||
- **Issue:** When an attribute's `mod_count` changes, `compile_attr()` overwrites the cache entry in place without reclaiming the old code region. A comment already says "stale code heap space is leaked; future: reclaim". Under a write-heavy softcode workload this causes unbounded code-heap growth until the pool exhausts.
|
||||
- **Opportunity:** Maintain a free-list of abandoned code regions per arena, or bump `arena_id` on invalidation so the pool can be reused wholesale when the old generation has no live references.
|
||||
|
||||
### Branch offset backpatching assumes `code.size() * 4` fits in 21 bits
|
||||
- **File:** `mux/modules/engine/hir_codegen.cpp` (multiple backpatch sites)
|
||||
- **Issue:** All backpatch arithmetic casts `(target - source) * 4` to `int32_t` then pipes through the B-type/JAL encoders. No check that the B-type branch (12-bit signed, ±4 KiB) or JAL (21-bit signed, ±1 MiB) range is honored. A compiled attribute larger than ~4 KiB for branches or ~1 MiB for jumps would silently produce invalid instructions. Add an assertion and fall back to trampolines where needed.
|
||||
|
||||
## High — Lua Module Refcount & Concurrency (New, 2026-04-10)
|
||||
|
||||
### Non-atomic `CLuaMod::m_cRef` / factory `m_cRef`
|
||||
- **File:** `mux/modules/engine/lua_mod.h:159`, `mux/modules/engine/lua_mod.cpp:1004-1016`
|
||||
- **Issue:** `m_cRef` is a plain `uint32_t` and `AddRef`/`Release` use `m_cRef++` / `m_cRef--`. The comsys/mail modules already converted to `std::atomic<uint32_t>` (see fixed entries above) to close the decrement/zero-check race for double-delete. The Lua module was not converted.
|
||||
- **Fix:** Change `m_cRef` to `std::atomic<uint32_t>` in both `CLuaMod` and its factory, matching the comsys/mail pattern.
|
||||
|
||||
### `s_next_key++` in Lua JIT cache is not atomic
|
||||
- **File:** `mux/modules/engine/jit_lua.cpp:168` (approx)
|
||||
- **Issue:** The Lua JIT-compiled chunk cache mints keys via `uint64_t key = s_next_key++;` on a plain static. Two concurrent `CompileLuaBytecode()` calls can collide on the same key, and the cache map can lose the earlier entry — producing a dangling reference if another evaluator is still executing against it.
|
||||
- **Fix:** `std::atomic<uint64_t> s_next_key{0}; uint64_t key = s_next_key.fetch_add(1, std::memory_order_relaxed);`
|
||||
|
||||
### `LuaAlloc` can drive `m_nMemUsed` negative on realloc failure
|
||||
- **File:** `mux/modules/engine/lua_mod.cpp:556-571`
|
||||
- **Issue:** `LuaAlloc()` accounts `m_nMemUsed -= osize` on free and `m_nMemUsed += delta` on grow. On `realloc()` failure the grow path leaves `m_nMemUsed` unchanged, but the subsequent free of the *original* pointer still subtracts `osize`. If any prior grow failed, the running total drifts and eventually wraps the unsigned counter to a huge value, defeating the memory limit check.
|
||||
- **Fix:** Only update `m_nMemUsed` after a successful allocation; update the decrement path to match the actual allocator bookkeeping.
|
||||
|
||||
### `int n = snprintf(...)` cast to `size_t` without error check
|
||||
- **File:** `mux/modules/engine/lua_mod.cpp:758` (approx)
|
||||
- **Issue:** `int n = snprintf(buf, sz, ...); if ((size_t)n < nResultMax) ...` — a negative return (encoding error) casts to a huge `size_t` that passes the comparison. The same pattern appears in websocket.cpp (already reported). Guard with `n >= 0` first.
|
||||
|
||||
## Medium — SQLite Backend Error Handling (New, 2026-04-10)
|
||||
|
||||
### `GetAttribute` blob returned without null-check
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `GetAttribute()` around line 1364)
|
||||
- **Issue:** After a successful `sqlite3_step() == SQLITE_ROW`, the attribute-fetch path reads `sqlite3_column_bytes()` as `blobLen` and then `memcpy`s from `sqlite3_column_blob()` without checking that the blob pointer is non-null when `blobLen > 0`. Per the SQLite docs, `sqlite3_column_blob()` may return `NULL` even when `bytes > 0` in OOM conditions. Add a null guard and treat as "not found".
|
||||
|
||||
### `GetAllAttributes` callback receives unvalidated blob pointers
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `GetAllAttributes()` around line 1562)
|
||||
- **Issue:** Same pattern as `GetAttribute` — the row callback hands `sqlite3_column_blob(stmt, 1)` directly to the consumer without a null guard. An OOM-triggered null blob becomes a crash in the consumer.
|
||||
|
||||
### `CodeCacheFlush` reset ordering on `sqlite3_step` error
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `CodeCacheFlush()` around line 1972)
|
||||
- **Issue:** The flush path calls `sqlite3_reset()` *before* `sqlite3_step()`. If `sqlite3_step()` returns an error (`SQLITE_IOERR`, `SQLITE_FULL`, …) the statement is left with unreset error state; the next use fails spuriously. Reset on the error return path.
|
||||
|
||||
### `RunMigration` rollback silently ignored on error
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `RunMigration()` around line 356)
|
||||
- **Issue:** The schema-migration failure path calls `sqlite3_exec(db, "ROLLBACK", nullptr, nullptr, nullptr)` with a null `errmsg`. If `ROLLBACK` itself fails (busy, I/O error, …) the failure is dropped on the floor and the migration aborts with the DB in an indeterminate transaction state. Log the rollback result and escalate to `abort_dump_no_restart()` if it fails.
|
||||
|
||||
### WAL checkpoint does not retry on `SQLITE_BUSY`
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `RunCheckpoint()` around line 2116)
|
||||
- **Issue:** `sqlite3_wal_checkpoint_v2()` returns immediately on busy. `@dump` then reports success while the WAL file keeps growing because the checkpoint was never completed. Retry a few times with short backoff (the engine is single-threaded, so busy should be rare but is possible under `dbconvert -m`).
|
||||
|
||||
## Low — SQLite Defensive Hygiene (New, 2026-04-10)
|
||||
|
||||
### `sqlite3_column_text()` consumed without null-check in `LoadAllAttrNames`
|
||||
- **File:** `mux/modules/engine/sqlitedb.cpp` (approximately `LoadAllAttrNames()` around line 1656)
|
||||
- **Issue:** Attribute-name loading passes `(const char*)sqlite3_column_text(stmt, 1)` directly into callers expecting a valid C string. A NULL text column (possible after OOM or constraint violation) crashes. Skip rows with null name columns.
|
||||
|
||||
## Low — Lua Bytecode Hardening (New, 2026-04-10)
|
||||
|
||||
### `read_size()` varint loop lacks length cap
|
||||
- **File:** `mux/modules/engine/lua_bytecode.cpp:62-68` (approx)
|
||||
- **Issue:** `while ((b & 0x80) == 0)` loops over varint bytes accumulating into `size_t`. A malformed bytecode with >10 continuation bytes can overflow `size_t` before the caller's bounds check catches the resulting length. Cap iterations at `sizeof(size_t) * 8 / 7 + 1`.
|
||||
|
||||
## Low — Technical Debt
|
||||
|
||||
### 9-digit threshold for integer fast-path in HIR lowering (interpreter parity)
|
||||
- **File:** `mux/modules/engine/hir_lower.cpp:2537`
|
||||
- **Issue:** The fast integer path for ADD/SUB only kicks in for constants with <=9 digits (32-bit long legacy). Larger constants fall to double via mux_atof even if they would fit int64. The TODO notes that 64-bit int math could be used up to 18 digits in both interpreter and JIT, but would change observable results for [1e9, 1e18] values — requires coordinated change.
|
||||
- **Opportunity:** Track as future numeric parity / precision work once the current critical JIT safety items are closed.
|
||||
|
|
@ -1,62 +0,0 @@
|
|||
# Mail Module — Open Issues
|
||||
|
||||
Updated: 2026-03-27
|
||||
|
||||
## ~~High — Missing Error Handling in COM Interface Acquisition~~ FIXED
|
||||
|
||||
### ~~Unchecked `mux_CreateInstance()` calls in `FinalConstruct()`~~ FIXED
|
||||
|
||||
- All five core interface acquisitions now check `MUX_FAILED(mr)` and return early on failure, consistent with the `IID_ILog` pattern.
|
||||
|
||||
## Medium — Thread Safety
|
||||
|
||||
### ~~Global reference counters are not thread-safe~~ FIXED
|
||||
|
||||
- Changed `g_cComponents` and `g_cServerLocks` from `uint32_t` to `std::atomic<uint32_t>`.
|
||||
|
||||
## Medium — Memory Management
|
||||
|
||||
### ~~Possible malloc/free vs new/delete mismatch~~ FALSE ALARM
|
||||
|
||||
- `list` is allocated via `strdup()` (which uses `malloc`) at all call sites. `free()` is correct.
|
||||
|
||||
### ~~Partial interface cleanup on `FinalConstruct()` failure~~ FALSE ALARM
|
||||
|
||||
- Constructor initializes all interface pointers to `nullptr`. On `FinalConstruct` failure, the caller calls `Release()` which invokes the destructor. The destructor null-checks each pointer individually before releasing — partially-acquired interfaces are cleaned up correctly.
|
||||
|
||||
## ~~Medium — LBUF Truncation in Accessor Functions (GitHub #704)~~ FIXED
|
||||
|
||||
### ~~List-returning mail functions can truncate on large mailboxes~~ FIXED
|
||||
|
||||
- Added optional `offset`/`limit` pagination parameters to `maillist()`. Softcode can now paginate through arbitrarily large mailboxes.
|
||||
|
||||
## High — Thread Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Non-atomic instance reference counting (`m_cRef`)~~ FIXED
|
||||
|
||||
- **File:** `mail_mod.h:377, 396`, `mail_mod.cpp:320-335, 5583-5598`
|
||||
- `CMailMod::m_cRef` and `CMailModFactory::m_cRef` are now `std::atomic<uint32_t>`. `AddRef()` uses `fetch_add` (relaxed) and `Release()` uses `fetch_sub` (acq_rel) with the previous-value check, closing the decrement/zero-check race window for double-delete under concurrent access.
|
||||
|
||||
## Medium — Buffer Safety (New, 2026-04-04)
|
||||
|
||||
### ~~Off-by-one pointer arithmetic in `mail_to_list()` parsing~~ FIXED
|
||||
|
||||
- **File:** `mail_mod.cpp:2311, 2590, 2691`
|
||||
- All three token-parsing loops (`do_expmail_to`, `mail_to_list` senderlist build, `mail_to_list` recipient iteration) now guard the trailing `tail--; if (*tail != '"') tail++;` fixup with `if (tail > head)`. A malformed lone `"` token no longer walks `tail` before `head`, eliminating the OOB read and the out-of-bounds `*tail = '\0'` write in the recipient loop.
|
||||
|
||||
### ~~Missing strdup() null check in `do_mail_quick()`~~ FIXED
|
||||
|
||||
- **File:** `mail_mod.cpp:3145`
|
||||
- The `strdup(numlist.c_str())` result in `do_mail_quick()` is now checked; on allocation failure the player is notified via `RawNotify()` and the function returns without calling `mail_to_list()`.
|
||||
|
||||
### ~~Potential use-after-free in `shutdown()` under concurrent access~~ FIXED
|
||||
|
||||
- **File:** `mail_mod.cpp:5451-5465`
|
||||
- `CMailMod::shutdown()` now captures `m_pIStorage` into a local, nulls the member field first, and then calls `Release()` via the local. A concurrent reader (in a future multi-threaded evaluator) sees the null before the release decrement races, closing the double-Release window called out in the report.
|
||||
|
||||
## Low — Code Quality
|
||||
|
||||
### ~~HACK comments in @mail/quick object handling~~ NOT A BUG
|
||||
|
||||
- **File:** `mux/modules/engine/mail.cpp:2725, 5404` (the tracker's old `mail.cpp` path predated the engine module extraction)
|
||||
- Reviewed both HACK-tagged blocks. Neither is an actual architectural hack: one encodes a deliberate sender-attribution policy for object-sent mail (player → self, wizard-owned object → object, otherwise → owner, preventing spoofing through intermediate objects); the other gates interactive `@mail` subcommands to player executors because those subcommands depend on per-session state (composing buffer, folder selection) that does not exist for objects. The comments were relabeled from `HACK` to explanatory policy notes; no code change was required.
|
||||
|
|
@ -1,101 +0,0 @@
|
|||
# SQLSlave Module (`mux/modules/sqlslave/`) — Open Issues
|
||||
|
||||
Created: 2026-04-10. Refreshed: 2026-05-22.
|
||||
|
||||
The SQLSlave module is an optional helper module that proxies SQL
|
||||
queries to a MySQL backend. It is loaded via `MODULE_PATH` in the
|
||||
driver's `mux.conf` and exposes `CID_QueryServer`/`IID_IQueryControl`.
|
||||
|
||||
The module has not had a pass yet — tracker starts fresh today.
|
||||
|
||||
## High — Reference Counting (New, 2026-04-10)
|
||||
|
||||
### Non-atomic `CQueryServer::m_cRef` and factory `m_cRef`
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:192-207, 435-449`
|
||||
- **Issue:** `AddRef`/`Release` on both `CQueryServer` and
|
||||
`CQueryServerFactory` still use bare `uint32_t m_cRef` with
|
||||
`m_cRef++` / `m_cRef--`. The comsys and mail modules already
|
||||
converted to `std::atomic<uint32_t>` (see their ISSUES.md FIXED
|
||||
entries) to close the decrement/zero-check race for double-delete.
|
||||
- **Fix:** Change `m_cRef` to `std::atomic<uint32_t>` in both classes,
|
||||
mirroring the comsys/mail pattern with `fetch_add(relaxed)` on
|
||||
AddRef and `fetch_sub(acq_rel)` on Release.
|
||||
|
||||
### Non-atomic `g_cComponents` / `g_cServerLocks`
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:49-50`
|
||||
- **Issue:** Both module-global counters are plain `int32_t` and are
|
||||
touched from the constructor, destructor, and `LockServer()` without
|
||||
synchronization. Same issue the other modules already fixed.
|
||||
- **Fix:** Convert to `std::atomic<int32_t>`.
|
||||
|
||||
## High — Buffer & Pointer Safety (New, 2026-04-10)
|
||||
|
||||
### `ConnectionHelper()` dereferences `m_pServer` without null check
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:251`
|
||||
- **Issue:** `if ('\0' != m_pServer[0])` is the only precondition for
|
||||
touching `m_pServer`. A COM caller that passes `nullptr` for
|
||||
`pServer` (which `Connect()` stores verbatim without validation)
|
||||
crashes the module on the next query. `Connect()` also needs to
|
||||
reject `nullptr` for `pDatabase`, `pUser`, and `pPassword`, which
|
||||
`mysql_real_connect()` later dereferences through
|
||||
`reinterpret_cast<char *>`.
|
||||
|
||||
### `Connect()` takes ownership of caller buffers via `delete[]`
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:211-227, 160-167`
|
||||
- **Issue:** The destructor frees `m_pServer`, `m_pDatabase`,
|
||||
`m_pUser`, and `m_pPassword` with `delete[]`, yet the parameters to
|
||||
`Connect()` arrive as `const UTF8 *` from the caller with no
|
||||
documented ownership transfer. Across-module delivery through
|
||||
standard marshaling serializes into temporary buffers that the
|
||||
transport frees — the module is currently only safe to use in-process
|
||||
with a caller that allocates the strings via `new UTF8[]` and
|
||||
intentionally hands ownership over.
|
||||
- **Fix:** Copy the strings into module-owned `std::string`s on entry.
|
||||
That removes the hidden ABI contract and makes the out-of-process
|
||||
(proxy/stub) path work without a double-free.
|
||||
|
||||
## Medium — MySQL Error Propagation (New, 2026-04-10)
|
||||
|
||||
### `mysql_real_connect()` failure is silent
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:261-270`
|
||||
- **Issue:** On connection failure, `ConnectionHelper()` leaves
|
||||
`m_database` initialized but unconnected and returns normally. The
|
||||
next `Query()` call runs `mysql_ping()` which fails, triggering a
|
||||
reconnect attempt whose own failure is also silent. The caller has
|
||||
no way to distinguish "not yet connected" from "unreachable".
|
||||
- **Fix:** Log `mysql_error(m_database)` on failure, and surface a
|
||||
dedicated `QS_CONNECT_FAILED` result so softcode can differentiate.
|
||||
|
||||
### `mysql_next_result()` error result dropped
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:388-395`
|
||||
- **Issue:** The post-query drain loop continues only while
|
||||
`mysql_next_result() == 0`. A value `> 0` (error) aborts the loop
|
||||
silently and discards whatever error the server returned. Stored
|
||||
procedures that fail midway look successful from the MUX side.
|
||||
|
||||
### `mysql_options(MYSQL_OPT_RECONNECT)` return value unchecked
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:257, 268`
|
||||
- **Issue:** Both `mysql_options` calls ignore their return value.
|
||||
MySQL 8.0+ removed `MYSQL_OPT_RECONNECT` entirely, so the first call
|
||||
can fail silently on modern clients while the second call (after
|
||||
connect) relies on the option still being supported. Check the
|
||||
return and log, or switch to the MySQL 8.0 auto-reconnect pattern.
|
||||
|
||||
## Low — Housekeeping
|
||||
|
||||
### `ConnectionHelper()` does nothing on detected reconnect
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:326-332`
|
||||
- **Issue:** The `if (lThreadId_before != lThreadId_after)` block is
|
||||
empty — the code notices that the connection was reestablished and
|
||||
then takes no action. Either delete the dead check or restore
|
||||
whatever session-level state (`SET NAMES utf8`, per-session
|
||||
variables, prepared statements) a new MySQL session needs.
|
||||
|
||||
### `catch (...) { ; }` swallows allocator errors
|
||||
- **File:** `mux/modules/sqlslave/sqlslave.cpp:80-88, 462-469`
|
||||
- **Issue:** `new CQueryServerFactory` / `new CQueryServer` are wrapped
|
||||
in `try { ... } catch (...) { ; /* Nothing. */ }`. A `bad_alloc` is
|
||||
converted into a null pointer check on the next line, which is fine,
|
||||
but any non-`bad_alloc` exception (e.g., a constructor invariant
|
||||
violation) is silently dropped. Narrow the `catch` to `std::bad_alloc`
|
||||
to avoid masking real bugs.
|
||||
|
|
@ -1,295 +0,0 @@
|
|||
# Hydra Proxy -- Code Review Issues
|
||||
|
||||
Review date: 2026-03-27
|
||||
|
||||
## TLS Policy (decided 2026-03-27)
|
||||
|
||||
MUDs have accepted cleartext telnet connections for 30+ years. Hydra supports
|
||||
the full ecosystem, but defaults to secure settings. The cleartext code path is
|
||||
always compiled in but controlled by opt-in configuration:
|
||||
|
||||
**Front door (client to Hydra)**:
|
||||
- TLS required by default.
|
||||
- `allow_plaintext = yes` in hydra.conf to override (for SSH+TF users, etc.).
|
||||
- Hydra-specific credential commands (`create`, `/addcred`) always require TLS
|
||||
regardless of the `allow_plaintext` setting. Game traffic passthrough is
|
||||
allowed on plaintext connections.
|
||||
|
||||
**Back door (Hydra to game server)**:
|
||||
- TLS required by default per game block.
|
||||
- `tls_required = no` per game block to override (for colocated games on
|
||||
localhost/same VPC).
|
||||
- Operator must consciously acknowledge the cleartext backend risk.
|
||||
|
||||
## Critical
|
||||
|
||||
### C-1. Cleartext credentials on non-TLS connections -- FIXED
|
||||
- **Resolution**: Per TLS policy above. `allow_plaintext` config option
|
||||
(default: no) gates front-door connections. `create` and `/addcred` are
|
||||
rejected on non-TLS connections regardless of `allow_plaintext`.
|
||||
- **Files changed**: config.h, config.cpp, session_manager.cpp, hydra_main.cpp,
|
||||
hydra.conf.example
|
||||
|
||||
### C-2. `/addcred` sends game secrets in plaintext -- FIXED
|
||||
- **Resolution**: See C-1. Back-door TLS enforced via `tls_required` per game
|
||||
block (default: yes). Games on localhost set `tls_required no` explicitly.
|
||||
- **Files changed**: config.h, config.cpp, session_manager.cpp, hydra.conf.example
|
||||
|
||||
### C-3. No admin authorization on process management RPCs -- FIXED
|
||||
- **Resolution**: Added `AccountManager::isAdmin()` query. `/start`, `/stop`,
|
||||
`/restart` telnet commands and `StartGame`/`StopGame`/`RestartGame` gRPC RPCs
|
||||
now check admin flag before proceeding.
|
||||
- **Files changed**: account_manager.h, account_manager.cpp, session_manager.cpp,
|
||||
grpc_server.cpp
|
||||
|
||||
### C-4. CORS allows any origin -- FIXED
|
||||
- **Resolution**: Replaced wildcard `Access-Control-Allow-Origin: *` with
|
||||
configurable `cors_origin` in hydra.conf. Default: deny all. Multiple origins
|
||||
supported. `Vary: Origin` header added.
|
||||
- **Files changed**: config.h, config.cpp, grpc_web.h, grpc_web.cpp,
|
||||
session_manager.cpp, hydra.conf.example
|
||||
|
||||
## High
|
||||
|
||||
### H-1. Unbounded line buffer (DoS) -- FIXED
|
||||
- **Resolution**: Capped `lineBuf` at 8192 bytes (`MAX_LINE_LENGTH`).
|
||||
Connections exceeding the limit are dropped.
|
||||
- **Files changed**: session_manager.h, session_manager.cpp
|
||||
|
||||
### H-2. Unbounded HTTP request buffer (DoS) -- FIXED
|
||||
- **Resolution**: Capped `httpBuf` at 1MB. Returns HTTP 413 on overflow.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### H-3. Unbounded WebSocket fragment buffer (DoS) -- FIXED
|
||||
- **Resolution**: Added `WS_MAX_PAYLOAD` check for 16-bit extended lengths.
|
||||
Fragment reassembly capped at `WS_MAX_PAYLOAD`. Close frame 1009 sent on
|
||||
overflow.
|
||||
- **Files changed**: websocket.cpp
|
||||
|
||||
### H-4. Session tokens never expire or rotate -- FIXED
|
||||
- **Resolution**: Added `session_token_ttl` config (default 24h). Tokens now
|
||||
rotate on re-authentication. `dbPersistId` introduced to maintain scroll-back
|
||||
foreign keys across rotations.
|
||||
- **Files changed**: config.h, config.cpp, session_manager.h, session_manager.cpp
|
||||
|
||||
### H-5. No TLS on gRPC native listener -- FIXED
|
||||
- **Resolution**: Added `grpc_tls_cert` and `grpc_tls_key` configuration.
|
||||
`GrpcServer` now initializes `SslServerCredentials` if provided.
|
||||
- **Files changed**: config.h, config.cpp, grpc_server.cpp, hydra.conf.example
|
||||
|
||||
### H-6. Non-constant-time password comparison -- FIXED
|
||||
- **Resolution**: Replaced `std::string::operator!=` with `CRYPTO_memcmp` from
|
||||
OpenSSL in `AccountManager::authenticate()`.
|
||||
- **Files changed**: account_manager.cpp
|
||||
|
||||
### H-8. LBUF_SIZE discrepancy (8000 vs 32768) -- FIXED
|
||||
- **Resolution**: `telnet_bridge.cpp` now defines `LBUF_SIZE=32768` before
|
||||
including `color_ops.h` (matching `alloc.h`). All three TelnetBridge methods
|
||||
(`ingestGameOutput`, `renderForClient`, `charsetEncodeFromUtf8`) converted
|
||||
from fixed stack buffers to heap-allocated `std::vector` sized to input.
|
||||
- **Files changed**: telnet_bridge.cpp
|
||||
|
||||
### H-7. Weak key derivation on POSIX -- FIXED
|
||||
- **Resolution**: Replaced `crypt_r`-based key derivation with PBKDF2-HMAC-SHA256
|
||||
uniformly on all platforms (matching the existing Windows path).
|
||||
- **Files changed**: account_manager.cpp
|
||||
|
||||
## Medium
|
||||
|
||||
### M-1. `getpass()` is deprecated -- FIXED
|
||||
- **Resolution**: Implemented `readPassword()` in `hydra_main.cpp` using
|
||||
`tcsetattr` to disable echo on STDIN.
|
||||
- **Files changed**: hydra_main.cpp
|
||||
|
||||
### M-2. Idle/detached session timeouts not enforced -- FIXED
|
||||
- **Resolution**: Added session reaping in `runTimers()` using
|
||||
`sessionIdleTimeout` and `detachedSessionTimeout`. Expired sessions are
|
||||
flushed, their links closed, and persisted state deleted.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### M-3. `findByPersistId` is O(N) linear scan -- FIXED
|
||||
- **Resolution**: Added `persistIdIndex_` (`unordered_map<string, HydraSessionId>`)
|
||||
for O(1) session lookups.
|
||||
- **Files changed**: session_manager.h, session_manager.cpp
|
||||
|
||||
### M-4. No account creation rate limiting -- FIXED
|
||||
- **Resolution**: Added IP-based rate limiting (max 2 per hour) for account
|
||||
creation via both telnet and gRPC.
|
||||
- **Files changed**: session_manager.h, session_manager.cpp, grpc_server.cpp
|
||||
|
||||
### M-5. `ipTrackers_` map grows without bound -- FIXED
|
||||
- **Resolution**: Added periodic pruning (every 5 minutes) of `ipTrackers_`
|
||||
entries with zero connections and no active lockout.
|
||||
- **Files changed**: session_manager.h, session_manager.cpp
|
||||
|
||||
### M-6. `strerror_r` assumes GNU semantics -- FIXED
|
||||
- **Resolution**: Added `#if defined(_GNU_SOURCE)` check to handle both GNU and
|
||||
XSI variants of `strerror_r`.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### M-7. GMCP cache grows without bound -- FIXED
|
||||
- **Resolution**: Added `MAX_GMCP_CACHE_ENTRIES` (64) limit to `gmcpCache`.
|
||||
- **Files changed**: session_manager.h, session_manager.cpp
|
||||
|
||||
### M-8. Stack buffer overflow risk in telnet_bridge -- FIXED
|
||||
- **Resolution**: `HydraSession::OutputItem::render` moved from fixed stack
|
||||
buffer to heap-allocated `std::vector`. TelnetBridge methods also converted
|
||||
to heap buffers (see H-8).
|
||||
- **Files changed**: session_manager.cpp, telnet_bridge.cpp
|
||||
|
||||
### M-9. `std::stoul` without exception handling -- FIXED
|
||||
- **Resolution**: Wrapped `std::stoul` for Content-Length parsing in try/catch.
|
||||
Returns incomplete-request on parse failure.
|
||||
- **Files changed**: grpc_web.cpp
|
||||
|
||||
### M-10. IP-based rate limit bypass via pruning -- FIXED
|
||||
- **Resolution**: Pruning now removes expired `accountCreateTimes` entries
|
||||
first, then only erases the IP tracker if `accountCreateTimes` is also
|
||||
empty. Rate-limit window survives disconnection and pruning cycles.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
## Low
|
||||
|
||||
### L-1. `ListenConfig` bools uninitialized -- FIXED
|
||||
- **Resolution**: Added `{false}` default initializers.
|
||||
- **Files changed**: config.h
|
||||
|
||||
### L-2. Duplicate base64 implementations -- FIXED
|
||||
- **Resolution**: Consolidated into `base64.h`/`base64.cpp` with three
|
||||
overloads: `base64Encode(uint8_t*, size_t)`, `base64Encode(string)`,
|
||||
and `base64Decode(string)`. Removed duplicate implementations from
|
||||
`websocket.cpp` and `grpc_web.cpp`.
|
||||
- **Files added**: base64.h, base64.cpp
|
||||
- **Files changed**: websocket.cpp, grpc_web.cpp, grpc_web.h,
|
||||
session_manager.cpp, Makefile.am
|
||||
|
||||
### L-3. No log rotation -- FIXED
|
||||
- **Resolution**: Added SIGHUP signal handler to call `logReopen()`, enabling
|
||||
integration with standard `logrotate`.
|
||||
- **Files changed**: hydra_log.h, hydra_log.cpp, hydra_main.cpp
|
||||
|
||||
### L-4. Dead stub files -- FIXED
|
||||
- **Resolution**: Removed `front_door.cpp`, `front_door.h`, `back_door.cpp`, and
|
||||
`back_door.h` as they were superseded by GANL-integrated logic.
|
||||
- **Files changed**: Makefile.am, front_door.*, back_door.*
|
||||
|
||||
### L-5. Hardcoded 100ms poll interval -- FIXED
|
||||
- **Resolution**: Implemented adaptive polling in `hydra_main.cpp` (10ms when
|
||||
active, ramping to 100ms when idle).
|
||||
- **Files changed**: hydra_main.cpp
|
||||
|
||||
### L-6. gRPC insecure listener not restricted -- FIXED
|
||||
- **Resolution**: `GrpcServer::start()` rejects non-loopback bind addresses
|
||||
when TLS is not configured (must be `127.0.0.1`, `[::1]`, or `localhost`).
|
||||
- **Files changed**: grpc_server.cpp
|
||||
|
||||
## Bonus: Bug fix
|
||||
|
||||
### B-1. `handleGrpcWebRequest` uses undeclared `handle` variable -- FIXED
|
||||
- **Resolution**: All four instances of `safeWrite(handle, ...)` in
|
||||
`handleGrpcWebRequest()` changed to `safeWrite(fd.handle, ...)`. This was a
|
||||
compile error when `GRPC_ENABLED` was defined.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### B-2. `setFrontDoorTls` called before front-door entry exists -- FIXED
|
||||
- **Resolution**: Reordered event loop in `hydra_main.cpp` so `onAccept*` is
|
||||
called before `setFrontDoorTls`, ensuring the front-door entry exists in the
|
||||
map when TLS state is applied.
|
||||
- **Files changed**: hydra_main.cpp
|
||||
|
||||
## Production Deployment
|
||||
|
||||
### P-1. Systemd service file -- FIXED
|
||||
- **Resolution**: Added `hydra.service` with security hardening
|
||||
(NoNewPrivileges, ProtectSystem=strict, PrivateTmp), restart-on-failure,
|
||||
environment file for `HYDRA_MASTER_KEY`, and SIGHUP reload support.
|
||||
- **Files added**: hydra.service
|
||||
|
||||
### P-2. Health check endpoint -- FIXED
|
||||
- **Resolution**: grpc-web listeners serve `GET /healthz` returning HTTP 200
|
||||
`ok`. Telnet listeners support TCP health checks (accept = healthy).
|
||||
Documented in hydra.conf.example for ALB/NLB configuration.
|
||||
- **Files changed**: session_manager.cpp, hydra.conf.example
|
||||
|
||||
### P-3. Graceful connection drain on SIGTERM -- FIXED
|
||||
- **Resolution**: On SIGTERM, sessions are notified and flushed, gRPC server
|
||||
stops accepting new RPCs, then a 3-second drain loop flushes pending writes
|
||||
while rejecting new connections. `TimeoutStopSec=10` in systemd unit.
|
||||
- **Files changed**: hydra_main.cpp, hydra.service
|
||||
|
||||
### P-4. Build hardening flags -- FIXED
|
||||
- **Resolution**: Added `-fstack-protector-strong`, `-D_FORTIFY_SOURCE=2`,
|
||||
`-Wformat -Wformat-security` to compiler flags. Full RELRO
|
||||
(`-Wl,-z,relro,-z,now`) added to linker flags. Existing `-pie` retained.
|
||||
- **Files changed**: Makefile.am
|
||||
|
||||
### P-5. Metrics/monitoring
|
||||
- **Status**: Fixed (2026-03-29).
|
||||
- **Resolution**: Added a Prometheus-style `/metrics` endpoint on the grpc-web
|
||||
HTTP listener. It exports current sessions, front-doors by protocol, TLS
|
||||
front-doors, back-door links by state, scrollback bytes/lines, buffered
|
||||
write backlog, subscriber counts, IP rate-limit/lockout gauges, and
|
||||
monotonic counters for auth failures, reconnect attempts/failures, and
|
||||
backend disconnect/connect-failure events.
|
||||
- **Files changed**: session_manager.cpp, session_manager.h, deploy/DEPLOY.md
|
||||
|
||||
### P-6. Restart semantics are still disruptive -- FIXED (documentation)
|
||||
- **Resolution**: Restart is still disruptive by design (no zero-disconnect
|
||||
handoff), but the expectation is now surfaced in both the operator
|
||||
documentation and the systemd unit so it cannot be missed. `deploy/DEPLOY.md`
|
||||
explicitly says "Treat restart as controlled maintenance, not as a
|
||||
transparent zero-impact operation" in the systemctl restart section, and the
|
||||
certificate-renewal section repeats the warning. `hydra.service` now carries
|
||||
an inline comment next to `TimeoutStopSec=10` explaining that restart is
|
||||
*not* transparent — front-door connections drop and must reconnect — and
|
||||
pointing operators to DEPLOY.md for the full playbook. A future zero-disconnect
|
||||
upgrade would need a separate handoff design (socket passing, state
|
||||
migration, etc.) and is explicitly out of scope for this item.
|
||||
- **Files changed**: hydra.service, deploy/DEPLOY.md (documentation already
|
||||
present from an earlier pass)
|
||||
|
||||
## Deployment issues (discovered 2026-03-28)
|
||||
|
||||
### D-1. Write-path fix bypasses GANL transport layer
|
||||
- **Status**: Fixed (2026-03-28).
|
||||
- **Problem**: The first write-buffer refactor used raw `::send()` for all
|
||||
front-door traffic, which bypassed GANL transport handling for TLS listeners.
|
||||
- **Resolution**: `safeWrite()` now splits the path correctly: TLS front-doors
|
||||
enqueue plaintext into `tlsPlainOut`, run it through
|
||||
`flushFrontDoorTlsOutgoing()`, and drain ciphertext via
|
||||
`drainTlsCiphertext()`. Raw socket `::send()` remains only for non-TLS
|
||||
connections and back-door links.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### D-2. Proto field name drift: `system_notice` vs `notice`
|
||||
- **Status**: Fixed (2026-03-28).
|
||||
- **Problem**: `session_manager.cpp` called `mutable_system_notice()` but
|
||||
`hydra.proto` defines the field as `notice` (line 237:
|
||||
`SystemNotice notice = 3`). Compile error when `GRPC_ENABLED` is set.
|
||||
- **Resolution**: Changed `mutable_system_notice()` to `mutable_notice()`
|
||||
in session_manager.cpp (2 occurrences, lines 644 and 661).
|
||||
- **Note**: Cross-check all client code under `./client/` against
|
||||
`hydra.proto` for similar drift.
|
||||
- **Files changed**: session_manager.cpp
|
||||
|
||||
### D-3. NGINX stream listener conflicts with Hydra telnet port
|
||||
- **Status**: Fixed (configuration change).
|
||||
- **Problem**: DEPLOY.md configures both NGINX stream and Hydra to use
|
||||
port 4201. NGINX binds `0.0.0.0:4201` first, preventing Hydra from
|
||||
binding `127.0.0.1:4201`. Only the grpc-web listener starts.
|
||||
- **Resolution**: Hydra telnet listener moved to port 4202 in
|
||||
hydra.conf. NGINX stream upstream updated to proxy to 4202.
|
||||
DEPLOY.md and the shipped deploy configs now reflect this.
|
||||
- **Files affected**: hydra.conf, DEPLOY.md, hydra-stream.nginx.conf
|
||||
|
||||
### D-4. `GameOutput.text` receives malformed UTF-8
|
||||
- **Status**: Fixed (2026-03-28).
|
||||
- **Problem**: Hydra was serializing malformed UTF-8 into
|
||||
`hydra.GameOutput.text`, especially when UTF-8 sequences were split across
|
||||
reads or when claimed UTF-8 input was invalid.
|
||||
- **Resolution**: Added `utf8Carry` buffering for split trailing sequences in
|
||||
`TelnetBridge::ingestGameOutput()`, plus `sanitizeProtoTextForLog()` to log
|
||||
the first invalid offset/hex window and replace invalid/truncated sequences
|
||||
before populating protobuf text fields.
|
||||
- **Files changed**: telnet_bridge.cpp, session_manager.cpp, grpc_server.cpp,
|
||||
utf8_utils.h
|
||||
|
|
@ -1,296 +0,0 @@
|
|||
# MUX Core Server (mux/src/) — Open Issues
|
||||
|
||||
Updated: 2026-05-22 (Apple Silicon DBT JIT landed; Windows TODO unchanged)
|
||||
|
||||
## ~~Critical — Buffer Overflows & Memory Safety~~ FIXED
|
||||
|
||||
### ~~Buffer overflow in `load_restart_db()` memcpy calls~~ FIXED
|
||||
|
||||
- All `memcpy()` calls into fixed-size descriptor fields (`d->addr`, `d->doing`, `d->username`) and `alloc_lbuf()` buffers (`output_prefix`, `output_suffix`) now clamp to the destination size before copying.
|
||||
|
||||
### ~~Unvalidated array index from restart file~~ FIXED
|
||||
|
||||
- `num_main_game_ports` is now validated against `MAX_LISTEN_PORTS` (or `MAX_LISTEN_PORTS * 2` with SSL) before the loop. Out-of-range values close the file and abort restart gracefully.
|
||||
|
||||
### ~~Missing null check after `getstring_noalloc()`~~ NOT A BUG
|
||||
|
||||
- `getstring_noalloc()` returns a static buffer, never null. No fix needed.
|
||||
|
||||
### ~~Missing null check after `ConvertToUTF8()`~~ NOT A BUG
|
||||
|
||||
- `ConvertToUTF8()` returns a static buffer, never null. The real risk was the unbounded copy, now fixed above.
|
||||
|
||||
## Critical — Signal Handler Safety
|
||||
|
||||
### ~~Non-async-signal-safe calls in crash signal handler~~ FIXED
|
||||
|
||||
- Crash signals (SIGSEGV, SIGBUS, SIGILL, SIGFPE, SIGTRAP, etc.) now use only async-signal-safe functions: `check_panicking()` (volatile write + `signal`/`kill`), `PanicRestart()` (`fork` + `execl`), `_exit()`. Removed: `Flush()`, `log_signal()`, `drv_Report()`, `drv_PresyncDatabaseSigsegv()`, `final_stubslave()`, `final_modules()`, `raw_broadcast()`, and `GetBCanRestart()` COM call.
|
||||
- Restart decision uses cached `g_bCanRestart` (volatile sig_atomic_t) set by the GANL main loop from `GetBCanRestart()` before entering the event loop.
|
||||
- SIGABRT handler also stripped to `_exit(134)` — no logging or engine calls.
|
||||
|
||||
### ~~Non-async-signal-safe calls in normal signal handlers~~ FIXED
|
||||
|
||||
- All normal signal handlers converted to flag-based deferral. Signal handler now only sets `volatile sig_atomic_t` flags (`g_restart_flag`, `g_dump_flag`, `g_sigchld_flag`, `g_shutdown_signal`/`g_shutdown_flag`). The GANL main loop polls these flags and performs restart, dump, child reaping, and shutdown broadcast in safe context.
|
||||
- `signal_desc()` and `log_signal()` made non-static for use by the main loop.
|
||||
|
||||
### ~~`g_shutdown_flag` is not volatile~~ FIXED
|
||||
|
||||
- Changed from `bool` to `volatile sig_atomic_t` in `bsd.cpp` and `driverstate.h`. All write sites updated.
|
||||
|
||||
### ~~`g_panicking` is not volatile~~ FIXED
|
||||
|
||||
- Changed from `bool` to `volatile sig_atomic_t` in `bsd.cpp` and `driverstate.h`. All write sites updated.
|
||||
|
||||
### ~~`g_dump_child_pid` race condition~~ FIXED
|
||||
|
||||
- Changed from `volatile pid_t` to `volatile sig_atomic_t` in `bsd.cpp` and `driverstate.h`. Added explicit casts in `ganl_adapter.cpp`.
|
||||
|
||||
## High — Unchecked Return Values & Error Paths
|
||||
|
||||
### ~~`getsockname()` failure crashes server~~ FIXED
|
||||
|
||||
- Replaced `mux_assert(0)` with `mux_fclose(f); g_restarting = false; return;`
|
||||
|
||||
### ~~File not closed on invalid restart version~~ FIXED
|
||||
|
||||
- Replaced `mux_assert(0)` with `mux_fclose(f); g_restarting = false; return;`
|
||||
|
||||
### ~~`fgets()` return value unchecked~~ FIXED
|
||||
|
||||
- Now checks `fgets()` return and `strncmp()` result; closes file and returns on failure.
|
||||
|
||||
### ~~Unvalidated enum/range values from restart file~~ FIXED
|
||||
|
||||
- `d->height` and `d->width` now clamped to 1..512 (default to 24x78 on out-of-range). `d->encoding` clamped to 0..255 (default to `g_dc.default_charset`). Applied in both version 2 and version 3+ paths.
|
||||
|
||||
## Medium — Code Quality
|
||||
|
||||
### ~~`ISOUTOFMEMORY` macro terminates instead of recovering~~ FIXED
|
||||
|
||||
- Removed `ISOUTOFMEMORY` macro entirely from `config.h`. All 23 call sites replaced with site-appropriate handling: truly unrecoverable allocations (buffer pools, db array, anum table, markbuf) use `mux_assert` or `OutOfMemory`; recoverable sites (queue entries, commands, mail, guests, vattrs, config, restart ttype, forward lists) log the failure and return gracefully.
|
||||
|
||||
### ~~Inconsistent error handling patterns~~ MOSTLY RESOLVED
|
||||
|
||||
- Remaining `mux_assert(0)` sites in `load_restart_db()` were previously fixed. OOM handling now consistently uses per-site recovery.
|
||||
|
||||
### ~~Integer overflow potential in buffer calculations~~ FALSE ALARM
|
||||
|
||||
- `getstring_noalloc()` reads into a static `buf[2*LBUF_SIZE + 20]` (~65KB). The returned `nBuffer` is always bounded by this buffer size, so `nBuffer+1` cannot overflow `size_t`.
|
||||
|
||||
### ~~Volatile counters in slave.cpp lack atomicity~~ PARTIALLY FIXED
|
||||
|
||||
- Changed from `volatile int` to `volatile sig_atomic_t`. The `++` in a signal handler is still technically non-atomic, but `sig_atomic_t` is the POSIX-sanctioned type for this pattern.
|
||||
|
||||
## Low — Dead Code & Technical Debt
|
||||
|
||||
### TODO: Windows console signal handler
|
||||
|
||||
- **File:** `modules.cpp:1255`
|
||||
- **Issue:** `// TODO: SetConsoleCtrlHandler that maps events to PlatformSignal.`
|
||||
|
||||
### ~~Commented-out debug logging~~ FIXED
|
||||
|
||||
- **File:** `ganl_adapter.cpp`
|
||||
- Deleted eight stale `//GANL_CONN_DEBUG(...)` lines (close-notify, auth-success, event-receive warning, send-on-unmapped, close-on-unmapped, handle/DESC mapping/unmapping, and unknown-DESC remove). Three of them were the entire body of an `else` branch, so the empty `else { }` blocks were collapsed along with the comments. Verified with `g++ -std=c++17 -fsyntax-only` on `ganl_adapter.cpp`.
|
||||
|
||||
### ~~Telnet state array magic number~~ FIXED
|
||||
|
||||
- Added `static constexpr int NVT_TABLE_SIZE = 256` to `descriptor_data` in `interface.h`. All loop bounds in `net.cpp` now use `DESC::NVT_TABLE_SIZE` instead of bare `256`.
|
||||
|
||||
## Critical — Buffer Overflows (New, 2026-04-04)
|
||||
|
||||
### ~~Buffer overflow in telnet USER environment variable~~ FIXED
|
||||
|
||||
- **File:** `mux/src/telnet.cpp:1221`
|
||||
- The NEW-ENVIRON `USER` path now clamps `nVarval` to `sizeof(d->username) - 1` and re-terminates the temporary buffer before copying into `d->username[11]`.
|
||||
|
||||
### ~~Unbounded `set_doing_all()` and `set_doing_least_idle()`~~ FIXED
|
||||
|
||||
- **File:** `mux/src/net.cpp:1080, 1093`
|
||||
- Both functions now clamp `len` to `SIZEOF_DOING_STRING - 1` before copying into descriptor `doing[]` buffers.
|
||||
|
||||
## High — Null Pointer Dereferences (New, 2026-04-04)
|
||||
|
||||
### ~~Missing null check after `alloc_lbuf()` for output_prefix/output_suffix~~ FIXED
|
||||
|
||||
- **File:** `mux/src/net.cpp:3381-3383, 3400-3402`
|
||||
- Restart loading now aborts gracefully with `mux_fclose(f); g_restarting = false; return;` if either `alloc_lbuf("set_userstring")` call fails before copying the restored prefix/suffix.
|
||||
|
||||
### ~~Missing null check after `alloc_lbuf()` for raw_input_buf~~ FIXED
|
||||
|
||||
- **File:** `mux/src/telnet.cpp:606-607`
|
||||
- `process_input_helper()` now returns early if `alloc_lbuf("process_input.raw")` fails, avoiding a null dereference on `d->raw_input_buf`.
|
||||
|
||||
### ~~Missing null check after `MEMALLOC()` for ttype~~ FIXED
|
||||
|
||||
- **File:** `mux/src/telnet.cpp:1095-1096, 1487-1488`
|
||||
- The TTYPE and GMCP handlers now skip the `memcpy()` when `MEMALLOC()` fails, leaving `d->ttype` null instead of dereferencing it.
|
||||
|
||||
## High — Buffer Safety & Static Buffer Risks
|
||||
|
||||
### ~~Potential buffer overflow in `encode_iac()`~~ FIXED
|
||||
|
||||
- **File:** `mux/src/net.cpp:198`
|
||||
- `encode_iac()` now builds a dynamically sized `std::string` and `queue_string()` writes it with `queue_write_LEN()`, so telnet IAC doubling no longer depends on a fixed `2*LBUF_SIZE` scratch buffer.
|
||||
|
||||
### ~~Widespread use of `static` buffers in functions~~ FIXED
|
||||
|
||||
- **Files:** `mux/src/` (5 sites in `net.cpp`, `signals.cpp`, `stubslave.cpp`) and `mux/modules/` (38 sites across 18 files in engine + mail modules).
|
||||
- All 43 `static` scratch-buffer arrays (`UTF8 buf[...]`, `char buf[...]`, `uint8_t arg[...]`) converted to `thread_local`. This is a one-word, zero-allocation, zero-behavior-change swap under the current single-threaded evaluator: `thread_local` storage has the same lifetime and performance as `static`, but each thread gets its own copy, so these functions become safe for future multi-threaded evaluation without any locking. Read-only constant tables (`aRadix64`, `aRadixPenn36`, `aRadixPenn64`, `Empty`) were intentionally left as `static` since they are immutable shared data. All 21 modified `.cpp` files verified with `g++ -std=c++17 -fsyntax-only`.
|
||||
|
||||
## Critical — Protocol Logic & Data Integrity (New, 2026-04-10)
|
||||
|
||||
### ~~Data loss in `Stub_PipePump` on short writes or errors~~ FIXED
|
||||
- **File:** `mux/src/stubslave.cpp:32-100`
|
||||
- `Stub_PipePump` now drains each dequeued block with a retry loop around
|
||||
`write(1, ...)`, correctly handling short writes and `EAGAIN`/`EWOULDBLOCK`
|
||||
instead of dropping bytes after `Pipe_GetBytes()` removes them from
|
||||
`Queue_Out`.
|
||||
|
||||
## High — Buffer Overflows & Memory Safety (New, 2026-04-10)
|
||||
|
||||
### ~~1-2 byte buffer overflow in `slave.cpp` query processing~~ FIXED
|
||||
- **File:** `mux/src/slave.cpp:115-120`
|
||||
- `buf` is now sized `MAX_STRING * 2 + 3`, matching the worst-case
|
||||
write of `ip (≤999) + ' ' + pHName (≤999) + '\n' + '\0'` =
|
||||
2001 bytes. Previously sized at `MAX_STRING * 2` = 2000 bytes,
|
||||
a hostname-plus-IP pair of the maximum length overran by one
|
||||
byte on the helper process stack.
|
||||
|
||||
## High — Protocol Safety & Limits (New, 2026-04-10)
|
||||
|
||||
### ~~`SBUF_SIZE` (64) is too small for modern telnet sequences (TTYPE, GMCP)~~ FIXED
|
||||
- **File:** `mux/include/interface.h`, `mux/src/telnet.cpp`
|
||||
- The telnet subnegotiation accumulator is now sized by a dedicated
|
||||
`TELNET_OPTION_SIZE` constant (4096) in `interface.h`, decoupled
|
||||
from `SBUF_SIZE`. Previously `d->aOption[SBUF_SIZE]` (64 bytes)
|
||||
silently truncated any GMCP JSON, MSDP, long TTYPE chain, or
|
||||
CHARSET list exceeding that limit, breaking protocol features
|
||||
for feature-rich clients. Bumping the pool-allocator `SBUF_SIZE`
|
||||
directly would have affected dozens of unrelated sites; only the
|
||||
descriptor accumulator needed to grow.
|
||||
|
||||
## ~~High — Network Address Parsing~~ (2026-04-10)
|
||||
|
||||
### ~~Undefined Behavior in IPv4 decoding~~ FIXED
|
||||
- **File:** `mux/src/netaddr.cpp:67`
|
||||
- `DecodeN` now promotes `*pu32` to `uint64_t` before shifting by
|
||||
`decode_IPv4_table[nType].nShift`. For `nType=3` (single-element IPv4
|
||||
like `12345678`), `nShift` is 32 and shifting a 32-bit value by its
|
||||
own width was undefined behavior. Shifting a `uint64_t` by 32 is
|
||||
well-defined; the result is masked back to 32 bits.
|
||||
|
||||
### ~~Broken overflow check in decimal IPv4 parsing~~ FIXED
|
||||
- **File:** `mux/src/netaddr.cpp:189`
|
||||
- Decimal accumulation now runs in `uint64_t` and is bounded against
|
||||
`0xFFFFFFFFUL` after each digit. The previous 32-bit
|
||||
`ul = (ul * 10) & 0xFFFFFFFFUL` followed by `if (ul < ul2)` could
|
||||
coincidentally produce a post-wrap value larger than the pre-wrap
|
||||
value (e.g., `500,000,000 * 10` wraps to `705,032,704`, which is
|
||||
greater than 500,000,000), silently accepting decimal components
|
||||
above 2^32 - 1.
|
||||
|
||||
## ~~Critical — Completely Broken Hex IPv4 Parsing~~ FIXED (2026-04-10)
|
||||
|
||||
### ~~`DecodeN` subtracts wrong offset from hex digits~~ FIXED
|
||||
- **File:** `mux/src/netaddr.cpp:109-116`
|
||||
- The hexadecimal branch now decodes `A-F`/`a-f` as `ch - 'A' + 10` and
|
||||
`ch - 'a' + 10`. Previously the `+ 10` was missing, so `A`..`F` mapped to
|
||||
nibbles 0..5 and every hex IPv4 literal in `@site`/`@admit`/`@nosite`
|
||||
rules silently matched the wrong address range.
|
||||
|
||||
## High — WebSocket Protocol Handling (New, 2026-04-10)
|
||||
|
||||
### ~~Truncating `static_cast<size_t>` on snprintf return~~ FIXED
|
||||
- **File:** `mux/src/websocket.cpp:270-277`
|
||||
- The 101 Switching Protocols send site now validates `snprintf`
|
||||
returned `n >= 0 && n < sizeof(response)` before casting to
|
||||
`size_t`. On any anomaly (encoding error or truncation) the
|
||||
function sends a `500 Internal Server Error` rejection and
|
||||
returns, matching the existing handshake-failure pattern. This
|
||||
can't happen in practice given the fixed format and 28-character
|
||||
accept key, but the previous code would have read past the stack
|
||||
buffer on a negative `snprintf` return.
|
||||
|
||||
### ~~64-bit WebSocket frame length silently truncates large payloads~~ FIXED
|
||||
- **File:** `mux/src/websocket.cpp:342-351`
|
||||
- `ws_queue_frame` now writes all 8 bytes of the RFC 6455 §5.2
|
||||
extended length, promoting `len` to `uint64_t` so the high-word
|
||||
shifts are well-defined regardless of `size_t` width. Previously
|
||||
bytes 2-5 were hard-coded to zero, silently dropping the top 32
|
||||
bits of any payload exceeding 4 GiB on 64-bit builds.
|
||||
|
||||
### ~~Control frames not validated against RFC 6455 §5.5~~ FIXED
|
||||
- **File:** `mux/src/websocket.cpp:432-456`
|
||||
- `ws_process_input` now rejects control frames (opcodes 0x8-0xF)
|
||||
with `FIN=0` or `lenByte >= 126` in `WS_PARSE_HEADER1`, before
|
||||
entering any extended-length or payload state. A fragmented PING
|
||||
or an oversized CLOSE used to pass through unchecked; a 2 MiB
|
||||
CLOSE payload was echoed back verbatim via `ws_queue_frame`,
|
||||
amplifying the attacker's bandwidth on the server side.
|
||||
|
||||
### ~~Fragment assembly buffer is unbounded~~ FIXED
|
||||
- **File:** `mux/src/websocket.cpp` (text/binary final-fragment and
|
||||
continuation cases)
|
||||
- Every `frag_buf.append(frame_buf)` site now checks that the
|
||||
assembled size will stay within `WS_MAX_PAYLOAD`. On violation
|
||||
the fragmentation state is cleared and a close frame with code
|
||||
`1009` (`WS_CLOSE_MESSAGE_TOO_BIG`, newly added in `websocket.h`)
|
||||
is sent. Previously a client could push thousands of small
|
||||
non-FIN fragments and grow `frag_buf` without bound — a memory-
|
||||
exhaustion DoS from a single connection.
|
||||
|
||||
### ~~Dead `op` local after continuation lookup~~ FIXED
|
||||
- **File:** `mux/src/websocket.cpp` dispatch site
|
||||
- The dead `op = ws->frame_opcode; if (op == CONTINUATION) op = frag_opcode;`
|
||||
block was removed. The switch already dispatches on `ws->frame_opcode`
|
||||
directly; the CONTINUATION case appends to `frag_buf` and flushes
|
||||
on FIN, and `save_command` does not distinguish text from binary,
|
||||
so no frag-opcode substitution is needed. Pure dead-code cleanup.
|
||||
|
||||
## High — Platform Abstraction Issues (New, 2026-04-10)
|
||||
|
||||
### ~~`PanicRestart` reads undefined `argv[]` slots~~ FIXED
|
||||
- **File:** `mux/src/platform.cpp:313-370`
|
||||
- `PanicRestart` now materializes a NULL-terminated local argv
|
||||
bounded by the declared `argc` and invokes `execv`. Previously
|
||||
`UNUSED_PARAMETER(argc)` plus seven hand-coded `execl` slots read
|
||||
`argv[0..6]` unconditionally — uninitialized/OOB memory whenever
|
||||
a caller passed fewer than seven args, and silently dropped
|
||||
anything past index 6. `argc` is bounded at 16 slots, the
|
||||
per-arg read runs before any signal-unsafe work (validating
|
||||
`execPath`/`argv` and per-slot non-null), and `execv` is on the
|
||||
POSIX.1-2008 async-signal-safe list so the crash-recovery context
|
||||
is preserved. `std::vector` is deliberately avoided here because
|
||||
heap allocation is not async-signal-safe.
|
||||
|
||||
### ~~`MaximizeFileDescriptors` ignores `setrlimit` failure~~ FIXED
|
||||
- **File:** `mux/src/platform.cpp:292-330`
|
||||
- After the setrlimit call, the function now re-reads the actual
|
||||
kernel-enforced `rlim_cur` via `getrlimit` and reports that to
|
||||
the caller. Previously the post-set `rlp.rlim_cur` reflected the
|
||||
*desired* value regardless of whether the raise succeeded, so
|
||||
`select()`/`poll()`/`epoll` sizing could outrun the real ceiling.
|
||||
Also added `INT_MAX` saturation because `rlim_t` (often 64-bit)
|
||||
can exceed the `int` return type on modern systems with an
|
||||
unlimited nofile limit.
|
||||
|
||||
### ~~`BootHelperProcess` close-all loop is O(rlim) on modern systems~~ FIXED
|
||||
- **Files:** `mux/src/platform.cpp:202-234`, `mux/src/ganl_adapter.cpp` (`boot_stubslave`)
|
||||
- The post-fork close-all step in `BootHelperProcess` now uses
|
||||
`close_range(3, ~0U, 0)` on glibc ≥ 2.34 (Linux 5.9+ wrapped) or
|
||||
`closefrom(3)` on BSDs and Solaris — each a single syscall
|
||||
instead of O(rlim) `mux_close()` calls. The linear loop is kept
|
||||
as a fallback for systems without either API. `maxfds`
|
||||
computation was pushed into the fallback branch so the fast
|
||||
paths don't carry a dead store. The duplicate launcher logic in
|
||||
`ganl_adapter.cpp::boot_stubslave` was moved behind a local
|
||||
`close_fds_from()` helper that dispatches through `SYS_close_range`
|
||||
with the same linear fallback, so both call sites get the fast path.
|
||||
|
||||
### ~~Non-atomic `CPlatform::m_cRef` / `CPlatformFactory::m_cRef`~~ FIXED
|
||||
- **File:** `mux/src/platform.cpp:80-94, 412-427`
|
||||
- `CPlatform` and `CPlatformFactory` now use atomic increment/decrement
|
||||
operations in `AddRef()`/`Release()`, removing the unsynchronized
|
||||
`m_cRef++` / `m_cRef--` pattern from the platform abstraction layer while
|
||||
keeping the factory macro and ABI surface unchanged.
|
||||
|
|
@ -1,25 +0,0 @@
|
|||
# Parser Research Tools — Open Issues
|
||||
|
||||
Updated: 2026-03-27
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~Percent-escape handling still fails two documented MUX 2.13 cases~~ FIXED
|
||||
|
||||
- `noevalPass()` now mirrors the evaluator’s existing `NODE_ESCAPE` + `NODE_SUBST` sequence handling, so deferred FN_NOEVAL bodies in `switch()` and `iter()` collapse `\\%b` to `%b` before the second evaluation pass. `./test_eval.sh` now passes the documented `mux213` brace-body cases.
|
||||
|
||||
## Opportunities
|
||||
|
||||
### ~~`test_eval.sh` fails noisily when `./eval` is missing~~ FIXED
|
||||
|
||||
- `test_eval.sh` now enables `set -euo pipefail`, builds `./eval` on demand with `make eval`, and exits once with an actionable error if the build fails instead of emitting one identical failure per test case.
|
||||
|
||||
### ~~`make` is not warning-clean under `-Wall -Wextra`~~ FIXED
|
||||
|
||||
- Changed `token_name()` from `static` to `inline` in `mux_parse.h` — eliminates unused-function warning in translation units that don't call it.
|
||||
|
||||
### ~~Escape oracle corpus requires manual curation~~ FIXED
|
||||
|
||||
- **File:** `escape_oracle_cases.txt`, `validate_live_oracle.sh`
|
||||
- `parser/validate_live_oracle.sh` is a new automated check that runs every corpus case against the real `engine.so` (via `mux/game/bin/muxscript -e 'think <expr>'`), extracts the value, and compares it byte-for-byte against the `mux214` column. It reports `PASS`/`DRIFT` per row, flags rows where `live_status != confirmed`, and exits non-zero on any drift. An optional third argument accepts a built mux2.13_N `muxscript` binary so the same run can compare against the historical reference engine when one is available.
|
||||
- The initial run flagged five drifted rows (`plain_single_unknown`, `plain_double_unknown`, `plain_single_known`, `plain_double_known`, `plain_triple_known`). Reviewing `mux2.13_13/src/eval.cpp:2439-2460` (the backslash handler) and `mux2.13_13/src/eval.cpp:1680-1699` (the percent handler under `EV_EVAL` off) showed that the live mux2.14 engine agrees with the 2.13 ground truth on all five — the corpus columns were wrong, not the engines. Corpus updated to match; `./eval`'s theoretical model is intentionally left alone and `test_eval.sh` still exercises that model exactly as before. The validator now passes 9/9 and becomes the authoritative cross-check whenever someone touches either side.
|
||||
|
|
@ -1,160 +0,0 @@
|
|||
# Test Infrastructure — Open Issues
|
||||
|
||||
Updated: 2026-05-22 (Apple Silicon JIT now enabled; re-verify divergence on Mac+JIT)
|
||||
|
||||
## Open — JIT vs no-JIT eval path divergence on `{}` args
|
||||
|
||||
### `isjson({"a":1})` returns 1 with `--enable-jit`, 0 without — both platforms
|
||||
|
||||
**Cross-platform investigation resolved 2026-04-26.** Not a Mac-specific
|
||||
bug; the divergence is between the JIT and non-JIT eval paths. macOS
|
||||
reproduces what x86-64 Linux also does without `--enable-jit`.
|
||||
|
||||
- **Probe results** (instrumentation just before `JsonValidator jv;` in
|
||||
`mux/modules/engine/funcweb.cpp::fun_isjson`, logging `fargs[0]`):
|
||||
|
||||
| Build | Smoke | `fargs[0]` for `isjson({"a":1})` |
|
||||
| --- | --- | --- |
|
||||
| Linux x86-64, `--enable-jit` | 925/925 pass | `<{"a":1}>` (braces preserved) |
|
||||
| Linux x86-64, no JIT | TC001/TC003 fail | `<"a":1>` (braces stripped) |
|
||||
| macOS arm64, `--enable-jit` (post 2a1776e27) | *expected pass* | `<{"a":1}>` (should match Linux JIT) |
|
||||
| macOS arm64, no JIT | TC001/TC003 fail | `<"a":1>` (braces stripped) |
|
||||
|
||||
Linux (no JIT) and macOS (no JIT) produce identical probe output, ruling
|
||||
out host-specific causes. With Apple Silicon JIT now enabled, macOS +JIT
|
||||
is expected to match the Linux +JIT result (braces preserved). Hypothesis 1
|
||||
stands: the divergence is purely JIT/AST path vs. legacy `parse_to`
|
||||
`EV_STRIP_CURLY` handling.
|
||||
|
||||
- **Failing assertions** (both no-JIT builds):
|
||||
- `TC001: isjson valid. Failed (obj=0 array=0 str=1 num=1 true=1 false=1 null=1).`
|
||||
- `TC003: isjson type check. Failed (obj=0 obj_as_array=0 array=0 str=1 num=1 num_as_str=0 true=1 false=1 null=1).`
|
||||
|
||||
The only failing q-values are the `{"a":1}` cases. `[1,2,3]` is
|
||||
evaluated rather than passed literally (deliberate, see below).
|
||||
|
||||
- **Site of divergence:** `EV_STRIP_CURLY` handling. The JIT-driven path
|
||||
(`AST_BRACEGROUP` in `mux/modules/engine/ast.cpp:2336`) keeps the
|
||||
braces when the entire arg is `{…}`; the legacy `parse_to` in
|
||||
`mux/modules/engine/eval.cpp` strips them. Both are platform-agnostic
|
||||
C++; the divergence is purely which path runs, not which host runs it.
|
||||
|
||||
- **PR #705 portability changes:** confirmed clean on x86-64 Linux
|
||||
with `--enable-jit` (925/925 smoke pass). The new configure
|
||||
substitutions (`LIBMUX_SONAME_FLAG`, `ENGINE_SONAME_FLAG`,
|
||||
`LD_NOUNDEFINED`, `LD_RPATH_ORIGIN`, `LD_HARDENING`) produce
|
||||
byte-identical link lines to the previous hardcoded GNU-ld flags;
|
||||
`readelf -d` on `libmux.so`, `engine.so`, and `netmux` shows the same
|
||||
SONAME and `RUNPATH=$ORIGIN:…` as before the PR.
|
||||
|
||||
- **Open question — which side is the bug?** Two candidates:
|
||||
1. **Non-JIT path is wrong.** A bare `{"a":1}` passed to `isjson` is a
|
||||
single brace-grouped arg; stripping the braces before the function
|
||||
sees them changes the semantic content (`{"a":1}` → `"a":1`).
|
||||
Other functions that take JSON-like args may be silently broken
|
||||
under no-JIT in the same way. Fix: make `parse_to`'s
|
||||
`EV_STRIP_CURLY` not strip when the brace-group is the entire arg
|
||||
value, matching `AST_BRACEGROUP`'s behavior.
|
||||
2. **Test was JIT-coupled.** The smoke expectation was written against
|
||||
the JIT path's behavior without realising the no-JIT path differed.
|
||||
Fix: rewrite the test to avoid bare `{}` in `isjson(...)` calls.
|
||||
|
||||
(1) is the better target — JIT becoming mandatory in 1-2 years means
|
||||
the non-JIT path is a deprecation path, but until then it should match
|
||||
the JIT path's semantics, not silently miscompile JSON-shaped args.
|
||||
|
||||
- **Cross-link:** macOS arm64 build PR is
|
||||
https://github.com/brazilofmux/tinymux/pull/705 — landed at
|
||||
`bad833cbf` on master 2026-04-26. As of commit `2a1776e27` (2026-05-02,
|
||||
"Enable DBT JIT on Apple Silicon"), `--enable-jit` now works on
|
||||
darwin+aarch64; the old configure error and "Mac can't JIT" note are
|
||||
obsolete. Re-run the `isjson` probe + full smoke on a Mac+JIT build to
|
||||
confirm the JIT path (`<{"a":1}>`) now passes there too (it should,
|
||||
per Linux x86-64 +JIT results). The core open question (which eval path
|
||||
is correct for bare brace-group args) remains.
|
||||
|
||||
## Testing Levels
|
||||
|
||||
The project now has two complementary testing tiers:
|
||||
|
||||
- **`testcases/`** — Smoke tests: softcode-level integration tests that run inside a live MUX instance. 811 test cases across ~240 `.mux` files, exercising functions and commands end-to-end.
|
||||
- **`tests/`** — Standalone unit test harnesses that link directly against built `.so` files and run without a MUX instance:
|
||||
- `tests/libmux/` — stringutil, mathutil, alloc (29 tests, links libmux.so)
|
||||
- `tests/color_ops/` — Ragel DFA color primitives (C, standalone)
|
||||
- `tests/db/` — SQLite storage backend (C++, standalone)
|
||||
|
||||
## High — Test Brittleness
|
||||
|
||||
### 84% of smoke tests use SHA1 hash comparison with no semantic assertions
|
||||
|
||||
- **Scope:** ~217 of ~240 test files, ~590 SHA1 references
|
||||
- **Issue:** Tests compute `sha1(output)` and compare against a hardcoded hex hash. When a test fails, the hash mismatch gives zero clue about what actually changed. Adding or modifying any function behavior requires recalculating all dependent hashes.
|
||||
- **Risk:** Function behavior could drift without visibility if a hash is copied incorrectly.
|
||||
- **Opportunity:** Gradually introduce semantic assertions (`strmatch`, value comparisons) alongside or replacing hashes for the highest-risk functions.
|
||||
- Recent progress: `cmogrifier_fn.mux`, `clone_fn.mux`, `isjson_fn.mux`, `moon_fn.mux`, `wrapcolumns_fn.mux`, `printf_fn.mux`, and all of `paginate_fn.mux` now use direct semantic checks instead of SHA1 snapshots, but the broader corpus is still overwhelmingly hash-based.
|
||||
|
||||
## High — Coverage Gaps
|
||||
|
||||
### ~~Only 1 command-level test file~~ FIXED
|
||||
|
||||
- Now 7 command-level test files (1,101 lines total):
|
||||
`dolist_cmd.mux`, `if_cmd.mux`, `switch_cmd.mux`, `trigger_cmd.mux`,
|
||||
`wait_cmd.mux`, `assert_cmd.mux` (`@assert`/`@break`), `cmd_say.mux`.
|
||||
|
||||
### Minimal edge case testing across the board
|
||||
|
||||
- **Missing categories:**
|
||||
- Empty/null inputs
|
||||
- Maximum argument counts and string length limits
|
||||
- Numeric overflow and underflow
|
||||
- Nested depth limits
|
||||
- Invalid argument types and malformed syntax
|
||||
- Permission failure paths
|
||||
- Unicode: multi-byte characters, combining diacriticals, RTL text (only `accent_fn.mux` and `strdistance_fn.mux` touch this)
|
||||
- **Note:** The `tests/` standalone harnesses are the right place for low-level edge case and boundary testing (overflow, LBUF limits, UTF-8 multi-byte). Smoke tests (`testcases/`) are better suited for end-to-end semantic correctness.
|
||||
|
||||
### Single test case per function is the norm
|
||||
|
||||
- **Example:** `abs_fn.mux` has 1 test case (lines 11-34). Most function test files follow this pattern.
|
||||
- **Impact:** Happy-path coverage only; boundary and error cases untested.
|
||||
- Recent progress: `abs_fn.mux` and `sign_fn.mux` now cover decimals plus empty/malformed-input coercion, which reduces this gap for the basic numeric smoke tier, but the broader corpus still mostly has one-case files.
|
||||
- Additional progress: `between_fn.mux`, `bound_fn.mux`, `min_fn.mux`, and
|
||||
`max_fn.mux` now include decimal, negative-range, duplicate, and coercion
|
||||
cases instead of stopping at a single basic-path assertion block.
|
||||
|
||||
## Medium — Infrastructure
|
||||
|
||||
### ~~No automatic test discovery~~ FIXED
|
||||
|
||||
- `testcases/tools/generate_smoke_suite.py` now discovers top-level `testcases/*.mux` files automatically (excluding harness/setup files), emits generated `&suite.list.1` / `&suite.list.2` overrides, and `testcases/tools/Makesmoke` appends that generated suite file after the checked-in `.mux` corpus before unformatting. Adding a new smoke test no longer requires hand-editing `smoke.mux`. Reverified with `python3 testcases/tools/generate_smoke_suite.py` and `bash -n testcases/tools/Makesmoke`.
|
||||
|
||||
### ~~Tests cannot run in isolation or parallel~~ FIXED
|
||||
|
||||
- `testcases/tools/Makesmoke` and `testcases/tools/Smoke` now accept explicit workspaces and flatfile paths, so smoke runs no longer have to share the repo-root runtime directories.
|
||||
- `testcases/tools/SmokeParallel` fans selected tests out across isolated temp workspaces (`./tools/SmokeParallel -j 4 abs_fn route_fn ...`), which closes the remaining parallel multi-test execution and isolation gap.
|
||||
|
||||
### ~~No cleanup of orphaned test objects~~ FIXED
|
||||
|
||||
- `testcases/tools/generate_smoke_suite.py` now emits per-test `&suite.cleanup.<name>` hooks that destroy named helper fixtures, stored dbref fixtures, and transient channels both before and after each smoke test. `testcases/smoke.mux` runs those hooks around every testcase, so interrupted reruns no longer inherit stale route rooms/exits, search fixtures, temporary channels, or similar leftovers from prior attempts.
|
||||
- Remaining runtime-only leaks are handled at the testcase level where needed; `clone_fn.mux` now destroys the cloned thing it creates during assertion.
|
||||
|
||||
## Medium — Script Reliability (New, 2026-04-04)
|
||||
|
||||
### ~~`mktemp` trap refresh invalidates cleanup in Makesmoke~~ FIXED
|
||||
|
||||
- `Makesmoke` now tracks temp files in `TMP_FILES[]` with a `cleanup_temps()` trap, so both the initial unformat temp file and the later scrub temp file are removed on exit.
|
||||
|
||||
### ~~Insufficient error context on dbconvert failure~~ FIXED
|
||||
|
||||
- `Smoke` now prints the exact `dbconvert` command line and exit code before dumping `netmux.log`.
|
||||
|
||||
### ~~Optional omega validation silently skipped~~ FIXED
|
||||
|
||||
- `Makesmoke` now emits an explicit warning when `$REPO_ROOT/mux/convert/omega` is unavailable, so validation skips are visible in CI and local runs.
|
||||
|
||||
## Low — Hardcoded References
|
||||
|
||||
### ~~Some tests reference `#0` directly~~ FIXED
|
||||
|
||||
- **File:** `objid_fn.mux`
|
||||
- `objid_fn.mux` still uses `#0` (the TinyMUX master room, which is a database invariant), but no longer hashes `name(objid(#0))` — the brittleness was the SHA1 capture of a configurable room name. Converted to a 14-bit semantic bitstring assertion that compares `name(objid(#0))` to `name(#0)` for the round-trip check, eliminating the SHA1 hash entirely and the configured-name dependency with it.
|
||||
|
|
@ -1,21 +0,0 @@
|
|||
# DB Backend Tests — Open Issues
|
||||
|
||||
Updated: 2026-04-10
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~Standalone DB test harness no longer builds after backend file moves~~ FIXED
|
||||
|
||||
- Updated `db/Makefile` to use `INCDIR=../mux/include` for headers and `ENGDIR=../mux/modules/engine` for `.cpp` sources. `SQLITEDIR` unchanged.
|
||||
|
||||
## Opportunities
|
||||
|
||||
### ~~Add a path-stability check for test harnesses that compile moved engine code~~ FIXED
|
||||
|
||||
- `tests/db/Makefile` now has a `check-paths` target that verifies the required engine, header, and SQLite source paths before `all` or `test` builds run. If a dependency moves again, the harness now fails immediately with an actionable error instead of breaking later during compilation.
|
||||
|
||||
## Opportunities (New, 2026-04-10)
|
||||
|
||||
### ~~Interface tests never exercise reopen-on-disk persistence semantics~~ FIXED
|
||||
|
||||
- `test_backend.cpp` now has a temp-file-backed factory path plus `test_backend_persist_reopen()`, which writes data to an on-disk SQLite database, closes it, reopens it, revalidates `Get()` and `GetAll()`, deletes one attribute, then reopens again to confirm the deletion persisted. Reverified with `make -C tests/db test`.
|
||||
|
|
@ -1,46 +0,0 @@
|
|||
# WorldBuilder — Open Issues
|
||||
|
||||
Updated: 2026-04-10
|
||||
|
||||
## Bugs
|
||||
|
||||
### ~~`test_worldbuilder.py` only works when run from `tools/worldbuilder/`~~ FIXED
|
||||
|
||||
- Test harness now calls `os.chdir()` to its own directory at startup, so fixture paths resolve correctly regardless of the caller's working directory.
|
||||
|
||||
## Additional Issues Found (2026-03-27 Survey)
|
||||
|
||||
### ~~File handle leak in executor.py~~ FALSE ALARM
|
||||
|
||||
- On inspection, `executor.py:593-595` already closes the log file in a `finally` block.
|
||||
|
||||
### ~~Softcode lint KNOWN_FUNCTIONS is incomplete and hand-maintained~~ FIXED
|
||||
|
||||
- Replaced hand-maintained ~100-entry set with authoritative 482-entry set extracted from `builtin_function_list[]` in `mux/modules/engine/functions.cpp`. Regeneration command documented in the source comment.
|
||||
|
||||
### ~~Dangerous pattern detection is incomplete~~ FIXED
|
||||
|
||||
- `softcode_lint.py` now flags additional high-risk patterns for review: `@pemit` with `%#`, `@trigger`, dynamic `%(...)` substitution, `setr()`, and `mail*()` calls. `test_worldbuilder.py` includes focused coverage for each new heuristic.
|
||||
|
||||
### ~~Executor SSL validation disabled~~ FIXED
|
||||
|
||||
- **File:** `executor.py:37-40`
|
||||
- `MuxConnection` now verifies certificates and hostnames by default. `executor.py` and `importer.py` expose an explicit `--insecure` flag for self-signed or internal development servers that still need the previous behavior.
|
||||
|
||||
### ~~Live adapter undocumented server format assumptions~~ FIXED
|
||||
|
||||
- **File:** `live_adapter.py`, `executor.py`
|
||||
- Replaced the free-form response-parsing path with a sentinel-wrapped `think` round-trip. `live_adapter.think_expr()` wraps a softcode expression as `think WB1[<expr>]WB2`; `parse_think_response()` recovers the evaluated value from the response buffer (taking the *last* sentinel pair so we land on the `think` output rather than the echoed command); `query_think()` and `query_last_created()` are the high-level helpers. `executor.py` now uses `lastcreate(me, R|E|T)` for room/exit/thing dbref capture after `@dig`/`@open`/`@create`, eliminating the regex scan of `@dig`/`@open` output text via `extract_dbref()`. Thing creation also now captures its dbref (previously silently dropped). The module docstring in `live_adapter.py` documents the server-format contract the tool still depends on (`%L` for current location; `lastcreate` + `think` sentinel for everything else). `extract_dbref()` was removed as dead code.
|
||||
|
||||
## Verified Checks
|
||||
|
||||
- `python3 tools/worldbuilder/test_diff_v3.py` passes.
|
||||
- `python3 tools/worldbuilder/test_reconciler.py` passes.
|
||||
|
||||
### ~~Attribute writes are emitted without MUX escaping~~ FIXED
|
||||
|
||||
- `executor.py` and `live_adapter.py` now route room/thing attribute payloads through `mux_escape()` before emitting `&attr=` commands, so `%`, tabs, and newlines are serialized safely instead of being interpreted by the server. `mux_escape()` itself now actually converts `\t` to `%t`, matching its documented contract. `test_worldbuilder.py` includes focused coverage for escaped room and thing attributes.
|
||||
|
||||
### ~~`verify()` can report success while exits and things are wrong~~ FIXED
|
||||
|
||||
- `executor.verify()` now verifies exits by stored dbref/object id, checks exit names, and compares `home(<exit>)` against the expected destination. It also verifies managed things with the same object-id/name/description/attr checks used for rooms, plus a `loc()` check for placement. `test_worldbuilder.py` now exercises both exit-destination drift and thing drift.
|
||||
Loading…
Add table
Add a link
Reference in a new issue