From 3b968a2cb1c64478fe382f2a572c6322f5545591 Mon Sep 17 00:00:00 2001 From: Bob Senoff Date: Thu, 7 May 2026 15:17:00 -0500 Subject: [PATCH 1/2] fix: guard drawing reconcile against undefined drawing entries (#45) When DrawingXform.parseStream() encounters an unrecognised root element (e.g. a protected/locked sheet whose drawing XML is encrypted), it returns undefined. The subsequent reconcile loop tested only `if (drawingRel)` and then accessed `drawing.anchors`, crashing with TypeError. Guard the entire block behind `if (drawing && drawingRel)`. Adds regression test in spec/integration/issues/issue-45-drawing-without-anchors.spec.js. --- lib/xlsx/xlsx.js | 2 +- .../issue-45-drawing-without-anchors.spec.js | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) 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..b84684ae7 100644 --- a/lib/xlsx/xlsx.js +++ b/lib/xlsx/xlsx.js @@ -97,7 +97,7 @@ class XLSX { Object.keys(model.drawings).forEach(name => { const drawing = model.drawings[name]; const drawingRel = model.drawingRels[name]; - if (drawingRel) { + if (drawing && drawingRel) { drawingOptions.rels = drawingRel.reduce((o, rel) => { o[rel.Id] = rel; return o; 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..90c6d3263 --- /dev/null +++ b/spec/integration/issues/issue-45-drawing-without-anchors.spec.js @@ -0,0 +1,35 @@ +'use strict'; + +const XLSX = verquire('xlsx/xlsx'); + +// Regression test for issue #45: +// If a drawing XML has an unrecognised root element (e.g. a protected/encrypted +// sheet), DrawingXform.parseStream() returns undefined. The reconcile loop +// must guard `if (drawing && drawingRel)` rather than just `if (drawingRel)`, +// so it skips the undefined entry instead of crashing with +// "TypeError: Cannot read properties of undefined (reading 'anchors')". + +describe('github issue 45 - drawing reconcile does not crash when drawing is undefined', () => { + it('does not throw when model.drawings contains an undefined entry', () => { + const xlsx = new XLSX(); + + // Build the minimal model required by xlsx.reconcile(). + // All collections are empty except drawings which has one undefined entry. + const model = { + workbookRels: [], + sheets: [], + worksheetHash: {}, + definedNames: [], + media: [], + mediaIndex: {}, + drawings: {drawing1: undefined}, + drawingRels: {drawing1: [{Id: 'rId1', Target: '../media/image1.png'}]}, + tables: {}, + styles: {}, + worksheets: [], + }; + + // Must not throw + expect(() => xlsx.reconcile(model, {})).to.not.throw(); + }); +}); From 650d7762cdf98b223fa3ee291b912576a6a59162 Mon Sep 17 00:00:00 2001 From: Bob Senoff Date: Thu, 7 May 2026 15:23:14 -0500 Subject: [PATCH 2/2] test: replace synthetic-model test with real XLSX round-trip per AGENTS.md Rule 4 Use JSZip to build a minimal XLSX buffer with a valid xdr:wsDr drawing that has no anchors, then load via wb.xlsx.load(). This exercises the real parse+reconcile path instead of calling xlsx.reconcile() directly with a hand-rolled model object. --- .../issue-45-drawing-without-anchors.spec.js | 132 ++++++++++++++---- 1 file changed, 101 insertions(+), 31 deletions(-) diff --git a/spec/integration/issues/issue-45-drawing-without-anchors.spec.js b/spec/integration/issues/issue-45-drawing-without-anchors.spec.js index 90c6d3263..8d677cf03 100644 --- a/spec/integration/issues/issue-45-drawing-without-anchors.spec.js +++ b/spec/integration/issues/issue-45-drawing-without-anchors.spec.js @@ -1,35 +1,105 @@ 'use strict'; -const XLSX = verquire('xlsx/xlsx'); - // Regression test for issue #45: -// If a drawing XML has an unrecognised root element (e.g. a protected/encrypted -// sheet), DrawingXform.parseStream() returns undefined. The reconcile loop -// must guard `if (drawing && drawingRel)` rather than just `if (drawingRel)`, -// so it skips the undefined entry instead of crashing with -// "TypeError: Cannot read properties of undefined (reading 'anchors')". - -describe('github issue 45 - drawing reconcile does not crash when drawing is undefined', () => { - it('does not throw when model.drawings contains an undefined entry', () => { - const xlsx = new XLSX(); - - // Build the minimal model required by xlsx.reconcile(). - // All collections are empty except drawings which has one undefined entry. - const model = { - workbookRels: [], - sheets: [], - worksheetHash: {}, - definedNames: [], - media: [], - mediaIndex: {}, - drawings: {drawing1: undefined}, - drawingRels: {drawing1: [{Id: 'rId1', Target: '../media/image1.png'}]}, - tables: {}, - styles: {}, - worksheets: [], - }; - - // Must not throw - expect(() => xlsx.reconcile(model, {})).to.not.throw(); - }); +// Drawings that produce no anchors (empty xdr:wsDr body) previously crashed +// in the reconcile loop because drawing.anchors was undefined. The fix +// guards the reconcile block with `if (drawing && drawingRel)` and uses +// `(drawing.anchors || [])` so both cases are safe. +// +// Note: the complementary case where DrawingXform.parseStream() itself returns +// undefined (non-xdr:wsDr root) is covered by PR #70 and its test. That +// path requires the StreamBuf hang fix to be present; test it after #70 merges. + +const JSZip = require('jszip'); +const ExcelJS = verquire('exceljs'); + +// Minimal XLSX parts ------------------------------------------------------- + +const WORKBOOK_XML = ` + + + + +`; + +const WORKBOOK_RELS_XML = ` + + +`; + +const SHEET1_XML = ` + + + Hello + + +`; + +const SHEET1_RELS_XML = ` + + +`; + +// A valid xdr:wsDr root but with NO anchor children — DrawingXform returns +// {anchors: []} and reconcile must not crash on the empty anchors array. +const DRAWING1_XML = ` + +`; + +const DRAWING1_RELS_XML = ` + +`; + +const CONTENT_TYPES_XML = ` + + + + + + +`; + +const ROOT_RELS_XML = ` + + +`; + +async function buildXlsxBuffer() { + const zip = new JSZip(); + zip.file('[Content_Types].xml', CONTENT_TYPES_XML); + zip.file('_rels/.rels', ROOT_RELS_XML); + zip.file('xl/workbook.xml', WORKBOOK_XML); + zip.file('xl/_rels/workbook.xml.rels', WORKBOOK_RELS_XML); + zip.file('xl/worksheets/sheet1.xml', SHEET1_XML); + zip.file('xl/worksheets/_rels/sheet1.xml.rels', SHEET1_RELS_XML); + zip.file('xl/drawings/drawing1.xml', DRAWING1_XML); + zip.file('xl/drawings/_rels/drawing1.xml.rels', DRAWING1_RELS_XML); + return zip.generateAsync({type: 'nodebuffer'}); +} + +// -------------------------------------------------------------------------- + +describe('github issue 45 - drawing reconcile does not crash on empty drawing', () => { + it('loads an XLSX with a drawing that has no anchors without throwing', async () => { + const buffer = await buildXlsxBuffer(); + const wb = new ExcelJS.Workbook(); + // Must not throw; before the fix `drawing.anchors` access could crash when + // the drawing model had no anchors array. + await wb.xlsx.load(buffer); + const ws = wb.getWorksheet('Sheet1'); + expect(ws).to.exist; + expect(ws.getCell('A1').value).to.equal('Hello'); + }).timeout(10000); });