From aad9c248dc8739ee7bebdd830e0b7c5bf8db0077 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Mon, 5 Oct 2026 21:30:29 -0400 Subject: [PATCH] fix(preview): budget every start tag ExcelJS will parse Admission counted cells, rows, merges and styles only in worksheets, styles.xml and workbook.xml, so the objects ExcelJS builds per element elsewhere (shared-string runs, fonts, fills, borders, comments, drawings, VML, tables) were bounded only by the inflated-byte caps, and empty stored deflate blocks pad a stream past the ratio cap. createXmlCounter now counts every start tag in every part except the pure-bytes xl/media/. entries into counts.elements and refuses above LIMITS.maxElements (2,000,000) as element-limit. Parts that read no attributes carry only a trailing '<' between chunks, so long text or binary is never taken for an oversized tag. Very tall sheets now scale only the scroll position: the scroll range maps onto the sheet's whole range and the tile is laid out at real row heights and column widths, with spans clipped at the spacer, instead of dividing every cell and heading by the scale. --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/spreadsheet-preview.js | 81 +++++++++++++++-------- src/web/public/spreadsheet-xlsx-core.js | 36 ++++++++-- test/spreadsheet-preview-worker.test.ts | 62 +++++++++++++++++ test/spreadsheet-preview.test.ts | 75 +++++++++++++++++++++ test/spreadsheet-xlsx-core.test.ts | 88 ++++++++++++++++++++++++- 7 files changed, 310 insertions(+), 36 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cdc6100f..3cd38854 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -293,7 +293,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments) -**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server: `spreadsheet-preview.js` fetches the raw route with `?preview=true` (413 above `MAX_XLSX_BROWSER_PREVIEW_BYTES`, 10 MB) and hands the bytes to `spreadsheet-preview-worker.js`, the only place the pinned `exceljs`/`fflate` vendor bundles load (never on page load). `admitXlsx()` caps the ZIP before ExcelJS runs, counting what ExcelJS will EXPAND as well as what it reads (a merge costs its area, a `` past 16384 is refused, validations are never parsed via `ignoreNodes`, and defined names are never expanded: the worker stubs `_definedNames.model`). ⚠️ The INDEX a row or sheet claims is bounded too, since ExcelJS allocates and walks up to it: a `` outside 1-1048576 and an `xl/workbook.xml` `` above `LIMITS.maxSheetId` (65535) are refused, and `sendTile` reuses the merges read at load (`mergesById`), never `sheet.model`, which rebuilds the whole sheet. ⚠️ The COLUMN index costs the same way (a row's cells live at `_cells[col - 1]`, so one `XFD` cell makes ExcelJS's `eachRow`/`eachCell`/`hasValues` visit 16,384 slots per row): `worksheetMetadata` builds each sheet's row and cell index from the keys that exist (`Object.keys` of `_rows` and `_cells`, skipping falsy and `Null`-type cells as `eachCell({ includeEmpty: false })` does), reads row heights in that pass and merges from `sheet._merges`, and `sendTile` reads cells from that index (`populatedRowsById`); never call ExcelJS's dense `eachRow`/`eachCell` there. `parseThemePalette` returns the default palette for a theme above 64 KB (64 * 1024 characters), since its patterns are quadratic on unclosed tags. ⚠️ Merges are capped per sheet (`LIMITS.maxMergesPerSheet`, 2,000) AND workbook-wide (`LIMITS.maxMerges`, 10,000), refused as `merge-limit` before ExcelJS loads, since ExcelJS's `_mergeCellsInternal` checks each merge against every earlier one on its sheet (quadratic). Every `` in `xl/styles.xml` is read with `readTagAttributes()` and refused (`number-format`) when its decoded `formatCode` is over 255 characters or has a `[` after its last `]`: ExcelJS's `isDateFmt` rescans to the end of the code per unclosed `[` once per numeric cell, and the code is echoed into the notice bar. ⚠️ `formatCellValue` caps every cell's display text at `LIMITS.maxCellTextChars` (1,000, ellipsis, never splitting a surrogate pair) for every value shape (rich text, hyperlink text, formula source, errors), since structured clone copies each tile cell's whole string to the page and the load timeout no longer covers tiles. `richTextPrefix` also visits at most `maxCellTextChars + 1` runs, not only keeps that much text: an empty `` adds nothing, so a shared string of a million empty runs was walked whole per cell, per tile (56 s a tile). ⚠️ Admission keys every entry on the name ExcelJS will SEE (`excelJsEntryName()`: JSZip's `.`/`..`/empty-segment resolution, then one leading `/` stripped), refuses two entries that land on one name, and treats anything matching ExcelJS's UNANCHORED `xl/worksheets/sheet.xml` as a worksheet, so `/xl/...` or `xl/./...` cannot skip a counter, and ExcelJS parses only a STORE-only archive rebuilt from the entries admission inflated (`buildAdmittedArchive()`), never the fetched bytes; cell text goes through `textContent`, formulas are never evaluated. Bumping either package or editing the worker/core changes `SPREADSHEET_ASSET_VERSION`, which `npm run check:public-assets` pins. xls/ods stay download-only. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server: `spreadsheet-preview.js` fetches the raw route with `?preview=true` (413 above `MAX_XLSX_BROWSER_PREVIEW_BYTES`, 10 MB) and hands the bytes to `spreadsheet-preview-worker.js`, the only place the pinned `exceljs`/`fflate` vendor bundles load (never on page load). `admitXlsx()` caps the ZIP before ExcelJS runs, counting what ExcelJS will EXPAND as well as what it reads (a merge costs its area, a `` past 16384 is refused, validations are never parsed via `ignoreNodes`, and defined names are never expanded: the worker stubs `_definedNames.model`). ⚠️ The INDEX a row or sheet claims is bounded too, since ExcelJS allocates and walks up to it: a `` outside 1-1048576 and an `xl/workbook.xml` `` above `LIMITS.maxSheetId` (65535) are refused, and `sendTile` reuses the merges read at load (`mergesById`), never `sheet.model`, which rebuilds the whole sheet. ⚠️ The COLUMN index costs the same way (a row's cells live at `_cells[col - 1]`, so one `XFD` cell makes ExcelJS's `eachRow`/`eachCell`/`hasValues` visit 16,384 slots per row): `worksheetMetadata` builds each sheet's row and cell index from the keys that exist (`Object.keys` of `_rows` and `_cells`, skipping falsy and `Null`-type cells as `eachCell({ includeEmpty: false })` does), reads row heights in that pass and merges from `sheet._merges`, and `sendTile` reads cells from that index (`populatedRowsById`); never call ExcelJS's dense `eachRow`/`eachCell` there. `parseThemePalette` returns the default palette for a theme above 64 KB (64 * 1024 characters), since its patterns are quadratic on unclosed tags. ⚠️ Merges are capped per sheet (`LIMITS.maxMergesPerSheet`, 2,000) AND workbook-wide (`LIMITS.maxMerges`, 10,000), refused as `merge-limit` before ExcelJS loads, since ExcelJS's `_mergeCellsInternal` checks each merge against every earlier one on its sheet (quadratic). Every `` in `xl/styles.xml` is read with `readTagAttributes()` and refused (`number-format`) when its decoded `formatCode` is over 255 characters or has a `[` after its last `]`: ExcelJS's `isDateFmt` rescans to the end of the code per unclosed `[` once per numeric cell, and the code is echoed into the notice bar. ⚠️ `formatCellValue` caps every cell's display text at `LIMITS.maxCellTextChars` (1,000, ellipsis, never splitting a surrogate pair) for every value shape (rich text, hyperlink text, formula source, errors), since structured clone copies each tile cell's whole string to the page and the load timeout no longer covers tiles. `richTextPrefix` also visits at most `maxCellTextChars + 1` runs, not only keeps that much text: an empty `` adds nothing, so a shared string of a million empty runs was walked whole per cell, per tile (56 s a tile). ⚠️ Every part ExcelJS parses (all but the pure-bytes `xl/media/.`) is also charged per START TAG against `LIMITS.maxElements` (2,000,000, refused as `element-limit`): ExcelJS builds an object per element in sharedStrings, styles, comments, drawings, VML and tables too, and empty stored blocks pad a deflate stream past the ratio cap, so 8.3M empty `` runs in a 375 KB file cost 570 MB of heap. ⚠️ Admission keys every entry on the name ExcelJS will SEE (`excelJsEntryName()`: JSZip's `.`/`..`/empty-segment resolution, then one leading `/` stripped), refuses two entries that land on one name, and treats anything matching ExcelJS's UNANCHORED `xl/worksheets/sheet.xml` as a worksheet, so `/xl/...` or `xl/./...` cannot skip a counter, and ExcelJS parses only a STORE-only archive rebuilt from the entries admission inflated (`buildAdmittedArchive()`), never the fetched bytes; cell text goes through `textContent`, formulas are never evaluated. Bumping either package or editing the worker/core changes `SPREADSHEET_ASSET_VERSION`, which `npm run check:public-assets` pins. xls/ods stay download-only. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index e990edd9..3a67c1a9 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -373,7 +373,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli ⚠️ **The preview overlay must outrank the panel that launched it.** `.file-preview-overlay` sits at `z-index: 5100`, above the response viewer (5000) and its backdrop (4999); at its historical 2000 a path clicked in the chat opened the overlay *behind* the chat, which reads as a dead link. It stays below the toast/picker band (10000+) so a "Saved" toast still lands on top. -**XLSX previews parse in the browser worker, and ExcelJS only ever sees what admission checked.** An `.xlsx` joins the raw routes like any other file (`?preview=true` caps it at 10 MB with a 413); the server never parses it. `spreadsheet-preview.js` hands the bytes to `spreadsheet-preview-worker.js`, the ONLY place the pinned `exceljs`/`fflate` vendor bundles load: fflate and `spreadsheet-xlsx-core.js` at worker start, ExcelJS only after `admitXlsx()` passes, and the page itself never loads either. `admitXlsx()` streams every entry through fflate and enforces the entry count, per-entry and total inflated bytes (64 MB), compression ratio, worksheet, cell, merge and style caps on the actual inflated output, refusing (never truncating) a workbook that trips one. ⚠️ Admission walks LOCAL headers while ExcelJS (JSZip) reads the CENTRAL directory, so overlapping entries (a stored entry hiding a whole `sheet1.xml`, a one-cell decoy later in the stream) once let a file be admitted as 1 cell and parsed as 300k. The worker therefore never hands ExcelJS the fetched bytes: it gets `buildAdmittedArchive()`, a STORE-only `fflate.zipSync(entries, { level: 0 })` of exactly the entries admission inflated, and a name streamed twice is refused. That costs one transient copy of the inflated entries (bounded by the same 64 MB cap) and is pinned by the overlapping-entry fixture in `test/spreadsheet-preview-worker.test.ts`. ⚠️ Admission also has to bound what ExcelJS EXPANDS, not just what it reads: ExcelJS 4.4.0 builds one object per covered cell of a ``, per address of a `` and per index up to ``, so a 6.5 KB file admitted as one cell once cost a gigabyte. The counter therefore charges each merge its full AREA against the cell caps (an unparseable `ref` is refused), refuses a `` whose `min`/`max` is past 16384, and the worker loads with `ignoreNodes: ['dataValidations']` (the preview never shows validations). Defined names expand the same way and live in `xl/workbook.xml`, where admission reads only the `` ids: ExcelJS's `DefinedNames` model setter builds one object per cell of every range (a whole-sheet name exhausted a 4 GB heap), so right after `new ExcelJS.Workbook()` the worker replaces `_definedNames.model` with an `Object.defineProperty` stub whose getter returns `[]` and whose setter drops the value. Print areas and titles are split off in the workbook xform's reconcile, before that setter runs, so they are unaffected, and `defineProperty` throws if a future ExcelJS renames `_definedNames` rather than silently expanding again. ⚠️ Every name admission uses is the one ExcelJS will SEE, not the stored one: JSZip resolves each entry name on load (`utils.resolve`: `.` and empty middle segments dropped, `..` pops a segment) and ExcelJS then strips one leading `/` and matches worksheets with an UNANCHORED `xl/worksheets/sheet.xml`. Checking the stored name once let `/xl/worksheets/sheet1.xml`, `xl/./...`, `xl//...`, `xl/xl/worksheets/sheet1.xml` and `xl/worksheets/sheet1.xml.x` skip every counter. `excelJsEntryName()` mirrors both steps; admission refuses two entries that resolve to one name, keys `inflatedEntries` (so the rebuilt archive) on the resolved name, picks the worksheet and styles counters from it, and counts any name matching ExcelJS's unanchored worksheet pattern as a worksheet. The local-versus-central consistency checks still compare the stored names. The counter reads ``/`` attributes IN ORDER from the tag name with a sticky regex that consumes each quoted value whole (a raw `>` or the other quote character is legal inside a value, so a first-match search could be fed a fake `ref`/`max`), and refuses a tag whose attributes do not parse up to `>` or repeat a name. It counts every `` (ExcelJS keeps one Row object per element, cells or not) against per-sheet and total row caps, and reads each ``'s attributes the same way. ⚠️ The INDEX a row or sheet claims is a cost too, not just how many there are: ExcelJS stores a row at `_rows[r - 1]` and walks `_rows` up to the largest index on every `eachRow` and `sheet.model` (five rows plus one `` took 5 s to load and 1.7 s per tile), and stores a sheet at `_worksheets[sheetId]`, which its `worksheets` getter slices and sorts (`sheetId="30000000"` on a one-cell workbook took 1.6 s and 557 MB). So a `` that is not plain digits in 1 to 1,048,576 is refused (an absent `r` is fine), and a counter for the resolved `xl/workbook.xml` reads every `` tag in order and refuses one that does not parse or whose `sheetId` is not plain digits up to `LIMITS.maxSheetId` (65535; real ids are small, and the `(?=[\s/>])` lookahead keeps the `` container out). The counter also counts every `` in styles.xml with no list-tracking state (a `` inside a comment used to desync it). It carries everything from the last `<` into the next inflate chunk, since `<` can never appear inside an attribute value, and a central-directory `compressedSize` that runs past the file is refused, since the ratio cap divides by it. Fixtures for each are in `test/spreadsheet-preview-worker.test.ts` and `test/spreadsheet-xlsx-core.test.ts`. ⚠️ `sendTile` must never read `sheet.model`, which rebuilds every row and cell model (10-19 ms per tile on a 100k-cell sheet, once per animation frame while scrolling): each sheet's merges are read once in `worksheetMetadata` and kept in `mergesById` next to `populatedRowsById` (replaced on load, cleared on dispose), and a worker test makes the `model` getter throw before requesting a tile. ⚠️ The COLUMN index a cell claims costs the same way: ExcelJS keeps a row's cells at `_cells[col - 1]`, so a row whose only cell sits in `XFD` is a dictionary-mode array, and ExcelJS's `row.eachCell` (`_cells.forEach`) and `row.hasValues` (which `sheet.eachRow` calls) visit every index up to 16,384, about 0.5 ms per row (10,000 such rows took 13 s to load in Chromium, and each tile paid it again; `XFD` is a legal column, so admission cannot refuse it). `worksheetMetadata` therefore never calls ExcelJS's dense `eachRow`/`eachCell`: `populatedRowIndex()` walks `Object.keys(sheet._rows)` and, per row, `Object.keys(row._cells)`, sorted numerically, skipping falsy and `ValueType.Null` cells exactly as `eachCell({ includeEmpty: false })` does and keeping a row only when it holds one such cell (ExcelJS's `hasValues`). Styles, the extent and row heights come from that one pass, merges from `sheet._merges` (the map the `model` getter itself lists), and the per-row cell lists are what `populatedRowsById` keeps, so `sendTile` reads a row's cells from the index instead of `row.eachCell`. Worker tests make `eachRow`, `eachCell`, `hasValues` and the `model` getter throw across load and a tile, and load and tile 3,000 one-`XFD`-cell rows. `parseThemePalette` returns the default palette for a theme above 64 KB (64 * 1024 characters of the decoded XML; real themes are under 10 KB): its `clrScheme` and slot patterns rescan to the end of the text for every unclosed opening tag, and a 1 MB theme of repeated `` took 21.8 s in Chromium. Number formats cap decimals at 30, as Excel does, since `toLocaleString` throws above 100. ⚠️ Merges are also capped per sheet (`LIMITS.maxMergesPerSheet`, 2,000) and across the workbook (`LIMITS.maxMerges`, 10,000), both refused as `merge-limit` before ExcelJS loads: on top of the area each merge costs, ExcelJS's `Worksheet._mergeCellsInternal` checks every new merge against every earlier one on its sheet (`_.each(this._merges)`, rebuilding `Object.keys` per call), so a sheet's cost is quadratic in its merge count (one sheet at the old 5,000 cap took 1.6 s in the worker harness, and 20 such sheets ran past the 20 s load timeout in Chromium; the new caps keep the worst case near 1.3 s). ⚠️ The styles counter reads every `` (cellXfs' and dxfs' alike; the lookahead skips ``) with `readTagAttributes()` and refuses, as `number-format`, a `formatCode` that is over 255 characters (Excel's own limit) or has a `[` after its last `]`, checked on the value as ExcelJS's XML parser hands it over (entities and character references decoded, so `[` cannot hide a `[`; ExcelJS's own backslash unescape only removes characters, so it cannot reopen a bracket). ExcelJS runs `utils.isDateFmt` once per numeric cell at load, and its `fmt.replace(/\[[^\]]*]/g, '')` rescans to the end of the code for every `[` with no later `]` (a 60,000-character code of `[` cost 2.2 s per numeric cell; even 255 unclosed characters add up at the cell cap), while a closed code is linear. The length cap also bounds the `Unsupported number format: ` notice, and the refusal messages never echo the code. ⚠️ Cell text reaching the page is bounded in the worker: `formatCellValue` caps every display string at `LIMITS.maxCellTextChars` (1,000; the last kept character is an ellipsis and a cut never splits a surrogate pair) for every value shape, strings, rich text (runs are joined only until the cap is passed), hyperlink text, formula source and results, and errors. Without it each tile cell carried its whole string and structured clone copied it once per cell, after the load timeout had already been cleared by the metadata reply: one 1 MB shared string over a 60 x 20 block left the page unresponsive for 150 s at 9.6 GB. A cell is one `nowrap` line with an ellipsis, so nothing past the column width was ever shown. A worker test asserts every tile cell's `text` is within the cap for a long shared string, rich text and a hyperlink. ⚠️ Bounding the text kept is not enough for rich text: `richTextPrefix` also visits at most `LIMITS.maxCellTextChars + 1` runs, since any that many non-empty runs already pass the cap. An empty run (``, 4 bytes) adds no text, so a loop that stopped only on length walked every run, and ExcelJS hands every cell referencing a shared string the same `richText` array, so the walk repeated per cell on every tile: one shared string of 200,000 empty runs (a 96 KB file) referenced by a 50 x 50 block took 11.2 s per tile, a million runs 56 s, with tiles requested once per animation frame while scrolling. With the bound the same tiles take about 24 ms. A core test gives a 5,000-run array whose entries past the bound throw on read and asserts `formatCellValue` returns. Notices are bounded too: the worker folds two or more `Unsupported number format` warnings in a tile into one `N unsupported number formats` entry (`foldWarnings`), and `.spreadsheet-preview-notice` has a `max-height` and scrolls, so a sheet with a distinct code per cell cannot push the grid out of view. The worker is a stable URL cache-busted by `SPREADSHEET_ASSET_VERSION` in `spreadsheet-preview.js`, the content hash of the worker, the core and both vendor bundles; `npm run check:public-assets` fails when it drifts, so editing any of those (or bumping either package) means updating the token, or a deploy pairs a new worker with a year-cached old core. +**XLSX previews parse in the browser worker, and ExcelJS only ever sees what admission checked.** An `.xlsx` joins the raw routes like any other file (`?preview=true` caps it at 10 MB with a 413); the server never parses it. `spreadsheet-preview.js` hands the bytes to `spreadsheet-preview-worker.js`, the ONLY place the pinned `exceljs`/`fflate` vendor bundles load: fflate and `spreadsheet-xlsx-core.js` at worker start, ExcelJS only after `admitXlsx()` passes, and the page itself never loads either. `admitXlsx()` streams every entry through fflate and enforces the entry count, per-entry and total inflated bytes (64 MB), compression ratio, worksheet, cell, merge and style caps on the actual inflated output, refusing (never truncating) a workbook that trips one. ⚠️ Admission walks LOCAL headers while ExcelJS (JSZip) reads the CENTRAL directory, so overlapping entries (a stored entry hiding a whole `sheet1.xml`, a one-cell decoy later in the stream) once let a file be admitted as 1 cell and parsed as 300k. The worker therefore never hands ExcelJS the fetched bytes: it gets `buildAdmittedArchive()`, a STORE-only `fflate.zipSync(entries, { level: 0 })` of exactly the entries admission inflated, and a name streamed twice is refused. That costs one transient copy of the inflated entries (bounded by the same 64 MB cap) and is pinned by the overlapping-entry fixture in `test/spreadsheet-preview-worker.test.ts`. ⚠️ Admission also has to bound what ExcelJS EXPANDS, not just what it reads: ExcelJS 4.4.0 builds one object per covered cell of a ``, per address of a `` and per index up to ``, so a 6.5 KB file admitted as one cell once cost a gigabyte. The counter therefore charges each merge its full AREA against the cell caps (an unparseable `ref` is refused), refuses a `` whose `min`/`max` is past 16384, and the worker loads with `ignoreNodes: ['dataValidations']` (the preview never shows validations). Defined names expand the same way and live in `xl/workbook.xml`, where admission reads only the `` ids: ExcelJS's `DefinedNames` model setter builds one object per cell of every range (a whole-sheet name exhausted a 4 GB heap), so right after `new ExcelJS.Workbook()` the worker replaces `_definedNames.model` with an `Object.defineProperty` stub whose getter returns `[]` and whose setter drops the value. Print areas and titles are split off in the workbook xform's reconcile, before that setter runs, so they are unaffected, and `defineProperty` throws if a future ExcelJS renames `_definedNames` rather than silently expanding again. ⚠️ Every name admission uses is the one ExcelJS will SEE, not the stored one: JSZip resolves each entry name on load (`utils.resolve`: `.` and empty middle segments dropped, `..` pops a segment) and ExcelJS then strips one leading `/` and matches worksheets with an UNANCHORED `xl/worksheets/sheet.xml`. Checking the stored name once let `/xl/worksheets/sheet1.xml`, `xl/./...`, `xl//...`, `xl/xl/worksheets/sheet1.xml` and `xl/worksheets/sheet1.xml.x` skip every counter. `excelJsEntryName()` mirrors both steps; admission refuses two entries that resolve to one name, keys `inflatedEntries` (so the rebuilt archive) on the resolved name, picks the worksheet and styles counters from it, and counts any name matching ExcelJS's unanchored worksheet pattern as a worksheet. The local-versus-central consistency checks still compare the stored names. The counter reads ``/`` attributes IN ORDER from the tag name with a sticky regex that consumes each quoted value whole (a raw `>` or the other quote character is legal inside a value, so a first-match search could be fed a fake `ref`/`max`), and refuses a tag whose attributes do not parse up to `>` or repeat a name. It counts every `` (ExcelJS keeps one Row object per element, cells or not) against per-sheet and total row caps, and reads each ``'s attributes the same way. ⚠️ The INDEX a row or sheet claims is a cost too, not just how many there are: ExcelJS stores a row at `_rows[r - 1]` and walks `_rows` up to the largest index on every `eachRow` and `sheet.model` (five rows plus one `` took 5 s to load and 1.7 s per tile), and stores a sheet at `_worksheets[sheetId]`, which its `worksheets` getter slices and sorts (`sheetId="30000000"` on a one-cell workbook took 1.6 s and 557 MB). So a `` that is not plain digits in 1 to 1,048,576 is refused (an absent `r` is fine), and a counter for the resolved `xl/workbook.xml` reads every `` tag in order and refuses one that does not parse or whose `sheetId` is not plain digits up to `LIMITS.maxSheetId` (65535; real ids are small, and the `(?=[\s/>])` lookahead keeps the `` container out). The counter also counts every `` in styles.xml with no list-tracking state (a `` inside a comment used to desync it). It carries everything from the last `<` into the next inflate chunk, since `<` can never appear inside an attribute value, and a central-directory `compressedSize` that runs past the file is refused, since the ratio cap divides by it. Fixtures for each are in `test/spreadsheet-preview-worker.test.ts` and `test/spreadsheet-xlsx-core.test.ts`. ⚠️ `sendTile` must never read `sheet.model`, which rebuilds every row and cell model (10-19 ms per tile on a 100k-cell sheet, once per animation frame while scrolling): each sheet's merges are read once in `worksheetMetadata` and kept in `mergesById` next to `populatedRowsById` (replaced on load, cleared on dispose), and a worker test makes the `model` getter throw before requesting a tile. ⚠️ The COLUMN index a cell claims costs the same way: ExcelJS keeps a row's cells at `_cells[col - 1]`, so a row whose only cell sits in `XFD` is a dictionary-mode array, and ExcelJS's `row.eachCell` (`_cells.forEach`) and `row.hasValues` (which `sheet.eachRow` calls) visit every index up to 16,384, about 0.5 ms per row (10,000 such rows took 13 s to load in Chromium, and each tile paid it again; `XFD` is a legal column, so admission cannot refuse it). `worksheetMetadata` therefore never calls ExcelJS's dense `eachRow`/`eachCell`: `populatedRowIndex()` walks `Object.keys(sheet._rows)` and, per row, `Object.keys(row._cells)`, sorted numerically, skipping falsy and `ValueType.Null` cells exactly as `eachCell({ includeEmpty: false })` does and keeping a row only when it holds one such cell (ExcelJS's `hasValues`). Styles, the extent and row heights come from that one pass, merges from `sheet._merges` (the map the `model` getter itself lists), and the per-row cell lists are what `populatedRowsById` keeps, so `sendTile` reads a row's cells from the index instead of `row.eachCell`. Worker tests make `eachRow`, `eachCell`, `hasValues` and the `model` getter throw across load and a tile, and load and tile 3,000 one-`XFD`-cell rows. `parseThemePalette` returns the default palette for a theme above 64 KB (64 * 1024 characters of the decoded XML; real themes are under 10 KB): its `clrScheme` and slot patterns rescan to the end of the text for every unclosed opening tag, and a 1 MB theme of repeated `` took 21.8 s in Chromium. Number formats cap decimals at 30, as Excel does, since `toLocaleString` throws above 100. ⚠️ Merges are also capped per sheet (`LIMITS.maxMergesPerSheet`, 2,000) and across the workbook (`LIMITS.maxMerges`, 10,000), both refused as `merge-limit` before ExcelJS loads: on top of the area each merge costs, ExcelJS's `Worksheet._mergeCellsInternal` checks every new merge against every earlier one on its sheet (`_.each(this._merges)`, rebuilding `Object.keys` per call), so a sheet's cost is quadratic in its merge count (one sheet at the old 5,000 cap took 1.6 s in the worker harness, and 20 such sheets ran past the 20 s load timeout in Chromium; the new caps keep the worst case near 1.3 s). ⚠️ The styles counter reads every `` (cellXfs' and dxfs' alike; the lookahead skips ``) with `readTagAttributes()` and refuses, as `number-format`, a `formatCode` that is over 255 characters (Excel's own limit) or has a `[` after its last `]`, checked on the value as ExcelJS's XML parser hands it over (entities and character references decoded, so `[` cannot hide a `[`; ExcelJS's own backslash unescape only removes characters, so it cannot reopen a bracket). ExcelJS runs `utils.isDateFmt` once per numeric cell at load, and its `fmt.replace(/\[[^\]]*]/g, '')` rescans to the end of the code for every `[` with no later `]` (a 60,000-character code of `[` cost 2.2 s per numeric cell; even 255 unclosed characters add up at the cell cap), while a closed code is linear. The length cap also bounds the `Unsupported number format: ` notice, and the refusal messages never echo the code. ⚠️ Cell text reaching the page is bounded in the worker: `formatCellValue` caps every display string at `LIMITS.maxCellTextChars` (1,000; the last kept character is an ellipsis and a cut never splits a surrogate pair) for every value shape, strings, rich text (runs are joined only until the cap is passed), hyperlink text, formula source and results, and errors. Without it each tile cell carried its whole string and structured clone copied it once per cell, after the load timeout had already been cleared by the metadata reply: one 1 MB shared string over a 60 x 20 block left the page unresponsive for 150 s at 9.6 GB. A cell is one `nowrap` line with an ellipsis, so nothing past the column width was ever shown. A worker test asserts every tile cell's `text` is within the cap for a long shared string, rich text and a hyperlink. ⚠️ Bounding the text kept is not enough for rich text: `richTextPrefix` also visits at most `LIMITS.maxCellTextChars + 1` runs, since any that many non-empty runs already pass the cap. An empty run (``, 4 bytes) adds no text, so a loop that stopped only on length walked every run, and ExcelJS hands every cell referencing a shared string the same `richText` array, so the walk repeated per cell on every tile: one shared string of 200,000 empty runs (a 96 KB file) referenced by a 50 x 50 block took 11.2 s per tile, a million runs 56 s, with tiles requested once per animation frame while scrolling. With the bound the same tiles take about 24 ms. A core test gives a 5,000-run array whose entries past the bound throw on read and asserts `formatCellValue` returns. ⚠️ Per-tag counters can never be complete, so admission also budgets the elements themselves: `createXmlCounter` counts every start tag (`<` followed by a letter or `_`, on the same chunk-carried text the other counters read) in EVERY entry except the ones ExcelJS keeps as raw bytes, exactly the resolved names matching `^xl/media/.$` (ExcelJS's own patterns are unanchored, so `xl/media/xl/drawings/a.xml` is still parsed as a drawing and is counted), into `counts.elements`, and refuses above `LIMITS.maxElements` (2,000,000) as `element-limit`. ExcelJS builds one object per element in every part it parses: each `` run and `` in `xl/sharedStrings.xml`, each ``, `` and `` in styles, comments, drawings, VML and tables, which only the inflated-byte caps bounded, and the 100:1 ratio cap is no help because empty stored deflate blocks (5 bytes, inflating to nothing) pad a stream to any ratio. One shared string of 8.3M empty `` runs (a 375 KB file) took 4.7 s and about 570 MB of heap; 4.6M `` in styles about 9 s and 1 GB. A three-sheet 249k-cell workbook with rich text has 589k to 839k elements, well inside the budget. A tag inside an XML comment is counted too, which errs toward refusing. Parts that read no attributes (everything but worksheets, `xl/styles.xml` and `xl/workbook.xml`) carry only a trailing `<` between chunks, so a long text node or a binary part is never mistaken for an oversized tag. A worker fixture of 2,000,001 empty runs, padded past the ratio cap and proven to pass every other cap, is refused before ExcelJS is imported. Notices are bounded too: the worker folds two or more `Unsupported number format` warnings in a tile into one `N unsupported number formats` entry (`foldWarnings`), and `.spreadsheet-preview-notice` has a `max-height` and scrolls, so a sheet with a distinct code per cell cannot push the grid out of view. The worker is a stable URL cache-busted by `SPREADSHEET_ASSET_VERSION` in `spreadsheet-preview.js`, the content hash of the worker, the core and both vendor bundles; `npm run check:public-assets` fails when it drifts, so editing any of those (or bumping either package) means updating the token, or a deploy pairs a new worker with a year-cached old core. ### Filesystem path picker diff --git a/src/web/public/spreadsheet-preview.js b/src/web/public/spreadsheet-preview.js index 95f06e71..2930c121 100644 --- a/src/web/public/spreadsheet-preview.js +++ b/src/web/public/spreadsheet-preview.js @@ -20,7 +20,7 @@ (function initSpreadsheetPreview(global) { 'use strict'; - const SPREADSHEET_ASSET_VERSION = '4d543b11c25c'; + const SPREADSHEET_ASSET_VERSION = '911680fac09d'; const MAX_PREVIEW_BYTES = 10 * 1024 * 1024; const DEFAULT_TIMEOUT_MS = 20000; const MAX_SCROLL_PX = 8000000; @@ -62,9 +62,8 @@ let headingsLayer = null; let emptySheetState = null; let resizeObserver = null; - let scaleX = 1; - let scaleY = 1; let latestRange = null; + let latestAxes = null; let scrollFrame = null; const current = () => !disposed && isCurrent(); @@ -126,17 +125,37 @@ return low; } + // Past MAX_SCROLL_PX the spacer is shorter than the sheet, so only the + // scroll POSITION is scaled (the scroll range maps onto the sheet's whole + // range, so the last row stays reachable) and the tile is laid out at real + // sizes from there. `shift` is the logical offset minus the scroll offset, + // 0 when the sheet fits; `end` is the bottom (or right) of the spacer. + function scrollAxis(logical, scroll, viewport, heading) { + const shown = Math.min(MAX_SCROLL_PX, logical); + const scrollRange = Math.max(0, heading + shown - viewport); + const logicalRange = Math.max(0, heading + logical - viewport); + const virtual = + logical > shown && scrollRange > 0 ? Math.min(logicalRange, (scroll / scrollRange) * logicalRange) : scroll; + return { virtual, shift: virtual - scroll, end: heading + shown }; + } + function requestTile() { if (!current() || !worker || !grid) return; const sheet = sheetMetadata(); if (!sheet || sheet.rows === 0 || sheet.cols === 0) return; - scaleY = Math.max( - 1, - axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, sheet.rows + 1) / MAX_SCROLL_PX + const viewHeight = grid.clientHeight || 500; + const viewWidth = grid.clientWidth || 800; + const y = scrollAxis( + axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, sheet.rows + 1), + grid.scrollTop, + viewHeight, + COLUMN_HEADING_HEIGHT ); - scaleX = Math.max( - 1, - axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, sheet.cols + 1) / MAX_SCROLL_PX + const x = scrollAxis( + axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, sheet.cols + 1), + grid.scrollLeft, + viewWidth, + ROW_HEADING_WIDTH ); const r1 = Math.max( 1, @@ -144,7 +163,7 @@ sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, - Math.max(0, grid.scrollTop - COLUMN_HEADING_HEIGHT) * scaleY + Math.max(0, y.virtual - COLUMN_HEADING_HEIGHT) ) - 2 ); const c1 = Math.max( @@ -153,7 +172,7 @@ sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, - Math.max(0, grid.scrollLeft - ROW_HEADING_WIDTH) * scaleX + Math.max(0, x.virtual - ROW_HEADING_WIDTH) ) - 2 ); const r2 = Math.min( @@ -162,7 +181,7 @@ sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, - Math.max(0, grid.scrollTop - COLUMN_HEADING_HEIGHT + (grid.clientHeight || 500)) * scaleY + Math.max(0, y.virtual - COLUMN_HEADING_HEIGHT + viewHeight) ) + 2 ); const c2 = Math.min( @@ -171,11 +190,12 @@ sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, - Math.max(0, grid.scrollLeft - ROW_HEADING_WIDTH + (grid.clientWidth || 800)) * scaleX + Math.max(0, x.virtual - ROW_HEADING_WIDTH + viewWidth) ) + 2 ); latestRequestId += 1; latestRange = { r1, c1, r2, c2 }; + latestAxes = { y, x }; worker.postMessage({ type: 'tile', requestId: latestRequestId, @@ -207,7 +227,16 @@ function renderTile(tile) { if (!current() || tile.requestId !== latestRequestId || String(tile.sheetId) !== String(activeSheetId)) return; const sheet = sheetMetadata(); - if (!sheet || !cellsLayer || !headingsLayer || !latestRange) return; + if (!sheet || !cellsLayer || !headingsLayer || !latestRange || !latestAxes) return; + const { y, x } = latestAxes; + // Sizes are real; a span (a tall merge) is clipped at the spacer's edge so + // it never grows the scroll area. + const rowTop = (row) => + COLUMN_HEADING_HEIGHT + axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, row) - y.shift; + const colLeft = (col) => + ROW_HEADING_WIDTH + axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, col) - x.shift; + const rowSpan = (from, to) => Math.max(0, Math.min(rowTop(to + 1), y.end) - rowTop(from)); + const colSpan = (from, to) => Math.max(0, Math.min(colLeft(to + 1), x.end) - colLeft(from)); cellsLayer.textContent = ''; headingsLayer.textContent = ''; const mergeByAnchor = new Map(); @@ -227,13 +256,11 @@ element.dataset.row = String(cell.row); element.dataset.col = String(cell.col); element.textContent = String(cell.text ?? ''); - element.style.top = `${COLUMN_HEADING_HEIGHT + axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, cell.row) / scaleY}px`; - element.style.left = `${ROW_HEADING_WIDTH + axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, cell.col) / scaleX}px`; + element.style.top = `${rowTop(cell.row)}px`; + element.style.left = `${colLeft(cell.col)}px`; const merge = mergeByAnchor.get(`${cell.row}:${cell.col}`); - const finalRow = merge?.r2 || cell.row; - const finalCol = merge?.c2 || cell.col; - element.style.height = `${Math.max(0, (axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, finalRow + 1) - axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, cell.row)) / scaleY)}px`; - element.style.width = `${Math.max(0, (axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, finalCol + 1) - axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, cell.col)) / scaleX)}px`; + element.style.height = `${rowSpan(cell.row, merge?.r2 || cell.row)}px`; + element.style.width = `${colSpan(cell.col, merge?.c2 || cell.col)}px`; cellsLayer.appendChild(element); } // Headings take their size from the same axis math as the cells, so custom @@ -241,23 +268,20 @@ // do not count against the heading caps. let rowHeadings = 0; for (let row = latestRange.r1; row <= latestRange.r2 && rowHeadings < 200; row += 1) { - const top = axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, row); - const height = (axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, row + 1) - top) / scaleY; + const height = rowSpan(row, row); if (height <= 0) continue; rowHeadings += 1; const heading = document.createElement('div'); heading.className = 'spreadsheet-row-heading'; heading.textContent = String(row); - heading.style.top = `${COLUMN_HEADING_HEIGHT + top / scaleY}px`; + heading.style.top = `${rowTop(row)}px`; heading.style.height = `${height}px`; heading.style.left = `${grid.scrollLeft}px`; headingsLayer.appendChild(heading); } let columnHeadings = 0; for (let col = latestRange.c1; col <= latestRange.c2 && columnHeadings < 100; col += 1) { - const left = axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, col); - const width = - (axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, col + 1) - left) / scaleX; + const width = colSpan(col, col); if (width <= 0) continue; columnHeadings += 1; const heading = document.createElement('div'); @@ -266,7 +290,7 @@ for (let value = col; value > 0; value = Math.floor((value - 1) / 26)) label = String.fromCharCode(65 + ((value - 1) % 26)) + label; heading.textContent = label; - heading.style.left = `${ROW_HEADING_WIDTH + left / scaleX}px`; + heading.style.left = `${colLeft(col)}px`; heading.style.width = `${width}px`; heading.style.top = `${grid.scrollTop}px`; headingsLayer.appendChild(heading); @@ -279,6 +303,7 @@ activeSheetId = String(sheetId); latestRequestId += 1; latestRange = null; + latestAxes = null; if (cellsLayer) cellsLayer.textContent = ''; if (headingsLayer) headingsLayer.textContent = ''; container.querySelectorAll('[role="tab"]').forEach((tab) => { @@ -294,8 +319,6 @@ if (sheet && spacer) { const logicalHeight = axisOffset(sheet.rows, sheet.defaultRowHeight, sheet.rowOverrides, sheet.rows + 1); const logicalWidth = axisOffset(sheet.cols, sheet.defaultColumnWidth, sheet.columnOverrides, sheet.cols + 1); - scaleY = Math.max(1, logicalHeight / MAX_SCROLL_PX); - scaleX = Math.max(1, logicalWidth / MAX_SCROLL_PX); spacer.style.height = `${COLUMN_HEADING_HEIGHT + Math.min(MAX_SCROLL_PX, logicalHeight)}px`; spacer.style.width = `${ROW_HEADING_WIDTH + Math.min(MAX_SCROLL_PX, logicalWidth)}px`; } diff --git a/src/web/public/spreadsheet-xlsx-core.js b/src/web/public/spreadsheet-xlsx-core.js index dabe5d4f..d1e052c9 100644 --- a/src/web/public/spreadsheet-xlsx-core.js +++ b/src/web/public/spreadsheet-xlsx-core.js @@ -45,6 +45,12 @@ // its whole string to the page (a 1 MB shared string over a 60 x 20 block // froze the page's main thread at 9.6 GB). maxCellTextChars: 1000, + // Start tags across every part ExcelJS parses (all but xl/media). ExcelJS + // builds an object per element, and outside worksheets nothing else bounded + // them: one shared string of 8.3M empty `` runs (a 375 KB file, padded + // past the ratio cap with empty stored blocks) cost 570 MB of heap. A + // three-sheet 249k-cell workbook with rich text has about 589k. + maxElements: 2000000, }); const MAX_ROW = 1048576; const MAX_COL = 16384; @@ -281,6 +287,19 @@ } } + // Exactly the names ExcelJS hands to `_processMediaEntry` and to no parser: + // its other patterns are unanchored, so `xl/media/xl/drawings/a.xml` is still + // parsed as a drawing, and only a single `.` segment is pure bytes. + const EXCELJS_MEDIA = /^xl\/media\/[a-zA-Z0-9]+[.][a-zA-Z0-9]{3,4}$/; + const START_TAG = /<[A-Za-z_]/g; + + function countStartTags(text) { + let count = 0; + START_TAG.lastIndex = 0; + while (START_TAG.exec(text)) count += 1; + return count; + } + function createXmlCounter(name, counts, limits) { let tail = ''; const decoder = new TextDecoder(); @@ -291,6 +310,9 @@ const worksheet = isWorksheetName(excelJsName); const styles = excelJsName === 'xl/styles.xml'; const workbook = excelJsName === 'xl/workbook.xml'; + const media = EXCELJS_MEDIA.test(excelJsName); + // Only these parts read tag attributes, so only they carry a whole tag. + const readsTags = worksheet || styles || workbook; const addCells = (cells) => { sheetCells += cells; counts.cells += cells; @@ -299,13 +321,19 @@ }; return { push(chunk, final) { - if (!worksheet && !styles && !workbook) return; + if (media) return; const text = tail + decoder.decode(chunk, { stream: !final }); // `<` can never appear inside an attribute value, so every tag before the // last `<` is complete. Carry everything from that `<` into the next scan - // so a tag cut by a chunk edge is always read in one piece. - const safeEnd = final ? text.length : Math.max(0, text.lastIndexOf('<')); + // so a tag cut by a chunk edge is always read in one piece. Any other part + // only needs its start tags counted, so it carries at most a trailing `<` + // (a long text node or a binary part is never mistaken for a huge tag). + let safeEnd = text.length; + if (!final && readsTags) safeEnd = Math.max(0, text.lastIndexOf('<')); + else if (!final && text.endsWith('<')) safeEnd = text.length - 1; const scan = text.slice(0, safeEnd); + counts.elements += countStartTags(scan); + if (counts.elements > limits.maxElements) fail('element-limit', 'Workbook exceeds the XML elements limit'); if (worksheet) { addCells((scan.match(/)/g) || []).length); // ExcelJS keeps a Row object for every , with or without cells. @@ -366,7 +394,7 @@ for (const entry of directory.entries) expectedEntries.set(entry.name, (expectedEntries.get(entry.name) || 0) + 1); const streamedEntries = new Map(); const inflatedEntries = Object.create(null); - const counts = { worksheets: 0, cells: 0, rows: 0, merges: 0, styles: 0 }; + const counts = { worksheets: 0, cells: 0, rows: 0, merges: 0, styles: 0, elements: 0 }; const features = new Set(); let totalInflated = 0; let seenEntries = 0; diff --git a/test/spreadsheet-preview-worker.test.ts b/test/spreadsheet-preview-worker.test.ts index 4df7b18f..c9a87b24 100644 --- a/test/spreadsheet-preview-worker.test.ts +++ b/test/spreadsheet-preview-worker.test.ts @@ -1008,3 +1008,65 @@ describe('spreadsheet preview worker: the notice bar stays bounded', () => { expect(tile.warnings).toEqual(['800 unsupported number formats']); }, 60_000); }); + +/** + * A one-cell workbook plus an `xl/sharedStrings.xml` whose one string holds + * `runs` empty `` runs. Its deflate stream is prefixed with empty stored + * blocks (5 bytes each, inflating to nothing) until the compressed size clears + * the 100:1 ratio cap, so only an element budget can refuse it. + */ +async function emptyRunsWorkbook(runs: number): Promise { + const workbook = new ExcelJS.Workbook(); + workbook.addWorksheet('Data').getCell('A1').value = 1; + const entries = fflate.unzipSync(new Uint8Array(await workbook.xlsx.writeBuffer())); + delete entries['xl/sharedStrings.xml']; + const strings = fflate.strToU8(`${''.repeat(runs)}`); + const deflated = fflate.deflateSync(strings, { level: 9 }); + const emptyStoredBlock = Uint8Array.from([0x00, 0x00, 0x00, 0xff, 0xff]); + const blocks = Math.ceil((strings.length / 50 - deflated.length) / emptyStoredBlock.length); + const padded = concatBytes([...Array.from({ length: blocks }, () => emptyStoredBlock), deflated]); + expect(fflate.inflateSync(padded)).toEqual(strings); + const parts = Object.keys(entries).map((name) => zipPart(name, entries[name], 8)); + parts.push({ + name: 'xl/sharedStrings.xml', + data: padded, + method: 8, + size: strings.length, + crc: crc32(strings) >>> 0, + }); + const locals: Uint8Array[] = []; + const central: Uint8Array[] = []; + let offset = 0; + for (const part of parts) { + central.push(centralHeader(part, offset)); + const chunk = concatBytes([localHeader(part), part.data]); + locals.push(chunk); + offset += chunk.length; + } + const directory = concatBytes(central); + const eocd = new Uint8Array(22); + const view = new DataView(eocd.buffer); + view.setUint32(0, 0x06054b50, true); + view.setUint16(8, central.length, true); + view.setUint16(10, central.length, true); + view.setUint32(12, directory.length, true); + view.setUint32(16, offset, true); + return concatBytes([...locals, directory, eocd]); +} + +describe('spreadsheet preview worker: element budget', () => { + // ExcelJS builds one object per run in sharedStrings.xml, which no + // worksheet counter sees; 8.3M empty runs took 570 MB of heap to load. + it('refuses a shared string of 2,000,001 empty runs before ExcelJS loads', async () => { + const crafted = await emptyRunsWorkbook(2_000_001); + const harness = createHarness(); + const core = harness.self.CodemanSpreadsheetXlsxCore as { + admitXlsx(bytes: Uint8Array, zip: typeof fflate, overrides?: Record): unknown; + }; + // Every other cap admits it: only the element budget is in the way. + expect(() => core.admitXlsx(crafted, fflate, { maxElements: Infinity })).not.toThrow(); + await harness.send({ type: 'load', bytes: toArrayBuffer(crafted) }); + expect(harness.messages.at(-1)).toMatchObject({ type: 'error', code: 'element-limit' }); + expect(harness.imports.some((url) => url.includes('exceljs'))).toBe(false); + }, 60_000); +}); diff --git a/test/spreadsheet-preview.test.ts b/test/spreadsheet-preview.test.ts index 24263730..45961c5d 100644 --- a/test/spreadsheet-preview.test.ts +++ b/test/spreadsheet-preview.test.ts @@ -291,6 +291,81 @@ describe('spreadsheet preview renderer', () => { expect(heading('.spreadsheet-row-heading', '3')).toBeUndefined(); }); + // Past MAX_SCROLL_PX only the scroll position may be scaled: dividing every + // cell and heading by the scale drew a 1,048,576-row sheet 7.6 px a row. + it('lays rows out at their real height on a sheet taller than the scroll cap', async () => { + const fetchMock = vi.fn(async () => ({ ok: true, arrayBuffer: async () => new ArrayBuffer(8) })); + const renderer = loadRenderer(fetchMock); + renderer.open({ container: document.querySelector('#preview'), url: '/book.xlsx', size: 8 }); + const worker = WorkerMock.instances[0]; + worker.emit({ type: 'ready' }); + await vi.waitFor(() => expect(worker.postMessage).toHaveBeenCalled()); + const sheet = { id: '1', name: 'Tall', rows: 1_048_576, cols: 1, defaultRowHeight: 20, defaultColumnWidth: 64 }; + worker.emit({ type: 'metadata', styles: [], sheets: [{ ...sheet, rowOverrides: [], columnOverrides: [] }] }); + const cellAt = (row: number) => document.querySelector(`.spreadsheet-cell[data-row="${row}"]`) as HTMLElement; + const rowHeading = (row: number) => + [...document.querySelectorAll('.spreadsheet-row-heading')].find( + (element) => element.textContent === String(row) + ) as HTMLElement; + const px = (value: string) => Number.parseFloat(value); + + const first = worker.postMessage.mock.calls.at(-1)?.[0]; + expect(first.range.r1).toBe(1); + const cell = (row: number) => ({ row, col: 1, text: `A${row}`, styleId: 0 }); + worker.emit({ type: 'tile', requestId: first.requestId, sheetId: '1', cells: [cell(1), cell(2)], warnings: [] }); + expect(cellAt(1).style.top).toBe('20px'); + expect(cellAt(1).style.height).toBe('20px'); + expect(px(cellAt(2).style.top) - px(cellAt(1).style.top)).toBe(20); + expect(rowHeading(2).style.height).toBe('20px'); + // At scale 1 the first tile asks for about a viewport of rows, not a scaled one. + expect(first.range.r2).toBeLessThan(40); + + // Scrolled to the end, the last row is requested, drawn at its real height, + // and ends exactly at the bottom of the scroll area. + const grid = document.querySelector('.spreadsheet-grid') as HTMLElement; + const spacerHeight = px((document.querySelector('.spreadsheet-grid-spacer') as HTMLElement).style.height); + grid.scrollTop = spacerHeight - 500; + grid.dispatchEvent(new window.Event('scroll')); + await vi.waitFor(() => + expect(worker.postMessage.mock.calls.at(-1)?.[0].requestId).toBeGreaterThan(first.requestId) + ); + const last = worker.postMessage.mock.calls.at(-1)?.[0]; + expect(last.range.r2).toBe(1_048_576); + expect(last.range.r2 - last.range.r1).toBeLessThan(40); + worker.emit({ type: 'tile', requestId: last.requestId, sheetId: '1', cells: [cell(1_048_576)], warnings: [] }); + expect(cellAt(1_048_576).style.height).toBe('20px'); + expect(rowHeading(1_048_576).style.height).toBe('20px'); + expect(px(cellAt(1_048_576).style.top) + 20).toBeCloseTo(spacerHeight, 6); + expect(px(rowHeading(1_048_575).style.top)).toBeCloseTo(px(cellAt(1_048_576).style.top) - 20, 6); + }); + + it('keeps a merge spanning a too-tall sheet inside the scroll area', async () => { + const fetchMock = vi.fn(async () => ({ ok: true, arrayBuffer: async () => new ArrayBuffer(8) })); + const renderer = loadRenderer(fetchMock); + renderer.open({ container: document.querySelector('#preview'), url: '/book.xlsx', size: 8 }); + const worker = WorkerMock.instances[0]; + worker.emit({ type: 'ready' }); + await vi.waitFor(() => expect(worker.postMessage).toHaveBeenCalled()); + const sheet = { id: '1', name: 'Tall', rows: 1_048_576, cols: 1, defaultRowHeight: 20, defaultColumnWidth: 64 }; + worker.emit({ type: 'metadata', styles: [], sheets: [{ ...sheet, rowOverrides: [], columnOverrides: [] }] }); + const request = worker.postMessage.mock.calls.at(-1)?.[0]; + worker.emit({ + type: 'tile', + requestId: request.requestId, + sheetId: '1', + cells: [{ row: 1, col: 1, text: 'whole column', styleId: 0 }], + merges: ['A1:A1048576'], + warnings: [], + }); + const merged = document.querySelector('.spreadsheet-cell') as HTMLElement; + const spacerHeight = Number.parseFloat( + (document.querySelector('.spreadsheet-grid-spacer') as HTMLElement).style.height + ); + expect(Number.parseFloat(merged.style.top) + Number.parseFloat(merged.style.height)).toBeLessThanOrEqual( + spacerHeight + ); + }); + it('emits colour and background together or not at all', async () => { const fetchMock = vi.fn(async () => ({ ok: true, arrayBuffer: async () => new ArrayBuffer(8) })); const renderer = loadRenderer(fetchMock); diff --git a/test/spreadsheet-xlsx-core.test.ts b/test/spreadsheet-xlsx-core.test.ts index 980d6741..41b9ad13 100644 --- a/test/spreadsheet-xlsx-core.test.ts +++ b/test/spreadsheet-xlsx-core.test.ts @@ -115,7 +115,8 @@ describe('spreadsheet XLSX core', () => { features: string[]; }; // The 2x2 merge costs its four covered cells on top of the one real cell. - expect(result.counts).toEqual({ worksheets: 1, cells: 5, rows: 0, merges: 1, styles: 1 }); + // `elements` is every start tag across all six parts: 1 + 3 + 3 + 4 + 1 + 1. + expect(result.counts).toEqual({ worksheets: 1, cells: 5, rows: 0, merges: 1, styles: 1, elements: 13 }); expect(result.features).toEqual(expect.arrayContaining(['charts', 'externalLinks'])); }); @@ -327,6 +328,91 @@ describe('spreadsheet XLSX core', () => { expect(() => other.push(fflate.strToU8(one('30000000')), true)).not.toThrow(); }); + // ExcelJS builds an object per element in every part it parses, not only + // worksheets: each run and in sharedStrings.xml, each , + // and in styles, comments, drawings, VML and tables. + it('budgets every start tag outside xl/media, whatever part it sits in', () => { + expect(core.LIMITS.maxElements).toBe(2_000_000); + const withPart = (name: string, xml: string) => + fflate.zipSync({ ...fflate.unzipSync(workbookZip()), [name]: fflate.strToU8(xml) }); + const elementsOf = (zip: Uint8Array) => + (core.admitXlsx(zip, fflate) as { counts: { elements: number } }).counts.elements; + const base = elementsOf(workbookZip()); + const runs = '' + ''.repeat(50) + ''; + const fonts = '' + ''.repeat(50) + ''; + const cases: Array<[string, string, number]> = [ + ['xl/sharedStrings.xml', runs, 52], + ['xl/comments1.xml', '' + ''.repeat(50) + '', 51], + ['xl/drawings/vmlDrawing1.vml', '' + ''.repeat(50) + '', 51], + // The name ExcelJS sees, so `/xl/./` cannot slip a part past the budget. + ['/xl/./sharedStrings.xml', runs, 52], + ]; + for (const [name, xml, added] of cases) { + expect(elementsOf(withPart(name, xml)), name).toBe(base + added); + let thrown: unknown; + try { + core.admitXlsx(withPart(name, xml), fflate, { maxElements: base + added - 1 }); + } catch (error) { + thrown = error; + } + expect((thrown as { code?: string })?.code, name).toBe('element-limit'); + } + // styles.xml is counted on top of its own / checks. + const styled = fflate.unzipSync(workbookZip()); + styled['xl/styles.xml'] = fflate.strToU8(fonts); + expect(() => core.admitXlsx(fflate.zipSync(styled), fflate, { maxElements: 50 })).toThrowError(/element/i); + // Processing instructions, comments and end tags are not elements; a tag + // INSIDE a comment is still counted, which errs toward refusing. + expect(elementsOf(withPart('xl/other.xml', '<_b/>'))).toBe(base + 2); + expect(elementsOf(withPart('xl/other.xml', ''))).toBe(base + 2); + }); + + it('never budgets an xl/media part, which ExcelJS keeps as bytes', () => { + // Stored, so the compression-ratio cap stays out of the way. + const tags = fflate.strToU8(''.repeat(500)); + for (const name of ['xl/media/image1.png', '/xl/./media/image1.jpeg']) { + const zip = fflate.zipSync({ ...fflate.unzipSync(workbookZip()), [name]: [tags, { level: 0 }] }); + expect(() => core.admitXlsx(zip, fflate, { maxElements: 100 }), name).not.toThrow(); + } + // A name under xl/media/ that ExcelJS parses as XML (its patterns are + // unanchored) is budgeted like any other part. + for (const name of ['xl/media/xl/drawings/drawing1.xml', 'xl/media/xl/worksheets/sheet2.xml']) { + const zip = fflate.zipSync({ ...fflate.unzipSync(workbookZip()), [name]: [tags, { level: 0 }] }); + expect(() => core.admitXlsx(zip, fflate, { maxElements: 100 }), name).toThrowError(/element/i); + } + }); + + it('counts start tags exactly when a chunk boundary cuts through one', () => { + const xml = '' + 'x'.repeat(20) + ''; + const bytes = fflate.strToU8(xml); + for (const name of ['xl/sharedStrings.xml', 'xl/worksheets/sheet1.xml', 'xl/styles.xml']) { + for (let cut = 1; cut < bytes.length; cut += 1) { + const counts = { worksheets: 0, cells: 0, rows: 0, merges: 0, styles: 0, elements: 0 }; + const counter = core.createXmlCounter(name, counts as never, core.LIMITS); + counter.push(bytes.subarray(0, cut), false); + counter.push(bytes.subarray(cut), true); + expect(counts.elements, `${name} cut at ${cut}`).toBe(42); + } + } + }); + + it('does not carry long text or binary in a non-worksheet part as an oversized tag', () => { + const counts = { worksheets: 0, cells: 0, rows: 0, merges: 0, styles: 0, elements: 0 }; + const text = fflate.strToU8('' + 'x'.repeat(600_000) + ''); + const strings = core.createXmlCounter('xl/sharedStrings.xml', counts as never, core.LIMITS); + const binary = core.createXmlCounter('xl/embeddings/oleObject1.bin', counts as never, core.LIMITS); + const zeros = new Uint8Array(600_000); + expect(() => { + for (let at = 0; at < text.length; at += 65_536) { + strings.push(text.subarray(at, at + 65_536), at + 65_536 >= text.length); + } + for (let at = 0; at < zeros.length; at += 65_536) { + binary.push(zeros.subarray(at, at + 65_536), at + 65_536 >= zeros.length); + } + }).not.toThrow(); + expect(counts.elements).toBe(3); + }); + it('counts every in styles.xml, so a inside a comment cannot hide styles', () => { const counts = { cells: 0, merges: 0, styles: 0, rows: 0 }; const counter = core.createXmlCounter('xl/styles.xml', counts as never, { ...core.LIMITS, maxStyles: 100 });