Fix #45: defensive handling of drawings without anchors

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;
 

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();
+    });
+  });
+});