Skip to content

fix(ui): take api.js error text from the language pack instead of CJK literals - #183

Merged
argszero merged 1 commit into
mainfrom
fix/api-client-error-i18n
Sep 12, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/api-client-error-i18n

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

ui/js/api.js is the request choke point — every api.get/post/patch/del call site flows through its request(), and it is the only place that both builds error messages and feeds them to mapErr(). But it is not in src/i18n_pack.rs's input surface (I18N_JS / INDEX_HTML / APP_JS), so a Chinese literal written there escapes all three i18n assertions for three different reasons:

assertion why it misses api.js
C2006 key-existence both packs do contain the corresponding key → green
C2007 key-usage the matcher only recognises T("literal"), not mapErr("中文")
C2007 placeholders scan_t_call_sites / scan_static_attributes never open this file

Consequence, measured rather than inferred: in EN mode the user gets Chinese. The backend genuinely returns non-JSON bodies (400 / 415 with content-type: text/plain) and empty bodies (404 / 405 — verified live against dev), so the fallback branch is reachable, not defensive. Running the real api.js + real i18n.js over those byte-for-byte responses leaked CJK in 6 of 9 shapes:

LEAK 请求失败(HTTP 404)   LEAK 请求失败(HTTP 405)   LEAK 请求失败(HTTP 400)
LEAK 请求失败(HTTP 415)   LEAK 请求失败(HTTP 500)   LEAK 登录已过期,请重新登录

The 401 literal was not merely dead code: its ZH value is byte-identical to the existing key login.session.expired, which __atpLogout renders (app.js:2932). So a single user-visible sentence was English when reached via the status hook and Chinese when reached via the thrown message — the defect was the divergence between two render paths.

Provenance (git log -S): the literals were introduced by 8bd1063 (#78, P2-A — which is what api.js:1's rant 2026-08-18T11:49:52 citation points at). 68f9f70 (#86, Rant 2026-08-18T20:49:22, "i18n") wrapped 2 of the 3 literals feeding mapErr and left the third unwrapped while adding the mapErr(message) line — a same-commit omission inside a real rant's scope, not an orphaned-by-a-sibling-commit case.

Related Issue

None (no open issue covers this; the direction came from auditing which files the i18n gates actually read).

Changes

  • ui/js/api.js: the three CJK literals become key references — T("err.network"), T("login.session.expired"), T("err.http", { n: resp.status }). mapErr now applies only to the backend-supplied message (which is what an error table is for). After this change the file contains zero CJK string literals, so the new invariant needs no exemption list.
  • ui/js/api.js: the translator is a hoisted function declaration, deliberately not const T = window.t as in app.jsindex.html loads api.js (line 845) before i18n.js (line 847), so a module-level capture would bind permanently to undefined. The name T is reused on purpose so the gate's existing T("literal") recogniser applies to these call sites (adding a second recognition rule would make the two gates measure different sets — pitfall 75).
  • ui/js/i18n.js: one new key err.http in both packs (请求失败(HTTP {n}) / Request failed (HTTP {n})). The ZH value keeps today's exact wording, which is what makes the zh-output byte-comparison below a meaningful control. The 401 half needs no new key because login.session.expired already carries that literal.
  • src/i18n_pack.rs: API_JS added as a gate input + two new test-only assertions (no production code, no new dependency).
  • ui/README.md: key count 785 → 786; documented the "api.js holds keys, not copy" convention.
  • ui/index.html: cache-bust -4-5.

Why the fallback must not go through ERR_MAP

mapErr ends in t(ERR_MAP[best][1]) with no vars argument, so a value containing {n} passed through it would render a literal {n} forever. The division of labour is therefore: mapErr for backend prose, a direct t(key, { n }) for api.js's own text. Verified: t("err.http")"Request failed (HTTP {n})", t("err.http", {n:500})"Request failed (HTTP 500)".

Tests

  • cargo test 全部通过 — 167 passed (165 before + 2 new)
  • cargo fmt --check 通过
  • cargo clippy --all-targets -- -D warnings 通过
  • 新增/更新了单元测试

New assertions (both test-only, include_str!, zero production code, zero new dependency):

  1. api_client_error_text_is_key_based — (a) api.js contains no non-ASCII outside comments (comment-aware stripping, so a same-line code; // 注释 mix is still caught); (b) every key literal in api.js resolves in both packs; (c) every placeholder a used key declares is supplied by its call site.
  2. api_js_checker_detects_injected_defects — counting-neutral injected corpora proving each assertion can actually fail (a CJK literal, a same-line code+comment mix, a mistyped key, a placeholder without vars), plus the positive controls that comments must stay exempt and a real key must not be reported.

Evidence (A/B over 9 response shapes × {en, zh}, real files, vm sandbox, fetch stubbed with live-captured bodies):

before (HEAD) after
EN CJK leaks 6 / 9 0 / 9
positive controls translated 3 / 3 3 / 3
zh output baseline byte-identical
EN values changed exactly 6

The 3 positive controls (a mapped 400 JSON, a mapped 502 JSON, and the fetch-rejection path) translate correctly before and after, so the probe discriminates rather than just reporting "0".

Teeth proof: injecting a single CJK literal back into a temp copy of api.js — a change that alters no counts — makes exactly the new assertion fail and the leak reappear in the probe (1/9, with 2/3 controls intact). The file was restored byte-identically afterwards and re-verified.

Deliberate boundaries (not oversights)

  • err.loadFail remains on its 15 direct T("err.loadFail") call sites and must not enter ERR_MAP.
  • The backend 服务器内部错误 / 内部错误 producers have 0 production sources in src/, so they are unreachable rather than missed.
  • Forms pre-validate amount / name / reason (app.js:2638 / 3659 / 1224), so the backend amount 必须大于 0 is client-unreachable.
  • err.generic was not added: the fallback branch always has a resp.status, so a status-less variant would be unreachable — adding a mapping nobody can reach is unverifiable work.

Checklist

  • 分支命名符合约定(fix/
  • Commit message 使用 Conventional Commits 格式
  • 单一职责,改动最小化(一个 PR 只处理 api.js 的错误文案来源)

… literals

`ui/js/api.js` is the request choke point (all `api.*` call sites flow through it)
and the only place that both builds error messages and calls `mapErr`. But it is
outside `src/i18n_pack.rs`'s input surface, so its matching form `mapErr("中文")`
escapes all three i18n assertions for three different reasons: the key-existence
gate sees both packs contain the key; the key-usage gate only recognises
`T("literal")`; the placeholder gate never opens the file.

Measured against real dev responses: the backend returns non-JSON (`400`/`415`
text/plain) and empty (`404`/`405`) bodies, so the fallback branch is reachable,
not defensive. Running the real files in EN mode over those byte-for-byte bodies
leaked Chinese in 6 of 9 shapes ("请求失败(HTTP 404/405/400/415/500)",
"登录已过期,请重新登录").

The 401 literal was not merely dead: its ZH value is byte-identical to the
existing key `login.session.expired`, which `__atpLogout` renders - so one
user-visible sentence was translated when reached via the status hook and
Chinese when reached via the message.

Fix: `api.js` now holds keys, not copy.
- `T("err.network")`, `T("login.session.expired")`, `T("err.http", { n })`
  replace the three CJK literals; `mapErr` now applies only to the
  backend-supplied message (which is what an error table is for).
- `T` is a hoisted function declaration, not `const T = window.t` as in
  app.js: index.html loads api.js *before* i18n.js, so a module-level capture
  would bind permanently to undefined. The name is reused deliberately so the
  shared `T("literal")` scanner (pitfall 75) recognises these call sites.
- One new key `err.http` (ZH keeps today's exact wording, so zh output is
  byte-identical); the 401 half reuses an existing key, so no second key.
- The fallback must not go through `ERR_MAP`: `mapErr` ends in `t(key)` with no
  `vars`, so a value containing `{n}` would render a literal `{n}` forever.

New tests (test-only, `include_str!`, no production code, no new dependency):
- `api_client_error_text_is_key_based` - (a) no non-ASCII in api.js outside
  comments, (b) every key literal resolves in both packs, (c) every placeholder
  a used key needs is supplied by its call site.
- `api_js_checker_detects_injected_defects` - counting-neutral injected corpora
  proving each assertion can fail; comments must stay exempt.

Verification: A/B over 9 response shapes x {en, zh}: en 6 leaks -> 0, zh
byte-identical, exactly 6 values changed; 3/3 positive controls still translate
before and after; a counting-neutral CJK injection reproduces the leak and fails
exactly the one new assertion. cargo test 167 passed, fmt/clippy clean.
@argszero
argszero merged commit 6f8b599 into main Sep 12, 2026
1 check passed
@argszero
argszero deleted the fix/api-client-error-i18n branch September 12, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant