From 88ce06deacef90dbc5414c593f6e6b77e223e251 Mon Sep 17 00:00:00 2001 From: Aybook <265313202+Aybook@users.noreply.github.com> Date: Sat, 18 Jul 2026 18:09:43 +0200 Subject: [PATCH] fix(blocklist/whitelist): localize the export/import failure reasons (#456) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both plugins render every success reason and most failure reasons through a lang template, but three technical failure returns in the JSONL export/import helpers were raw English literals - "open failed: " (export + import) and "json.encode failed: " (export) - and the dispatcher's `reply( msg or tostring( err ) )` surfaced them verbatim regardless of cfg.language. (The issue also listed `add` as affected; re-derived from source, `add` was already localized - do_add_entry always returns a utf_format(msg_*) message - so only the three export/import strings were the real gap.) Fixed by adding two per-reason keys, msg_open_failed ("Could not open %s: %s", now also naming the path) and msg_encode_failed ("JSON encode failed: %s"), in en + de for both plugins, and routing the three raw returns through them - matching the plugins' existing per-reason localization convention. The per-row _sanitize_import_row reasons are deliberately left English: they route to hub_debug only (script log), never to the operator reply, which shows the already-localized added/skipped/errors summary. Also gave etc_blocklist the _do_export_jsonl / _do_import_jsonl test seams its twin etc_whitelist already exported (§1a.1 twin parity). Both plugins v0.02 -> v0.03. Provably fails pre-fix (§1a.7): etc_blocklist_test.lua (via the real dispatcher) and etc_whitelist_test.lua (via the seams), both already on each smoke.yml leg, gain cases that force an import open-failure and an export encode-failure and assert the reply renders the template ("Could not open" / "JSON encode failed") and no longer the raw strings - four checks per plugin, red on the unpatched code. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 ++ scripts/etc_blocklist.lua | 12 ++++++--- scripts/etc_whitelist.lua | 10 ++++--- scripts/lang/etc_blocklist.lang.de | 2 ++ scripts/lang/etc_blocklist.lang.en | 2 ++ scripts/lang/etc_whitelist.lang.de | 2 ++ scripts/lang/etc_whitelist.lang.en | 2 ++ tests/unit/etc_blocklist_test.lua | 42 ++++++++++++++++++++++++++++++ tests/unit/etc_whitelist_test.lua | 32 +++++++++++++++++++++++ 9 files changed, 98 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5789e486..b97d4c61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,8 @@ land on `release/3.1.x` per ### Bugfixes +- **`etc_blocklist` / `etc_whitelist`: `+blocklist`/`+whitelist export` and `import` printed their failure reason in English regardless of `cfg.language`** ([#456](https://github.com/luadch-ng/luadch/issues/456)). Both plugins localise every *success* and most *failure* reasons through a lang template, but three technical failure returns in the JSONL export/import helpers were raw English string literals - `"open failed: "` (export and import) and `"json.encode failed: "` - and the dispatcher's `reply( msg or tostring( err ) )` surfaced them verbatim. The matching templates (the generic `msg_export_fail` / `msg_import_fail`) existed in the lang files once but were never wired in and were removed as dead by [#458](https://github.com/luadch-ng/luadch/pull/458) (the #447 dead-code arc); this wires localisation back with granular per-reason keys instead. (The issue's table also listed `add` as affected; re-derived from source, `add` was already localised - `do_add_entry` always returns a `utf_format( msg_*, ... )` message - so only the three export/import strings were the real gap.) Fixed by adding two per-reason keys, `msg_open_failed` (`"Could not open %s: %s"`, now also naming the path) and `msg_encode_failed` (`"JSON encode failed: %s"`), in **en + de** for both plugins, and routing the three raw returns through them - matching the plugins' existing per-reason localisation convention. `etc_blocklist` / `etc_whitelist` v0.02 -> v0.03. Also gave `etc_blocklist` the `_do_export_jsonl` / `_do_import_jsonl` test seams its twin `etc_whitelist` already exported (§1a.1 twin parity). Provably fails pre-fix (§1a.7): the existing `tests/unit/etc_blocklist_test.lua` and `etc_whitelist_test.lua` (both smoke.yml legs) gain cases that force an import open-failure and an export encode-failure and assert the reply renders the template (`"Could not open"` / `"JSON encode failed"`) and no longer the raw `"open failed:"` / `"json.encode failed:"` - four checks per plugin that are red on the unpatched code. 3.2.x only, not backported. + - **`cmd_userinfo`: the `+userinfo` value column was ragged - it jumped between positions and the `Sent:` / `Received:` pair sat offset from the block above** ([#459](https://github.com/luadch-ng/luadch/issues/459)). The labels come from the (translatable) lang file with inconsistent widths (`Nick:` vs `Version:` vs `Received:`), and the code prepended a **fixed tab count** to each value (`\t\t`, and inconsistently a single `\t` for `Sent`/`Received`). Tabs snap to 8-column stops, so the value column landed wherever the label length pushed it, and it also depended on how each client renders a tab - so the alignment was never reliable. Fixed by aligning in code, independent of tab-stop width: a small `align_labels` pass pads every value line's label to a single width **computed from the actual labels** (so it is correct in any language, and re-derives automatically if a label is retranslated) and lets the value follow directly; the fixed tab prefixes are gone. Space-padding in a monospace chat is the only client-independent alignment. Banner and blank lines are untouched, and the second `%s` on the `Level: %s [ %s ]` line rides along in the tail. The lang files are not touched - `align_labels` normalises the loaded format string, so no per-language space-counting to maintain. Cosmetic only, no functional change; `cmd_userinfo` v0.23 -> v0.24. Provably fails pre-fix (§1a.7): a new unit test (`tests/unit/cmd_userinfo_align_test.lua`, both smoke.yml legs) drives `align_labels` against the **real** `msg_userinfo` of both shipped lang files and asserts the raw format's value column is ragged (the bug) while the aligned one is uniform, in `en` and `de` independently. (`align_labels` is exposed as an `_`-prefixed test seam, so `cmd_userinfo` joins the seam-only plugins noted in `docs/PLUGIN_API.md` §8.) 3.2.x only, not backported. - **`etc_cmdlog`: `+cmdlog show` and `GET /v1/log/cmd` showed a frozen, possibly mixed-language log because the language labels were baked into each line at write-time** ([#460](https://github.com/luadch-ng/luadch/issues/460)). The `onBroadcast` writer stored `date .. msg1 .. cmd .. args .. msg2 .. nick`, embedding the labels active *when the command ran*; both readers dumped the file raw and never re-formatted. So every entry was frozen in its write-time language, a hub whose `cfg.language` changed showed a **mix**, and on a German hub whose entries all predated the switch `+cmdlog show` printed English (`Command:` / `used by:`) despite every other plugin reporting German. Distinct from the v1.4 `#301`-class fix below (that one was unreachable lang keys; this is a design flaw in *when* localization happens). Fixed by storing entries **language-neutral** - four fields (`ts`, `cmd`, `args`, `nick`) separated by the US control byte (`0x1f`) - and applying the current `msg1` / `msg2` labels at **read-time** via one shared `render_line`, on both the ADC and HTTP path, so old and new entries alike render in the hub's current language. Migration is automatic and lossless: an old baked line has no delimiter, so `render_line` returns it as-is (its frozen language) rather than dropping or mis-parsing it. Every field is run through `util.strip_control_bytes` at write-time, which both keeps the delimiter out of any field and closes a **latent corruption** where a newline in a command's arguments split one entry across two log lines. The `GET /v1/log/cmd` response contract is **unchanged** (`{lines:[string], returned, total_lines}`); its lines are simply re-localized now instead of frozen - so no `docs/HTTP_API.md` change was needed. Returning structured fields there is a possible future contract improvement, deliberately deferred to keep this a non-breaking language fix. `etc_cmdlog` v1.4 -> v1.5. Provably fails pre-fix (§1a.7): a new unit test (`tests/unit/etc_cmdlog_test.lua`, both smoke.yml legs) writes an entry under `en`, switches the hub to `de`, and asserts `+cmdlog show` renders `Befehl:` / `benutzt von:` - on the unpatched plugin the labels are baked English and it fails; it also covers the old-line raw fallback, a mixed-language file, redacted-arg storage, control-byte stripping, and the HTTP path. 3.2.x only, not backported. diff --git a/scripts/etc_blocklist.lua b/scripts/etc_blocklist.lua index 2bd56003..b14887d5 100644 --- a/scripts/etc_blocklist.lua +++ b/scripts/etc_blocklist.lua @@ -55,7 +55,7 @@ -------------- local scriptname = "etc_blocklist" -local scriptversion = "0.02" +local scriptversion = "0.03" local cmd_main = "blocklist" @@ -139,6 +139,8 @@ local msg_import_ok = lang.msg_import_ok or "%s imported %d entries from local msg_unsafe_path = lang.msg_unsafe_path or "Path '%s' is unsafe: %s" local msg_import_level = lang.msg_import_level or "Import requires level %d or higher (you are %d)." local msg_save_failed = lang.msg_save_failed or "Failed to persist blocklist: %s" +local msg_open_failed = lang.msg_open_failed or "Could not open %s: %s" +local msg_encode_failed = lang.msg_encode_failed or "JSON encode failed: %s" ---------- @@ -324,7 +326,7 @@ local function do_export_jsonl( actor_label ) local f, ferr = io.open( path, "w" ) if not f then - return false, "open failed: " .. tostring( ferr ) + return false, utf_format( msg_open_failed, path, tostring( ferr ) ) end local count = 0 @@ -347,7 +349,7 @@ local function do_export_jsonl( actor_label ) } if not line then f:close( ) - return false, "json.encode failed: " .. tostring( jerr ) + return false, utf_format( msg_encode_failed, tostring( jerr ) ) end f:write( line, "\n" ) count = count + 1 @@ -412,7 +414,7 @@ local function do_import_jsonl( path, actor_label, actor_level ) end local f, ferr = io.open( path, "r" ) if not f then - return false, "open failed: " .. tostring( ferr ) + return false, utf_format( msg_open_failed, path, tostring( ferr ) ) end local added, skipped, errors = 0, 0, 0 @@ -1041,6 +1043,8 @@ return { _parse_expires_date = parse_expires_date, _do_add_entry = do_add_entry, _do_del_entry = do_del_entry, + _do_export_jsonl = do_export_jsonl, + _do_import_jsonl = do_import_jsonl, _sanitize_import_row = _sanitize_import_row, _format_show = format_show, _format_count = format_count, diff --git a/scripts/etc_whitelist.lua b/scripts/etc_whitelist.lua index cb443e96..fa702c99 100644 --- a/scripts/etc_whitelist.lua +++ b/scripts/etc_whitelist.lua @@ -60,7 +60,7 @@ -------------- local scriptname = "etc_whitelist" -local scriptversion = "0.02" +local scriptversion = "0.03" local cmd_main = "whitelist" @@ -152,6 +152,8 @@ local msg_bad_cidr = lang.msg_bad_cidr or "Invalid CIDR / IP: %s" local msg_bad_expires = lang.msg_bad_expires or "Invalid expires date '%s'. Expected YYYY-MM-DD." local msg_added = lang.msg_added or "%s added whitelist entry #%d (%s, source=%s)." local msg_save_failed = lang.msg_save_failed or "Failed to persist whitelist: %s" +local msg_open_failed = lang.msg_open_failed or "Could not open %s: %s" +local msg_encode_failed = lang.msg_encode_failed or "JSON encode failed: %s" local msg_removed = lang.msg_removed or "%s removed whitelist entry #%d (%s, source=%s)." local msg_remove_failed = lang.msg_remove_failed or "whitelist.del failed: %s" local msg_not_found = lang.msg_not_found or "No whitelist entry with id #%d." @@ -294,7 +296,7 @@ local function do_export_jsonl( actor_label ) end local f, ferr = io.open( path, "w" ) if not f then - return false, "open failed: " .. tostring( ferr ) + return false, utf_format( msg_open_failed, path, tostring( ferr ) ) end local count = 0 local rows = whitelist.list( { source = "manual" } ) @@ -312,7 +314,7 @@ local function do_export_jsonl( actor_label ) } if not line then f:close( ) - return false, "json.encode failed: " .. tostring( jerr ) + return false, utf_format( msg_encode_failed, tostring( jerr ) ) end f:write( line, "\n" ) count = count + 1 @@ -364,7 +366,7 @@ local function do_import_jsonl( path, actor_label, actor_level ) end local f, ferr = io.open( path, "r" ) if not f then - return false, "open failed: " .. tostring( ferr ) + return false, utf_format( msg_open_failed, path, tostring( ferr ) ) end local added, skipped, errors = 0, 0, 0 local error_log_cap = 5 diff --git a/scripts/lang/etc_blocklist.lang.de b/scripts/lang/etc_blocklist.lang.de index f70a1c77..fe25fecb 100644 --- a/scripts/lang/etc_blocklist.lang.de +++ b/scripts/lang/etc_blocklist.lang.de @@ -37,6 +37,8 @@ return { msg_export_ok = "%s hat %d Blocklist-Eintraege nach %s exportiert.", msg_import_ok = "%s hat %d Eintraege aus %s importiert (%d uebersprungen, %d Fehler).", msg_unsafe_path = "Pfad '%s' ist unsicher: %s", + msg_open_failed = "Konnte %s nicht oeffnen: %s", + msg_encode_failed = "JSON-Kodierung fehlgeschlagen: %s", msg_import_level = "Import erfordert Level %d oder hoeher (du hast %d).", msg_save_failed = "Speichern der Blocklist fehlgeschlagen: %s", diff --git a/scripts/lang/etc_blocklist.lang.en b/scripts/lang/etc_blocklist.lang.en index 3be4c385..c3afbe59 100644 --- a/scripts/lang/etc_blocklist.lang.en +++ b/scripts/lang/etc_blocklist.lang.en @@ -37,6 +37,8 @@ return { msg_export_ok = "%s exported %d blocklist entries to %s.", msg_import_ok = "%s imported %d entries from %s (%d skipped, %d errors).", msg_unsafe_path = "Path '%s' is unsafe: %s", + msg_open_failed = "Could not open %s: %s", + msg_encode_failed = "JSON encode failed: %s", msg_import_level = "Import requires level %d or higher (you are %d).", msg_save_failed = "Failed to persist blocklist: %s", diff --git a/scripts/lang/etc_whitelist.lang.de b/scripts/lang/etc_whitelist.lang.de index a39124e1..4fd3c27f 100644 --- a/scripts/lang/etc_whitelist.lang.de +++ b/scripts/lang/etc_whitelist.lang.de @@ -38,6 +38,8 @@ return { msg_export_ok = "%s hat %d Whitelist-Eintraege nach %s exportiert.", msg_import_ok = "%s hat %d Eintraege aus %s importiert (%d uebersprungen, %d Fehler).", msg_unsafe_path = "Pfad '%s' ist unsicher: %s", + msg_open_failed = "Konnte %s nicht oeffnen: %s", + msg_encode_failed = "JSON-Kodierung fehlgeschlagen: %s", msg_import_level = "Import benoetigt Level %d oder hoeher (du bist %d).", } diff --git a/scripts/lang/etc_whitelist.lang.en b/scripts/lang/etc_whitelist.lang.en index 221e8925..9a4503b8 100644 --- a/scripts/lang/etc_whitelist.lang.en +++ b/scripts/lang/etc_whitelist.lang.en @@ -38,6 +38,8 @@ return { msg_export_ok = "%s exported %d whitelist entries to %s.", msg_import_ok = "%s imported %d entries from %s (%d skipped, %d errors).", msg_unsafe_path = "Path '%s' is unsafe: %s", + msg_open_failed = "Could not open %s: %s", + msg_encode_failed = "JSON encode failed: %s", msg_import_level = "Import requires level %d or higher (you are %d).", } diff --git a/tests/unit/etc_blocklist_test.lua b/tests/unit/etc_blocklist_test.lua index 5639155d..577e0b57 100644 --- a/tests/unit/etc_blocklist_test.lua +++ b/tests/unit/etc_blocklist_test.lua @@ -582,6 +582,48 @@ do _vfs_disable( ) end +---------------------------------------------------------------------- +-- #456: export/import failure reasons must be localized (rendered +-- through a lang template), not the raw English helper string. The +-- lang is empty here so the plugin uses its English fallbacks; the +-- point is that the FAILURE path now renders msg_open_failed / +-- msg_encode_failed at all (pre-fix it returned the raw +-- "open failed:" / "json.encode failed:" strings). +---------------------------------------------------------------------- + +do + _entries = { }; _next_id = 1 + _vfs_enable( ) + + -- import a path with no vfs content -> io.open returns nil -> the + -- open-failed branch. Reason must be the template, not the raw one. + local r1 + local u1 = { level = function( ) return 100 end, nick = function( ) return "op" end, + reply = function( _self, m ) r1 = m end } + _registered.hub.blocklist( u1, nil, "import cfg/blocklist-missing.jsonl" ) + truthy( "#456 import open-fail: localized 'Could not open'", + r1 and r1:find( "Could not open", 1, true ) ) + falsy( "#456 import open-fail: raw 'open failed:' gone", + r1 and r1:find( "open failed:", 1, true ) ) + + -- export with a failing encoder -> the json-encode branch. + _entries = { }; _next_id = 1 + plugin._do_add_entry( "192.0.2.0/24", { reason = "x" }, "op", 100 ) + local saved_encode = _G.dkjson.encode + _G.dkjson.encode = function( ) return nil, "boom" end + local r2 + local u2 = { level = function( ) return 100 end, nick = function( ) return "op" end, + reply = function( _self, m ) r2 = m end } + _registered.hub.blocklist( u2, nil, "export" ) + _G.dkjson.encode = saved_encode + truthy( "#456 export encode-fail: localized 'JSON encode failed'", + r2 and r2:find( "JSON encode failed", 1, true ) ) + falsy( "#456 export encode-fail: raw 'json.encode failed:' gone", + r2 and r2:find( "json.encode failed:", 1, true ) ) + + _vfs_disable( ) +end + ---------------------------------------------------------------------- -- Dispatcher: unknown verb path goes to msg_unknown_verb ---------------------------------------------------------------------- diff --git a/tests/unit/etc_whitelist_test.lua b/tests/unit/etc_whitelist_test.lua index 98fa1a73..4d92db5a 100644 --- a/tests/unit/etc_whitelist_test.lua +++ b/tests/unit/etc_whitelist_test.lua @@ -524,6 +524,38 @@ do _cfg_overrides = { } end +---------------------------------------------------------------------- +-- #456: export/import failure reasons must be localized (rendered +-- through a lang template), not the raw English helper string. Lang is +-- empty here so the English fallbacks apply; the point is that the +-- FAILURE path renders msg_open_failed / msg_encode_failed at all +-- (pre-fix it returned the raw "open failed:" / "json.encode failed:"). +---------------------------------------------------------------------- + +do + _vfs = { } + -- import a path not in the vfs -> io.open read returns nil -> the + -- open-failed branch. Reason must be the template, not the raw one. + local iok, ireason = wl._do_import_jsonl( "cfg/whitelist-missing.jsonl", "master", 100 ) + truthy( "#456 import open-fail: returns false", not iok ) + truthy( "#456 import open-fail: localized 'Could not open'", + ireason and ireason:find( "Could not open", 1, true ) ) + truthy( "#456 import open-fail: raw 'open failed:' gone", + not ( ireason and ireason:find( "open failed:", 1, true ) ) ) + + -- export with a failing encoder -> the json-encode branch. + wl._do_add_entry( "1.2.3.0/24", { reason = "r" }, "op", 100 ) + local saved = _G.dkjson.encode + _G.dkjson.encode = function( ) return nil, "boom" end + local eok, ereason = wl._do_export_jsonl( "op" ) + _G.dkjson.encode = saved + truthy( "#456 export encode-fail: returns false", not eok ) + truthy( "#456 export encode-fail: localized 'JSON encode failed'", + ereason and ereason:find( "JSON encode failed", 1, true ) ) + truthy( "#456 export encode-fail: raw 'json.encode failed:' gone", + not ( ereason and ereason:find( "json.encode failed:", 1, true ) ) ) +end + ---------------------------------------------------------------------- if _fails > 0 then