mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-11 01:39:41 +02:00
fix(preview): bound row and sheet indices before ExcelJS, reuse merges per tile, cap format decimals
ExcelJS stores a row at _rows[r - 1] and a sheet at _worksheets[sheetId], and walks or slices those arrays up to the largest index, so the index a row or sheet claims is a cost of its own. Admission now reads each <row> tag's attributes in order and refuses an r outside 1-1048576 (absent r is fine), and a counter for the resolved xl/workbook.xml reads every <sheet> tag and refuses one that does not parse or whose sheetId is not plain digits up to LIMITS.maxSheetId (65535). sendTile no longer reads sheet.model, which rebuilt every row and cell model on each tile: the merges read in worksheetMetadata are kept in mergesById next to populatedRowsById, replaced on load and cleared on dispose. Number formats cap decimals at 30, as Excel does; toLocaleString throws a RangeError above 100 and the whole grid was replaced by the error. Docs: CLAUDE.md and architecture-invariants describe both bounds and the merge reuse. SPREADSHEET_ASSET_VERSION is recomputed for the edited worker and core.
This commit is contained in:
@@ -696,3 +696,53 @@ describe('spreadsheet preview worker: defined names', () => {
|
||||
expect(harness.peek('Object.keys(workbook.definedNames.matrixMap).length')).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('spreadsheet preview worker: indices a row or sheet claims', () => {
|
||||
// ExcelJS stores a row at `_rows[r - 1]`, and eachRow and `sheet.model` walk
|
||||
// every index up to the largest, so one far row made each tile cost seconds.
|
||||
it('refuses a <row r> past the last Excel row before ExcelJS loads', async () => {
|
||||
const rows =
|
||||
Array.from({ length: 4 }, (_, i) => `<row r="${i + 2}"><c r="A${i + 2}"><v>${i + 2}</v></c></row>`).join('') +
|
||||
'<row r="50000000"><c r="A50000000"><v>6</v></c></row>';
|
||||
const bytes = await emptyRowsWorkbook(rows);
|
||||
const harness = createHarness();
|
||||
await harness.send({ type: 'load', bytes });
|
||||
expect(harness.messages.at(-1)).toMatchObject({ type: 'error', code: 'malformed' });
|
||||
expect(harness.imports.some((url) => url.includes('exceljs'))).toBe(false);
|
||||
}, 60_000);
|
||||
|
||||
// ExcelJS stores a sheet at `_worksheets[sheetId]`; 30,000,000 took 1.6 s
|
||||
// and 557 MB on a one-cell workbook.
|
||||
it('refuses a sheetId above the cap in xl/workbook.xml before ExcelJS loads', async () => {
|
||||
const workbook = new ExcelJS.Workbook();
|
||||
workbook.addWorksheet('Data').getCell('A1').value = 'one';
|
||||
const entries = fflate.unzipSync(new Uint8Array(await workbook.xlsx.writeBuffer()));
|
||||
const book = fflate.strFromU8(entries['xl/workbook.xml']);
|
||||
const patched = book.replace('sheetId="1"', 'sheetId="30000000"');
|
||||
expect(patched).not.toBe(book);
|
||||
entries['xl/workbook.xml'] = fflate.strToU8(patched);
|
||||
const harness = createHarness();
|
||||
await harness.send({ type: 'load', bytes: toArrayBuffer(fflate.zipSync(entries)) });
|
||||
expect(harness.messages.at(-1)).toMatchObject({ type: 'error', code: 'malformed' });
|
||||
expect(harness.imports.some((url) => url.includes('exceljs'))).toBe(false);
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
describe('spreadsheet preview worker: tiles reuse the merges read at load', () => {
|
||||
// `sheet.model` rebuilds every row and cell model; a tile must not pay that.
|
||||
it('serves a tile with its merge without touching sheet.model', async () => {
|
||||
const harness = createHarness();
|
||||
const metadata = await loadMetadata(harness, await fixture());
|
||||
const sheetId = metadata.sheets[0].id;
|
||||
harness.peek(
|
||||
`Object.defineProperty(sheetsById.get(${JSON.stringify(sheetId)}), 'model', {
|
||||
configurable: true,
|
||||
get() { throw new Error('sheet.model touched'); },
|
||||
})`
|
||||
);
|
||||
const tile = await requestTile(harness, sheetId, { r1: 4, c1: 2, r2: 4, c2: 3 });
|
||||
expect(tile.type, JSON.stringify(tile)).toBe('tile');
|
||||
expect(tile.merges).toEqual(['A4:C4']);
|
||||
expect(tile.cells).toContainEqual(expect.objectContaining({ row: 4, col: 1, text: 'Merged' }));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -283,6 +283,49 @@ describe('spreadsheet XLSX core', () => {
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
// ExcelJS stores a row at `_rows[r - 1]` and walks `_rows` up to the largest
|
||||
// index on every eachRow and `sheet.model`, so the index a row CLAIMS is cost.
|
||||
it('refuses a <row r> that is not plain digits in 1-1048576 and a <row> whose attributes do not parse', () => {
|
||||
const sheet = (rowTag: string) =>
|
||||
workbookZip(`<worksheet><sheetData><row r="1"><c r="A1"/></row>${rowTag}</sheetData></worksheet>`);
|
||||
for (const r of ['50000000', '1048577', '0', '1a', '-1', '1e3', ' 2', '']) {
|
||||
expect(() => core.admitXlsx(sheet(`<row r="${r}"/>`), fflate), r).toThrowError(/row index/i);
|
||||
}
|
||||
expect(() => core.admitXlsx(sheet('<row r="2" junk/>'), fflate)).toThrowError(/do not parse/i);
|
||||
// A quoted fake `r` cannot stand in for the real one.
|
||||
expect(() => core.admitXlsx(sheet(`<row x=' r="2"' r="50000000"/>`), fflate)).toThrowError(/row index/i);
|
||||
for (const ok of ['<row r="1048576"/>', '<row/>', '<row spans="1:1"><c r="A2"/></row>']) {
|
||||
expect(() => core.admitXlsx(sheet(ok), fflate), ok).not.toThrow();
|
||||
}
|
||||
});
|
||||
|
||||
// ExcelJS stores a sheet at `_worksheets[sheetId]`, and the `worksheets`
|
||||
// getter slices and sorts that array, so a large id allocates a huge array.
|
||||
it('refuses a <sheet sheetId> in xl/workbook.xml that is not plain digits or is above the cap', () => {
|
||||
expect(core.LIMITS.maxSheetId).toBe(65535);
|
||||
const book = (sheets: string, name = 'xl/workbook.xml') => {
|
||||
const entries = fflate.unzipSync(workbookZip());
|
||||
delete entries['xl/workbook.xml'];
|
||||
entries[name] = fflate.strToU8(`<workbook><sheets>${sheets}</sheets></workbook>`);
|
||||
return fflate.zipSync(entries);
|
||||
};
|
||||
const one = (id: string) => `<sheet name="S" sheetId="${id}" r:id="rId1"/>`;
|
||||
for (const id of ['30000000', '65536', '1a', '-1', '1e3', ' 1', '']) {
|
||||
expect(() => core.admitXlsx(book(one(id)), fflate), id).toThrowError(/sheetId/i);
|
||||
}
|
||||
// Every <sheet> is read, not just the first; the name is the one ExcelJS sees.
|
||||
expect(() => core.admitXlsx(book(one('1') + one('30000000')), fflate)).toThrowError(/sheetId/i);
|
||||
expect(() => core.admitXlsx(book(one('30000000'), '/xl/./workbook.xml'), fflate)).toThrowError(/sheetId/i);
|
||||
expect(() => core.admitXlsx(book('<sheet name="S" sheetId="1" junk/>'), fflate)).toThrowError(/do not parse/i);
|
||||
expect(() => core.admitXlsx(book(`<sheet x=' sheetId="1"' sheetId="30000000"/>`), fflate)).toThrowError(/sheetId/i);
|
||||
// `<sheets>` is the container, never a sheet; an absent sheetId is harmless.
|
||||
expect(() => core.admitXlsx(book(one('1') + one('65535') + '<sheet name="S"/>'), fflate)).not.toThrow();
|
||||
// Only the workbook part is read this way.
|
||||
const counts = { cells: 0, merges: 0, styles: 0, rows: 0 };
|
||||
const other = core.createXmlCounter('xl/other.xml', counts as never, core.LIMITS);
|
||||
expect(() => other.push(fflate.strToU8(one('30000000')), true)).not.toThrow();
|
||||
});
|
||||
|
||||
it('counts every <xf> in styles.xml, so a </cellXfs> 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 });
|
||||
@@ -344,6 +387,11 @@ describe('spreadsheet XLSX core', () => {
|
||||
expect(core.formatCellValue(7, '[Red][<0]0.0')).toMatchObject({ text: '7', warning: expect.any(String) });
|
||||
expect(core.formatCellValue({ formula: 'SUM(A1:A2)' }, 'General')).toMatchObject({ text: '=SUM(A1:A2)' });
|
||||
});
|
||||
|
||||
// toLocaleString throws a RangeError above 100 fraction digits; Excel caps at 30.
|
||||
it('caps a number format at 30 decimals instead of throwing', () => {
|
||||
expect(core.formatCellValue(1.5, `0.${'0'.repeat(120)}`).text).toBe(`1.5${'0'.repeat(29)}`);
|
||||
});
|
||||
});
|
||||
|
||||
const themeXml = `<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
|
||||
|
||||
Reference in New Issue
Block a user