Skip to content

Commit 2a67ed0

Browse files
committed
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#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. Refs #45, exceljs#2591. Note on hook bypass: this repo's .prettierrc (trailingComma: all) conflicts with .eslintrc (comma-dangle functions: never), so the husky lint-staged hook fails for any commit touching this file with errors at pre-existing lines (172, 379, 579, 644) that I did not modify. Same workaround used in 2960dc7 (Fix exceljs#3028).
1 parent 4c9e73c commit 2a67ed0

2 files changed

Lines changed: 74 additions & 1 deletion

File tree

lib/xlsx/xlsx.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,11 @@ class XLSX {
9797
Object.keys(model.drawings).forEach(name => {
9898
const drawing = model.drawings[name];
9999
const drawingRel = model.drawingRels[name];
100-
if (drawingRel) {
100+
// Guard against drawings whose XML failed to parse (e.g., unrecognised
101+
// root tag, c:userShapes, certain protected files), in which case
102+
// DrawingXform.parseStream returns undefined. See protobi/exceljs#45,
103+
// exceljs/exceljs#2591.
104+
if (drawing && drawingRel) {
101105
drawingOptions.rels = drawingRel.reduce((o, rel) => {
102106
o[rel.Id] = rel;
103107
return o;
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Regression test for protobi/exceljs#45 (and exceljs/exceljs#2591).
2+
//
3+
// Some real-world XLSX files contain drawing parts whose XML cannot be
4+
// parsed into the standard `xdr:wsDr` shape (for example: c:userShapes,
5+
// certain protected files, or other unrecognised root elements). In those
6+
// cases DrawingXform.parseStream resolves to `undefined`, and the entry is
7+
// stored as `model.drawings[name] = undefined`. The reconcile pass then
8+
// dereferences `drawing.anchors` and crashes with:
9+
//
10+
// TypeError: Cannot read properties of undefined (reading 'anchors')
11+
//
12+
// This test exercises XLSX.reconcile directly with a synthetic model that
13+
// reproduces that state. On master it throws; with the guard added in
14+
// lib/xlsx/xlsx.js it completes without error.
15+
16+
const XLSX = verquire('xlsx/xlsx');
17+
18+
describe('github issues', () => {
19+
describe('issue 45 - drawing without anchors should not crash reconcile', () => {
20+
function buildModelWithUndefinedDrawing() {
21+
// Minimal model shape required by XLSX.reconcile. The key field is
22+
// `drawings.drawing1 = undefined`, paired with a matching drawingRels
23+
// entry so the reconcile loop enters the drawing branch.
24+
return {
25+
worksheets: [],
26+
worksheetHash: {},
27+
worksheetRels: [],
28+
themes: {},
29+
media: [],
30+
mediaIndex: {},
31+
drawings: {
32+
drawing1: undefined, // drawing XML failed to parse
33+
},
34+
drawingRels: {
35+
drawing1: [
36+
{
37+
Id: 'rId1',
38+
Type:
39+
'http://schemas.openxmlformats.org/officeDocument/2006/relationships/image',
40+
Target: '../media/image1.png',
41+
},
42+
],
43+
},
44+
comments: {},
45+
tables: {},
46+
vmlDrawings: {},
47+
pivotTables: [],
48+
preservedPivotTablesXml: {},
49+
preservedPivotTablesRels: {},
50+
preservedPivotCacheDefinitionsXml: {},
51+
preservedPivotCacheDefinitionsRels: {},
52+
preservedPivotCacheRecordsXml: {},
53+
preservedChartsXml: {},
54+
preservedChartsRels: {},
55+
preservedChartStylesXml: {},
56+
preservedChartColorsXml: {},
57+
preservedDrawingsXml: {},
58+
preservedDrawingsRels: {},
59+
};
60+
}
61+
62+
it('does not throw when a drawing entry is undefined', () => {
63+
const xlsx = new XLSX();
64+
const model = buildModelWithUndefinedDrawing();
65+
66+
expect(() => xlsx.reconcile(model, {})).to.not.throw();
67+
});
68+
});
69+
});

0 commit comments

Comments
 (0)