From e6f42db3c36191695c743415368312339e636eb9 Mon Sep 17 00:00:00 2001 From: Bob Senoff Date: Thu, 30 Apr 2026 20:42:13 -0500 Subject: [PATCH] Fix #45: defensive handling of drawings without anchors Reading certain XLSX files (containing c:userShapes, certain protected shapes, or drawings whose XML cannot be parsed into the standard xdr:wsDr shape) crashes with: TypeError: Cannot read properties of undefined (reading 'anchors') at XLSX.reconcile (lib/xlsx/xlsx.js) at XLSX.load at XLSX.readFile Root cause: when DrawingXform.parseStream cannot match the XML against xdr:wsDr it resolves to undefined, and that undefined gets stored in model.drawings[name]. The reconcile loop then dereferences drawing.anchors without checking that drawing itself exists. Fix: extend the existing guard from `if (drawingRel)` to `if (drawing && drawingRel)` so unparseable drawings are skipped instead of crashing the whole read. Users typically only need cell data when this happens, matching the upstream proposals on exceljs/exceljs#2591 from @markusjohnsson and others. Adds spec/integration/issues/issue-45-drawing-without-anchors.spec.js which fails on master with the exact TypeError above and passes with this guard in place. Drive-by: collapsed four pre-existing multi-line constructs in lib/xlsx/xlsx.js (a .find() call, a deprecation throw, a regex match, and a Promise.all wrapper) to single-line / extracted-local form so the husky lint-staged hook passes. This repo's .prettierrc (trailingComma: all) conflicts with .eslintrc (comma-dangle functions: "never"); same workaround used in cebd81b (Fix #2943) and 2130a3a (Fix #2966). Behavior is unchanged. Refs protobi/exceljs#45, exceljs/exceljs#2591. --- lib/xlsx/xlsx.js | 65 ++++++++++-------- .../issue-45-drawing-without-anchors.spec.js | 68 +++++++++++++++++++ 2 files changed, 103 insertions(+), 30 deletions(-) create mode 100644 spec/integration/issues/issue-45-drawing-without-anchors.spec.js diff --git a/lib/xlsx/xlsx.js b/lib/xlsx/xlsx.js index 0d66a8c73..fd686c783 100644 --- a/lib/xlsx/xlsx.js +++ b/lib/xlsx/xlsx.js @@ -97,7 +97,11 @@ class XLSX { Object.keys(model.drawings).forEach(name => { const drawing = model.drawings[name]; const drawingRel = model.drawingRels[name]; - if (drawingRel) { + // Guard against drawings whose XML failed to parse (e.g., unrecognised + // root tag, c:userShapes, certain protected files), in which case + // DrawingXform.parseStream returns undefined. See protobi/exceljs#45, + // exceljs/exceljs#2591. + if (drawing && drawingRel) { drawingOptions.rels = drawingRel.reduce((o, rel) => { o[rel.Id] = rel; return o; @@ -164,9 +168,8 @@ class XLSX { const cacheId = cacheIdMatch[1]; // Find the pivot cache definition relationship // pivotTable.rels should have a relationship to the cache definition - const cacheDefRel = pivotTable.rels.find( - rel => rel.Type === RelType.PivotCacheDefinition - ); + const isPivotCacheDef = rel => rel.Type === RelType.PivotCacheDefinition; + const cacheDefRel = pivotTable.rels.find(isPivotCacheDef); if (cacheDefRel) { // Extract filename from Target like "../pivotCache/pivotCacheDefinition1.xml" const targetMatch = cacheDefRel.Target.match(/pivotCacheDefinition\d+\.xml/); @@ -371,9 +374,9 @@ class XLSX { * @deprecated since version 4.0. You should use `#read` instead. Please follow upgrade instruction: https://github.com/exceljs/exceljs/blob/master/UPGRADE-4.0.md */ createInputStream() { - throw new Error( - '`XLSX#createInputStream` is deprecated. You should use `XLSX#read` instead. This method will be removed in version 5.0. Please follow upgrade instruction: https://github.com/exceljs/exceljs/blob/master/UPGRADE-4.0.md' - ); + const deprecatedMsg = + '`XLSX#createInputStream` is deprecated. You should use `XLSX#read` instead. This method will be removed in version 5.0. Please follow upgrade instruction: https://github.com/exceljs/exceljs/blob/master/UPGRADE-4.0.md'; + throw new Error(deprecatedMsg); } async read(stream, options) { @@ -571,7 +574,9 @@ class XLSX { await this._processPivotCacheDefinitionEntry(entry, model, match[1]); break; } - match = entryName.match(/xl\/pivotCache\/_rels\/(pivotCacheDefinition\d+)[.]xml[.]rels/); + const pivotCacheRelsRe = + /xl\/pivotCache\/_rels\/(pivotCacheDefinition\d+)[.]xml[.]rels/; + match = entryName.match(pivotCacheRelsRe); if (match) { await this._processPivotCacheDefinitionRelsEntry(stream, model, match[1]); break; @@ -617,26 +622,25 @@ class XLSX { // Write async addMedia(zip, model) { - await Promise.all( - model.media.map(async medium => { - if (medium.type === 'image') { - const filename = `xl/media/${medium.name}.${medium.extension}`; - if (medium.filename) { - const data = await fsReadFileAsync(medium.filename); - return zip.append(data, {name: filename}); - } - if (medium.buffer) { - return zip.append(medium.buffer, {name: filename}); - } - if (medium.base64) { - const dataimg64 = medium.base64; - const content = dataimg64.substring(dataimg64.indexOf(',') + 1); - return zip.append(content, {name: filename, base64: true}); - } + const mediaPromises = model.media.map(async medium => { + if (medium.type === 'image') { + const filename = `xl/media/${medium.name}.${medium.extension}`; + if (medium.filename) { + const data = await fsReadFileAsync(medium.filename); + return zip.append(data, {name: filename}); + } + if (medium.buffer) { + return zip.append(medium.buffer, {name: filename}); } - throw new Error('Unsupported media'); - }) - ); + if (medium.base64) { + const dataimg64 = medium.base64; + const content = dataimg64.substring(dataimg64.indexOf(',') + 1); + return zip.append(content, {name: filename, base64: true}); + } + } + throw new Error('Unsupported media'); + }); + await Promise.all(mediaPromises); } addDrawings(zip, model) { @@ -702,7 +706,8 @@ class XLSX { addPivotTables(zip, model) { const hasProgrammaticPivots = model.pivotTables && model.pivotTables.length > 0; - const hasPreservedPivots = model.preservedPivotTables && + const hasPreservedPivots = + model.preservedPivotTables && Object.keys(model.preservedPivotTables.pivotTables || {}).length > 0; if (!hasProgrammaticPivots && !hasPreservedPivots) return; @@ -840,8 +845,8 @@ class XLSX { } addCharts(zip, model) { - const hasPreservedCharts = model.preservedChartsXml && - Object.keys(model.preservedChartsXml).length > 0; + const hasPreservedCharts = + model.preservedChartsXml && Object.keys(model.preservedChartsXml).length > 0; if (!hasPreservedCharts) return; diff --git a/spec/integration/issues/issue-45-drawing-without-anchors.spec.js b/spec/integration/issues/issue-45-drawing-without-anchors.spec.js new file mode 100644 index 000000000..ae54fe1a7 --- /dev/null +++ b/spec/integration/issues/issue-45-drawing-without-anchors.spec.js @@ -0,0 +1,68 @@ +// Regression test for protobi/exceljs#45 (and exceljs/exceljs#2591). +// +// Some real-world XLSX files contain drawing parts whose XML cannot be +// parsed into the standard `xdr:wsDr` shape (for example: c:userShapes, +// certain protected files, or other unrecognised root elements). In those +// cases DrawingXform.parseStream resolves to `undefined`, and the entry is +// stored as `model.drawings[name] = undefined`. The reconcile pass then +// dereferences `drawing.anchors` and crashes with: +// +// TypeError: Cannot read properties of undefined (reading 'anchors') +// +// This test exercises XLSX.reconcile directly with a synthetic model that +// reproduces that state. On master it throws; with the guard added in +// lib/xlsx/xlsx.js it completes without error. + +const XLSX = verquire('xlsx/xlsx'); + +describe('github issues', () => { + describe('issue 45 - drawing without anchors should not crash reconcile', () => { + function buildModelWithUndefinedDrawing() { + // Minimal model shape required by XLSX.reconcile. The key field is + // `drawings.drawing1 = undefined`, paired with a matching drawingRels + // entry so the reconcile loop enters the drawing branch. + return { + worksheets: [], + worksheetHash: {}, + worksheetRels: [], + themes: {}, + media: [], + mediaIndex: {}, + drawings: { + drawing1: undefined, // drawing XML failed to parse + }, + drawingRels: { + drawing1: [ + { + Id: 'rId1', + Type: 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/image', + Target: '../media/image1.png', + }, + ], + }, + comments: {}, + tables: {}, + vmlDrawings: {}, + pivotTables: [], + preservedPivotTablesXml: {}, + preservedPivotTablesRels: {}, + preservedPivotCacheDefinitionsXml: {}, + preservedPivotCacheDefinitionsRels: {}, + preservedPivotCacheRecordsXml: {}, + preservedChartsXml: {}, + preservedChartsRels: {}, + preservedChartStylesXml: {}, + preservedChartColorsXml: {}, + preservedDrawingsXml: {}, + preservedDrawingsRels: {}, + }; + } + + it('does not throw when a drawing entry is undefined', () => { + const xlsx = new XLSX(); + const model = buildModelWithUndefinedDrawing(); + + expect(() => xlsx.reconcile(model, {})).to.not.throw(); + }); + }); +});