From 9e8a851a2ca6aace12b653df9984280841544aa2 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 13 Oct 2026 17:42:52 +0200 Subject: [PATCH] fix(spreadsheet-preview): filter and index a shared worksheet rels list once (#612 merge-time fix) ExcelJS's worksheet pattern is unanchored, so several parsed worksheets can share one sheetNo and therefore one rels list. repairExcelJsModel filtered and indexed that list once per worksheet, which let a crafted admitted file run past the 20 s preview timeout. Each distinct list is now filtered and indexed once per sheetNo, and the relationship Type strings are built once. SPREADSHEET_ASSET_VERSION is refreshed for the edited core. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/spreadsheet-preview.js | 2 +- src/web/public/spreadsheet-xlsx-core.js | 41 ++++++++++++++++--------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/web/public/spreadsheet-preview.js b/src/web/public/spreadsheet-preview.js index 0f60015b..9093523e 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 = '0833a414e785'; + const SPREADSHEET_ASSET_VERSION = 'fd81c01bef7b'; const MAX_PREVIEW_BYTES = 10 * 1024 * 1024; const DEFAULT_TIMEOUT_MS = 20000; const MAX_SCROLL_PX = 8000000; diff --git a/src/web/public/spreadsheet-xlsx-core.js b/src/web/public/spreadsheet-xlsx-core.js index 8fe0e5e4..24f6b5d7 100644 --- a/src/web/public/spreadsheet-xlsx-core.js +++ b/src/web/public/spreadsheet-xlsx-core.js @@ -488,6 +488,9 @@ // preview shows none of these parts. Lookups are own-property only, so a Target // or rId such as `__proto__` cannot resolve through Object.prototype. const REL_TYPE = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/'; + const REL_COMMENTS = `${REL_TYPE}comments`; + const REL_VML_DRAWING = `${REL_TYPE}vmlDrawing`; + const REL_TABLE = `${REL_TYPE}table`; const EXCELJS_DRAWING_TARGET = /\/drawings\/([a-zA-Z0-9]+)[.][a-zA-Z]{3,4}$/; function ownValue(map, key) { @@ -529,27 +532,37 @@ for (const key of Object.keys(model.tables)) if (!isObject(model.tables[key])) delete model.tables[key]; } const worksheetRels = isObject(model.worksheetRels) ? model.worksheetRels : {}; + // Worksheets can share a sheetNo (ExcelJS's worksheet pattern is unanchored), + // and then they share one rels list: filter and index each list ONCE, or a + // long list costs once per worksheet. + const relsBySheetNo = new Map(); for (const worksheet of Array.isArray(model.worksheets) ? model.worksheets : []) { if (!isObject(worksheet)) continue; - const list = ownValue(worksheetRels, String(worksheet.sheetNo)); - const rels = Array.isArray(list) ? list : []; - const kept = rels.filter((rel) => { - if (!isObject(rel)) return false; - if (rel.Type === `${REL_TYPE}comments`) return resolveTarget(rel, model.comments, isObject); - if (rel.Type === `${REL_TYPE}vmlDrawing`) { - return resolveTarget(rel, model.vmlDrawings, (vml) => isObject(vml) && Array.isArray(vml.comments)); - } - if (rel.Type === `${REL_TYPE}table`) return resolveTarget(rel, model.tables, isObject); - return true; - }); - if (Array.isArray(list)) worksheetRels[String(worksheet.sheetNo)] = kept; - const byId = new Map(kept.map((rel) => [rel.Id, rel])); + const sheetNo = String(worksheet.sheetNo); + let byId = relsBySheetNo.get(sheetNo); + if (!byId) { + const list = ownValue(worksheetRels, sheetNo); + const rels = Array.isArray(list) ? list : []; + const kept = rels.filter((rel) => { + if (!isObject(rel)) return false; + if (rel.Type === REL_COMMENTS) return resolveTarget(rel, model.comments, isObject); + if (rel.Type === REL_VML_DRAWING) { + return resolveTarget(rel, model.vmlDrawings, (vml) => isObject(vml) && Array.isArray(vml.comments)); + } + if (rel.Type === REL_TABLE) return resolveTarget(rel, model.tables, isObject); + return true; + }); + if (Array.isArray(list)) worksheetRels[sheetNo] = kept; + byId = new Map(); + for (const rel of kept) byId.set(rel.Id, rel); + relsBySheetNo.set(sheetNo, byId); + } // ``, `` and `` are followed // through the rels unchecked, so each must name a rel that is still there. if (Array.isArray(worksheet.tables)) { worksheet.tables = worksheet.tables.filter((part) => { const rel = isObject(part) ? byId.get(part.rId) : undefined; - return Boolean(rel) && rel.Type === `${REL_TYPE}table`; + return Boolean(rel) && rel.Type === REL_TABLE; }); } if (isObject(worksheet.drawing)) {