diff --git a/lib/stream/xlsx/workbook-reader.js b/lib/stream/xlsx/workbook-reader.js index eb75b7220..3629209ed 100644 --- a/lib/stream/xlsx/workbook-reader.js +++ b/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); } } diff --git a/spec/integration/issues/issue-3064-streaming-workbook-model.spec.js b/spec/integration/issues/issue-3064-streaming-workbook-model.spec.js new file mode 100644 index 000000000..45c76a486 --- /dev/null +++ b/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'], + ]); + } + }); +});