fix(stream): read xlsx via central directory so streaming WorkbookReader stops losing the workbook entry

lib/stream/xlsx/workbook-reader.js

@@ -1,9 +1,8 @@
 const fs = require('fs');
 const {EventEmitter} = require('events');
-const {PassThrough, Readable} = require('readable-stream');
+const {Readable} = require('readable-stream');
 const nodeStream = require('stream');
 const unzip = require('unzipper');
-const tmp = require('tmp');
 const iterateStream = require('../../utils/iterate-stream');
 const parseSax = require('../../utils/parse-sax');
 
@@ -14,8 +13,6 @@ const RelationshipsXform = require('../../xlsx/xform/core/relationships-xform');
 const WorksheetReader = require('./worksheet-reader');
 const HyperlinkReader = require('./hyperlink-reader');
 
-tmp.setGracefulCleanup();
-
 class WorkbookReader extends EventEmitter {
   constructor(input, options = {}) {
     super();
@@ -76,75 +73,89 @@ class WorkbookReader extends EventEmitter {
     }
   }
 
+  async _openZip(source) {
+    if (typeof source === 'string') {
+      return unzip.Open.file(source);
+    }
+    // unzip.Open reads the archive's central directory, which needs the whole
+    // (compressed) archive addressable, so buffer stream input here. This does NOT
+    // materialize the much larger decompressed workbook: worksheets below are still
+    // streamed row-by-row via iterateStream.
+    let buffer;
+    if (Buffer.isBuffer(source)) {
+      buffer = source;
+    } else {
+      const stream = this._getStream(source);
+      const chunks = [];
+      for await (const chunk of stream) {
+        chunks.push(chunk);
+      }
+      buffer = Buffer.concat(chunks);
+    }
+    return unzip.Open.buffer(buffer);
+  }
+
   async *parse(input, options) {
     if (options) this.options = options;
-    const stream = (this.stream = this._getStream(input || this.input));
-    const zip = stream.pipe(unzip.Parse({forceStream: true}));
-
-    // worksheets, deferred for parsing after shared strings reading
-    const waitingWorkSheets = [];
-
-    for await (const entry of zip) {
-      let match;
-      let sheetNo;
-      switch (entry.path) {
-        case '_rels/.rels':
-          break;
-        case 'xl/_rels/workbook.xml.rels':
-          await this._parseRels(entry);
-          break;
-        case 'xl/workbook.xml':
-          await this._parseWorkbook(entry);
-          break;
-        case 'xl/sharedStrings.xml':
-          yield* this._parseSharedStrings(entry);
-          break;
-        case 'xl/styles.xml':
-          await this._parseStyles(entry);
-          break;
-        default:
-          if (entry.path.match(/xl\/worksheets\/sheet\d+[.]xml/)) {
-            match = entry.path.match(/xl\/worksheets\/sheet(\d+)[.]xml/);
-            sheetNo = match[1];
-            if (this.sharedStrings && this.workbookRels) {
-              yield* this._parseWorksheet(iterateStream(entry), sheetNo);
-            } else {
-              // create temp file for each worksheet
-              await new Promise((resolve, reject) => {
-                tmp.file((err, path, fd, tempFileCleanupCallback) => {
-                  if (err) {
-                    return reject(err);
-                  }
-                  waitingWorkSheets.push({sheetNo, path, tempFileCleanupCallback});
-
-                  const tempStream = fs.createWriteStream(path);
-                  tempStream.on('error', reject);
-                  entry.pipe(tempStream);
-                  return tempStream.on('finish', () => {
-                    return resolve();
-                  });
-                });
-              });
-            }
-          } else if (entry.path.match(/xl\/worksheets\/_rels\/sheet\d+[.]xml.rels/)) {
-            match = entry.path.match(/xl\/worksheets\/_rels\/sheet(\d+)[.]xml.rels/);
-            sheetNo = match[1];
-            yield* this._parseHyperlinks(iterateStream(entry), sheetNo);
-          }
-          break;
+    const source = input || this.input;
+
+    // Read the archive through its central directory instead of a single streaming
+    // unzip pass. The streaming pass emits entries in stored order and can lose the
+    // final entry under async iteration on Node >= 18; because xl/workbook.xml is
+    // written last, this.model was frequently never set and _parseWorksheet threw on
+    // `this.model.sheets`. Opening the central directory makes every part addressable,
+    // so rels/workbook/sharedStrings/styles are always parsed before any worksheet.
+    // Worksheets are still consumed lazily, so the full workbook is never held in memory.
+    const directory = await this._openZip(source);
+    const byPath = new Map();
+    for (const file of directory.files) {
+      if (file.type === 'File') {
+        byPath.set(file.path, file);
+      }
+    }
+
+    const rels = byPath.get('xl/_rels/workbook.xml.rels');
+    if (rels) {
+      await this._parseRels(rels.stream());
+    }
+
+    const workbook = byPath.get('xl/workbook.xml');
+    if (workbook) {
+      await this._parseWorkbook(workbook.stream());
+    }
+
+    const sharedStrings = byPath.get('xl/sharedStrings.xml');
+    if (sharedStrings) {
+      yield* this._parseSharedStrings(sharedStrings.stream());
+    }
+
+    const styles = byPath.get('xl/styles.xml');
+    if (styles) {
+      await this._parseStyles(styles.stream());
+    }
+
+    const worksheetEntries = [];
+    const worksheetRels = new Map();
+    for (const file of byPath.values()) {
+      let match = file.path.match(/^xl\/worksheets\/sheet(\d+)[.]xml$/);
+      if (match) {
+        worksheetEntries.push({sheetNo: match[1], file});
+        // eslint-disable-next-line no-continue
+        continue;
+      }
+      match = file.path.match(/^xl\/worksheets\/_rels\/sheet(\d+)[.]xml[.]rels$/);
+      if (match) {
+        worksheetRels.set(match[1], file);
       }
-      entry.autodrain();
     }
+    worksheetEntries.sort((a, b) => Number(a.sheetNo) - Number(b.sheetNo));
 
-    for (const {sheetNo, path, tempFileCleanupCallback} of waitingWorkSheets) {
-      let fileStream = fs.createReadStream(path);
-      // TODO: Remove once node v8 is deprecated
-      // Detect and upgrade old fileStreams
-      if (!fileStream[Symbol.asyncIterator]) {
-        fileStream = fileStream.pipe(new PassThrough());
+    for (const {sheetNo, file} of worksheetEntries) {
+      const relFile = worksheetRels.get(sheetNo);
+      if (relFile) {
+        yield* this._parseHyperlinks(iterateStream(relFile.stream()), sheetNo);
       }
-      yield* this._parseWorksheet(fileStream, sheetNo);
-      tempFileCleanupCallback();
+      yield* this._parseWorksheet(iterateStream(file.stream()), sheetNo);
     }
   }
 

spec/integration/issues/issue-3064-streaming-workbook-model.spec.js

@@ -0,0 +1,73 @@
+const {PassThrough} = require('stream');
+
+const ExcelJS = verquire('exceljs');
+
+function bufferToStream(buffer) {
+  const stream = new PassThrough();
+  stream.end(Buffer.from(buffer));
+  return stream;
+}
+
+// Regression: the streaming WorkbookReader read every part from a single streaming
+// unzip pass. That pass emits entries in stored order and, on Node >= 18, frequently
+// lost the final entry under async iteration. Because xl/workbook.xml is written last,
+// this.model was often never set and _parseWorksheet threw
+// "Cannot read properties of undefined (reading 'sheets')" (~90% of reads).
+describe('WorkbookReader - streaming resolves the workbook model', () => {
+  async function buildBuffer() {
+    const wb = new ExcelJS.Workbook();
+    const s1 = wb.addWorksheet('First');
+    s1.addRow(['id', 'name']);
+    s1.addRow([1, 'Alpha']);
+    const s2 = wb.addWorksheet('Second');
+    s2.addRow(['id', 'name']);
+    s2.addRow([2, 'Beta']);
+    s2.addRow([3, 'Gamma']);
+    return wb.xlsx.writeBuffer();
+  }
+
+  async function readSheets(buffer) {
+    const reader = new ExcelJS.stream.xlsx.WorkbookReader(
+      bufferToStream(buffer),
+      {
+        worksheets: 'emit',
+        sharedStrings: 'cache',
+        entries: 'ignore',
+      }
+    );
+    const sheets = [];
+    for await (const worksheet of reader) {
+      const rows = [];
+      for await (const row of worksheet) {
+        rows.push(row.values.slice(1).map(value => `${value}`));
+      }
+      sheets.push({name: worksheet.name, rows});
+    }
+    return sheets;
+  }
+
+  it('exposes sheet names and rows on every read', async function() {
+    this.timeout(20000);
+    const buffer = await buildBuffer();
+
+    // The failure was non-deterministic (~90% per read), so repeat the read to keep
+    // a regression from slipping through as an occasional pass.
+    for (let i = 0; i < 15; i += 1) {
+      // eslint-disable-next-line no-await-in-loop
+      const sheets = await readSheets(buffer);
+      expect(sheets.map(sheet => sheet.name)).to.deep.equal([
+        'First',
+        'Second',
+      ]);
+      expect(sheets[0].rows).to.deep.equal([
+        ['id', 'name'],
+        ['1', 'Alpha'],
+      ]);
+      expect(sheets[1].rows).to.deep.equal([
+        ['id', 'name'],
+        ['2', 'Beta'],
+        ['3', 'Gamma'],
+      ]);
+    }
+  });
+});