web: downloadFile.ts enhancements (downloadBlob, fileNameFromUri, isHttpUrl) #1560

Activity

1. cliffhall

   added

   v2Issues and PRs for v2Issues and PRs for v2

   on Jul 1, 2026
2. cliffhall

   added a parent issue on Jul 1, 2026

   • Tracking: re-implement PR #1510 (spec-conformant Apps host + programmatic review path) as scoped issues #1579
3. cliffhall

   self-assigned this

   on Jul 1, 2026
4. cliffhall

   linked a pull request that will close this issuefeat(web): downloadFile.ts enhancements (downloadBlob, fileNameFromUri, isHttpUrl) #1586on Jul 1, 2026
5. cliffhall commented on Jul 2, 2026

   

   cliffhall

   on Jul 2, 2026

   MemberAuthor

   More actions

   Smoke test & AGENTS.md audit — PR #1586

   Reviewed at commit 26478159 (feat(web): generalize downloadFile.ts with downloadBlob, fileNameFromUri, isHttpUrl). Diff touches only clients/web/src/lib/downloadFile.ts (+55/−9) and its test (clients/web/src/test/lib/downloadFile.test.ts).

   1. Smoke test (real output)

   Pure helpers (fileNameFromUri, isHttpUrl) — driven in Node 25 (native type-stripping) via a throwaway script importing the real source, fed http/https/data/blob URIs, query strings, backslash paths, control/format chars, and empty input:

   === fileNameFromUri ===
   "https://x/y/z/report.pdf"                   -> "report.pdf" (len 10)
   "http://example.com/files/data.csv?tok..."   -> "data.csv_token=abc&x=1" (len 22)
   "C:\Users\me\report.pdf"                      -> "report.pdf" (len 10)
   "https://x/a<ZWSP>b<RLO>c*?.txt"              -> "abc_.txt" (len 8)
   "https://x/"                                  -> "download" (len 8)
   "https://x/na:me<>|weird?.bin"                -> "na_me_weird_.bin" (len 16)
   "data:text/plain;base64,SGVsbG8="            -> "plain;base64,SGVsbG8=" (len 21)
   "blob:https://x/2b3c-uuid"                    -> "2b3c-uuid" (len 9)
   "just-a-name.json"                            -> "just-a-name.json" (len 16)
   "https://x/aaaa…(300 a's).txt"               -> "aaaa…" (len 255)  <- capped at 255
   ""                                            -> "download" (len 8)
   
   === isHttpUrl ===
   "https://example.com"              -> "https://example.com/"
   "http://localhost:3000/a?b=1"      -> "http://localhost:3000/a?b=1"
   "HTTPS://EXAMPLE.COM/Path"         -> "https://example.com/Path"
   "javascript:alert(1)"              -> null
   "data:text/html,<b>"               -> null
   "file:///etc/passwd"               -> null
   "ftp://host/file"                  -> null
   "blob:https://x/uuid"              -> null
   "not a url"                        -> null
   ""                                 -> null
   

   All correct: last path segment extracted (fwd- and back-slash split), control/format chars stripped, disallowed \/:*?"<>| collapsed to _, 255-char cap, "download" fallback; isHttpUrl allowlists http(s) only (scheme lower-cased) and returns null for every other scheme and for unparsable input. Two benign edge behaviors worth noting (not bugs, no UI consumer yet): a query string on a path-style URI is retained in the suggested name with ?→_ (e.g. data.csv?token=abc&x=1 → data.csv_token=abc&x=1), and a bare data:/blob: URI passed to fileNameFromUri yields a nonsense tail — but real call sites gate those through isHttpUrl first, so they never reach fileNameFromUri.

   downloadBlob (DOM) — needs a document, so exercised under happy-dom with the committed vitest suite (mocks URL.createObjectURL/revokeObjectURL, which happy-dom doesn't ship, and fake timers). It asserts the object URL is created, the temp anchor gets the right href/download + is clicked, is removed from the body, and the revoke is deferred (not called synchronously; fires on runAllTimers), including the try/finally cleanup-on-throw path and blob-type passthrough:

   ✓ creates an anchor with the right href + download attributes and clicks it
   ✓ still cleans up the anchor + URL when click() throws (try/finally guard)
   ✓ passes the JSON string into the Blob constructor verbatim
   ✓ downloadBlob passes the given blob through and honours its type
   ✓ buildExportFilename … (5)
   ✓ fileNameFromUri … (5)
   ✓ isHttpUrl … (2)
   
   Test Files  1 passed (1)
        Tests  16 passed (16)
   

   Playwright: not applicable. This PR adds library code only — no UI wiring (per the issue: "extends the library without any UI wiring"). There is no on-screen surface to drive, so a browser/visual check would have nothing to observe. happy-dom exercises the only DOM-dependent path (downloadBlob).

   2. AGENTS.md compliance

   • Mantine / React: N/A. downloadFile.ts is a framework-free browser library (no React, no components), so the Mantine/theme/subcomponent rules don't apply.

   • TypeScript: pass. No any, no @ts-*/eslint-disable suppressions in the diff. downloadBlob / downloadJsonFile return void; fileNameFromUri returns string; isHttpUrl returns URL | null. npx eslint on both changed files exits clean.

   • Coverage: passes the numeric bar, but the file is not gate-enforced. clients/web/src/lib/** is not in the coverage include globs in clients/web/vite.config.ts (which list src/components/**, src/utils/**, clients/web/server/**, and core/**), so downloadFile.ts never appears in npm run test:coverage and the ≥90 per-file gate does not apply to it in CI. Running v8 coverage scoped to just this file gives:

     File	% Stmts	% Branch	% Funcs	% Lines	Uncovered
     downloadFile.ts	100	92.3	100	100	line 47

     All four dimensions clear 90, so it would pass the gate if the glob covered it. The one uncovered branch (line 47, the ?? "" after uri.split(...).pop()) is a provably-dead defensive guard — String.prototype.split always returns a non-empty array, so .pop() is never undefined. Per AGENTS.md this is exactly the case for a justified /* v8 ignore … */ annotation; it's currently unannotated, which is harmless only because the file sits outside the gate.

     Flagged gap: src/lib escapes the per-file coverage gate. Worth adding src/lib/**/*.{ts,tsx} to the include array (and annotating that one dead branch) so this and future lib code is actually gated — otherwise a future regression here won't be caught by coverage.

   Verdict

   LGTM. The three helpers behave correctly across a wide input range, downloadBlob's deferred-revoke fix is verified under happy-dom, the diff is clean TypeScript with no suppressions, and per-file coverage is 100/92.3/100/100. Only follow-up: bring src/lib/** under the coverage include glob (and add the v8 ignore on line 47) so the file is gate-enforced, not just incidentally well-tested.

   Throwaway smoke script for the pure helpers (Node 25, run from /tmp — not committed)

   // Imports the real source (Node 25 strips TS types natively) and prints
   // fileNameFromUri / isHttpUrl output for a spread of inputs.
   import {
     fileNameFromUri,
     isHttpUrl,
   } from "clients/web/src/lib/downloadFile.ts";
   
   const pad = (s, n) => (s + " ".repeat(n)).slice(0, n);
   
   console.log("=== fileNameFromUri ===");
   const fnCases = [
     "https://x/y/z/report.pdf",
     "http://example.com/files/data.csv?token=abc&x=1",
     "C:\\Users\\me\\report.pdf",
     "https://x/a​b‮c*?.txt", // ZWSP + RLO override
     "https://x/",
     "https://x/na:me<>|weird?.bin",
     "data:text/plain;base64,SGVsbG8=",
     "blob:https://x/2b3c-uuid",
     "just-a-name.json",
     "https://x/" + "a".repeat(300) + ".txt",
     "",
   ];
   for (const c of fnCases) {
     const out = fileNameFromUri(c);
     console.log(`${pad(JSON.stringify(c), 44)} -> ${JSON.stringify(out)} (len ${out.length})`);
   }
   
   console.log("\n=== isHttpUrl ===");
   const urlCases = [
     "https://example.com",
     "http://localhost:3000/a?b=1",
     "HTTPS://EXAMPLE.COM/Path",
     "javascript:alert(1)",
     "data:text/html,<b>",
     "file:///etc/passwd",
     "ftp://host/file",
     "blob:https://x/uuid",
     "not a url",
     "",
   ];
   for (const c of urlCases) {
     const r = isHttpUrl(c);
     console.log(`${pad(JSON.stringify(c), 34)} -> ${r === null ? "null" : JSON.stringify(r.href)}`);
   }

   downloadBlob was exercised via the committed downloadFile.test.ts under happy-dom (needs a DOM), and per-file coverage via a throwaway vitest config scoped to src/lib/downloadFile.ts (both removed after the run).
6. cliffhall commented on Jul 2, 2026

   

   cliffhall

   on Jul 2, 2026

   MemberAuthor

   More actions

   🔗 Implemented by PR #1586. Its code is already integrated into the Wave 1 rollup branch 1579-wave-1 (see rollup PR #1601 / verification #1600). #1586 currently targets v2/main and will auto-show as Merged once the rollup lands in v2/main. The Development panel doesn't link it now because the closing keyword is on a non-default base branch.
7. cliffhall

   closed this as completedon Jul 9, 2026
8. pull

   added a commit that references this issue on Jul 28, 2026

   feat(web): generalize downloadFile.ts with downloadBlob, fileNameFrom…

   ...

   This issue will close once commit 2647815 is merged into the 'main' branch.

   2647815
9. cliffhall

   added this to the v2.0.0 milestone on Jul 28, 2026