Skip to content

fix(ui): quote a CSV cell that carries a bare CR (RFC 4180 §2.6) - #275

Merged
argszero merged 1 commit into
mainfrom
fix/tx-csv-quote-cr
Sep 21, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/tx-csv-quote-cr

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

The transactions CSV exporter escapes a cell only when it contains ,, " or LF — but the exporter
joins records with CRLF ("\r\n"). A field carrying a bare CR therefore goes out unquoted, and
one record becomes two: the exported file no longer round-trips.

This adds CR to the escape class (/[",\n]/ → /[",\r\n]/, the RFC 4180 §2.6 set), names CR in the
ui/README.md export convention, and adds a gate so the four special characters cannot drift apart again.

Related Issue

Changes

  • ui/js/app.js (exportTxCsv, the cell() helper): add CR to the escape class, with a comment
    explaining why CR is part of the set. The ternary shape ("quote on demand") is deliberately kept.
  • ui/README.md: the CSV export convention now names CR (\r) explicitly instead of only "newline",
    and records the new gate's lexical scope next to it.
  • ui/index.html: cache-bust for app.js (procedural — read the live token, write a strictly greater one).
  • src/state_gate.rs: new invariant the_csv_cell_escaper_quotes_every_rfc4180_special plus its
    negative control the_csv_escaper_scanners_have_teeth.

Why this is a real defect and not a style choice

ui/js/app.js:1819 builds the document with …join("\r\n"), so CR is a record separator. RFC 4180
§2.6 requires a field containing ,, ", CR or LF to be quoted; the escaper lists three of the four.
Two columns can carry a bare CR:

column value producer
user t.user users.name ≤ POST /api/auth/register's req.name, which only .trim()s
key t.key_name || t.key_label || "—" api_keys.name ≤ POST /api/api-keys / PATCH /api/api-keys/:id (no field validation at the route layer), and the key_label fallback comes from another user's sharing note

Honest boundary (same nature as the admin model-price fix): the UI cannot produce a bare CR —
single-line <input> elements and browsers normalise CR/LF — so this is reachable only from a client
that talks to the API directly. It is a data-integrity/interoperability defect, not something a UI user
will hit by accident. It is still worth fixing: the exporter's own contract is "what you see in the table,
you get in the file", and a mangled row is silent.

Tests

  • cargo test — 312 passed / 0 failed (base main is 310; this change adds two tests:
    the gate and its negative control)

  • cargo fmt --check passes

  • cargo clippy --all-targets -- -D warnings passes

  • New unit tests added (2)

  • Gate verified before landing against a mirror of the real tree (compile + run the real
    src/state_gate.rs test binary, one arm per rule) — 7 arms, all as declared:

    arm declared what it proves
    base (unfixed) gate RED on rule 1a the gate really catches this defect
    fix 24 tests green (22 existing + 2 new) the fix is complete and regresses nothing
    m_cosmetic_comment RED on 1a a comment that mentions the four characters does not satisfy it
    m_result_discarded RED on rule 1b keeping the class but discarding the .test() verdict is caught
    m_tail_escape (/[",\r\n]|/) RED on rule 3 an all-four-characters class that still always quotes is caught
    m_second_impl RED on rule 2 a second CSV dialect in the same corpus is caught
    m_always_quote RED on 1a ("not found") "quote everything" reports a different error than "character missing"

    The two errors are deliberately distinguishable, so a reader can tell "there is no escape class"
    from "the class is incomplete".

  • Gate scope is documented in ui/README.md: the gate is lexical — it proves the four characters
    are written into the class and that the .test() verdict drives the branch. It does not prove JS regex
    semantics; that is covered by the DOM probe below.

  • Invariants after the edit (measured before → after): /[",\n]/-shaped classes 1 → 0; /[",\r\n]/ 0 → 1;
    [", occurrences across the whole UI corpus 1 → 1 (the escaper is still the only quoting implementation);
    .replace(/"/g 1 → 1 (the doubling half was not touched by accident); README "newline" wording 1 → 0;
    exactly one js/app.js script tag in index.html (shape pinned, token value procedural)

  • DOM probe (jsdom, real index.html + the four real scripts, fetch stubbed and accounted; every leg
    declares its expectation, printed beside PASS/FAIL). It drives the real #tx-export-btn, captures the
    Blob handed to URL.createObjectURL, asserts the UTF-8 BOM, and reads the file back with an RFC 4180
    reader (CRLF, bare CR and bare LF all terminate a record outside quotes — the reading Excel uses):

    tree reading
    fixed 10/10 as declared — 1 header + 2 records, widths [11,11,11], the cell round-trips as Bob\rCarol
    unfixed (--base) 10/10 as declared, {A1,A2,A3,A4} red — 4 records, widths [11,11,3,9], the cell truncated to Bob
    competing fix (verdict discarded ⇒ always quotes) rejected — {B2} red (44 quote characters on an all-plain fixture)

    The ordinary-row leg (B2) pins the shape the work order asks for: the escaper quotes on demand, so a
    fixture with no special characters must contain no quotes at all. A comma/quote field is still quoted and
    its quotes still doubled (C1).

Checklist

  • Branch name follows the convention (fix/…)
  • Commit message follows the convention (fix(ui): …)
  • PR targets the default branch (main)
  • Tests added for the change
  • No unrelated changes

`exportTxCsv` writes an RFC 4180 file whose record separator is `\r\n`,
but its cell escaper's special-character class was `/[",\n]/` — CR was
missing. A field carrying a bare CR was therefore written unquoted, and a
reader that treats a bare CR as a line break (Excel's universal-newline
reading) splits one record into two: the row count grows, the widths go
ragged and the cell's tail is lost. Only a client talking to the API
directly can put a CR in `user_name` / `key_name` — there is no form that
accepts one — which is the same class of input as C2043.

- `ui/js/app.js`: `/[",\r\n]/`, citing RFC 4180 §2.6. Still the only
  quoting implementation, still conditional (quote on demand) — the
  quote-doubling half and the ternary are unchanged.
- `src/state_gate.rs`: `the_csv_cell_escaper_quotes_every_rfc4180_special`
  (four rules, each with its own isolating A/B arm) plus
  `the_csv_escaper_scanners_have_teeth` (synthetic self-check: the five
  malformed shapes must report *different* errors, and the classifier
  compares element tokens rather than substrings — `\r\n` contains `\n`).
- `ui/index.html`: cache-bust bump for the app.js change.
- `ui/README.md`: name CR among the escaped specials and record the
  gate's lexical scope.

Verification. Gate instrument: all 7 arms as declared (base reddens on
the axis rule; comment-only, verdict-discarded, second-implementation,
always-quote and tail-`|` mutations all rejected; clippy clean on both
trees). jsdom probe driving the real `#tx-export-btn`: 10/10 on the fixed
tree; on the unfixed tree 10/10 as declared with {A1,A2,A3,A4} red
(4 records, widths [11,11,3,9], cell truncated to `Bob`); the
"verdict discarded" competing fix is rejected (axis green, shape leg red).

`cargo test` 310 -> 312 passed, `cargo fmt --check` clean,
`cargo clippy --all-targets -- -D warnings` clean.
@argszero

Copy link
Copy Markdown
Owner Author

Self-review (author is the committer on this repo)

Verified before merging, on the pushed head bb7f6f9:

Gate (lexical). Spliced src/state_gate.rs and compiled it as a real test binary, one mutation per arm — 7/7 arms as declared, each mutation reddening on the axis rule it targets (clippy -D warnings clean on both the unfixed and the fixed tree):

arm declared actual
base (unfixed) gate FAIL on the element rule FAIL on missing element \r
fix PASS PASS (24 tests ran)
m_cosmetic_comment FAIL on the element rule FAIL (a comment mentioning the four characters is not code)
m_result_discarded FAIL on the verdict rule FAIL (the .test() verdict is discarded)
m_tail_escape (`/[",\r\n] /`) FAIL on the degenerate rule
m_second_impl FAIL on the uniqueness rule FAIL (exactly one)
m_always_quote FAIL, different message FAIL (not found)

Probe (runtime). jsdom, real index.html + the four real scripts, driving the real #tx-export-btn and reading the captured Blob back with an RFC 4180 parser:

  • fixed tree: 10/10 as declared — 1 header + 2 records, widths [11,11,11], the CR-bearing cell round-trips intact;
  • unfixed tree (--base): 10/10 as declared, {A1,A2,A3,A4} red — 4 records, widths [11,11,3,9], the cell truncated to Bob;
  • competing fix (class correct, .test() verdict discarded ⇒ always quotes): rejected — axis green, shape leg {B2} red (44 quotes on an all-plain fixture).

Local. cargo test 312 passed / 0 failed (base main 310; this PR adds the gate and its synthetic self-check), cargo fmt --check clean, cargo clippy --all-targets -- -D warnings clean.

Issue linkage. closingIssuesReferences is empty and that is by design — this repository has no issue for this defect; the exporter's own contract (RFC 4180 §2.6) is the specification.

@argszero
argszero merged commit ed74e32 into main Sep 21, 2026
1 check passed
@argszero
argszero deleted the fix/tx-csv-quote-cr branch September 21, 2026 14:03
@argszero argszero mentioned this pull request Sep 24, 2026
10 tasks
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