diff --git a/index.d.ts b/index.d.ts index cae3394bd..63754c40b 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1936,6 +1936,14 @@ export namespace stream { * Styles can add some performance overhead. Default is false */ useStyles: boolean; + + /** + * XLS-352: an optional Date stamped as the mtime of EVERY zip entry the writer emits, so + * the output is byte-deterministic across runs (a per-entry `date` otherwise defaults to + * `new Date()`). Applies to both the streaming WorkbookWriter and the buffered + * `xlsx.write`/`writeBuffer` path. When omitted, entries keep the wall clock. + */ + zipEntryDate?: Date; } interface ArchiverZipOptions { diff --git a/lib/stream/xlsx/workbook-writer.js b/lib/stream/xlsx/workbook-writer.js index 2d97cec88..c8d96099d 100644 --- a/lib/stream/xlsx/workbook-writer.js +++ b/lib/stream/xlsx/workbook-writer.js @@ -44,6 +44,13 @@ class WorkbookWriter { this.zipOptions = options.zip; + // XLS-352: an optional Date stamped as the mtime of EVERY zip entry this writer appends. + // `date` is a per-entry archiver option (default `new Date()`); archiver ignores any + // module-level `zip: {date}`, so byte-deterministic output requires forwarding this value to + // each `zip.append` call — which the `_append` choke point below does. Left undefined, entries + // keep the wall clock exactly as before, so callers that do not pass it see no change. + this.zipEntryDate = options.zipEntryDate; + this.media = []; this.commentRefs = []; @@ -65,9 +72,21 @@ class WorkbookWriter { return this._definedNames; } + // XLS-352 choke point: every zip entry is appended through here, so a caller-supplied + // `zipEntryDate` (see the constructor) is stamped as the entry's mtime. When it is undefined — + // or the individual call already carries its own `date` — the options pass through untouched and + // archiver's default (the wall clock) is preserved. This is the durable, in-fork replacement for + // the server's `PinnedWorkbookWriter` subclass, which wrapped `zip.append` from outside. + _append(data, options) { + if (this.zipEntryDate !== undefined && (!options || options.date === undefined)) { + return this.zip.append(data, {...options, date: this.zipEntryDate}); + } + return this.zip.append(data, options); + } + _openStream(path) { const stream = new StreamBuf({bufSize: 65536, batch: true}); - this.zip.append(stream, {name: path}); + this._append(stream, {name: path}); stream.on('finish', () => { stream.emit('zipped'); }); @@ -187,14 +206,14 @@ class WorkbookWriter { addStyles() { return new Promise(resolve => { - this.zip.append(this.styles.xml, {name: 'xl/styles.xml'}); + this._append(this.styles.xml, {name: 'xl/styles.xml'}); resolve(); }); } addThemes() { return new Promise(resolve => { - this.zip.append(theme1Xml, {name: 'xl/theme/theme1.xml'}); + this._append(theme1Xml, {name: 'xl/theme/theme1.xml'}); resolve(); }); } @@ -207,7 +226,7 @@ class WorkbookWriter { {Id: 'rId2', Type: RelType.CoreProperties, Target: 'docProps/core.xml'}, {Id: 'rId3', Type: RelType.ExtenderProperties, Target: 'docProps/app.xml'}, ]); - this.zip.append(xml, {name: '/_rels/.rels'}); + this._append(xml, {name: '/_rels/.rels'}); resolve(); }); } @@ -222,7 +241,7 @@ class WorkbookWriter { }; const xform = new ContentTypesXform(); const xml = xform.toXml(model); - this.zip.append(xml, {name: '[Content_Types].xml'}); + this._append(xml, {name: '[Content_Types].xml'}); resolve(); }); } @@ -236,12 +255,12 @@ class WorkbookWriter { return this.zip.file(medium.filename, {name: filename}); } if (medium.buffer) { - return this.zip.append(medium.buffer, {name: filename}); + return this._append(medium.buffer, {name: filename}); } if (medium.base64) { const dataimg64 = medium.base64; const content = dataimg64.substring(dataimg64.indexOf(',') + 1); - return this.zip.append(content, {name: filename, base64: true}); + return this._append(content, {name: filename, base64: true}); } } throw new Error('Unsupported media'); @@ -256,7 +275,7 @@ class WorkbookWriter { }; const xform = new AppXform(); const xml = xform.toXml(model); - this.zip.append(xml, {name: 'docProps/app.xml'}); + this._append(xml, {name: 'docProps/app.xml'}); resolve(); }); } @@ -265,7 +284,7 @@ class WorkbookWriter { return new Promise(resolve => { const coreXform = new CoreXform(); const xml = coreXform.toXml(this); - this.zip.append(xml, {name: 'docProps/core.xml'}); + this._append(xml, {name: 'docProps/core.xml'}); resolve(); }); } @@ -275,7 +294,7 @@ class WorkbookWriter { return new Promise(resolve => { const sharedStringsXform = new SharedStringsXform(); const xml = sharedStringsXform.toXml(this.sharedStrings); - this.zip.append(xml, {name: '/xl/sharedStrings.xml'}); + this._append(xml, {name: '/xl/sharedStrings.xml'}); resolve(); }); } @@ -308,13 +327,12 @@ class WorkbookWriter { return new Promise(resolve => { const xform = new RelationshipsXform(); const xml = xform.toXml(relationships); - this.zip.append(xml, {name: '/xl/_rels/workbook.xml.rels'}); + this._append(xml, {name: '/xl/_rels/workbook.xml.rels'}); resolve(); }); } addWorkbook() { - const {zip} = this; const model = { worksheets: this._worksheets.filter(Boolean), definedNames: this._definedNames.model, @@ -326,7 +344,7 @@ class WorkbookWriter { return new Promise(resolve => { const xform = new WorkbookXform(); xform.prepare(model); - zip.append(xform.toXml(model), {name: '/xl/workbook.xml'}); + this._append(xform.toXml(model), {name: '/xl/workbook.xml'}); resolve(); }); } diff --git a/lib/utils/zip-stream.js b/lib/utils/zip-stream.js index 96efd8e7b..b9f418f7d 100644 --- a/lib/utils/zip-stream.js +++ b/lib/utils/zip-stream.js @@ -35,7 +35,22 @@ class ZipWriter extends events.EventEmitter { } } + // XLS-352: stamp a caller-supplied `entryDate` as the mtime of EVERY entry before generating, + // so the buffered writer's output is byte-deterministic. Doing it here — the single generate + // choke point — rather than per-append is deliberate: JSZip AUTO-CREATES folder entries (`xl/`, + // `xl/worksheets/`, ...) that never pass through `append`, and each defaults to `new Date()`. + // Pinning at append would leave those folder entries on the wall clock (two runs then differ in + // exactly their headers). Iterating `this.zip.files` here covers the file entries and the + // auto-created folder entries alike. Undefined leaves JSZip's default (the wall clock) untouched. + _pinEntryDates() { + if (this.options.entryDate === undefined) return; + Object.keys(this.zip.files).forEach(name => { + this.zip.files[name].date = this.options.entryDate; + }); + } + async finalize() { + this._pinEntryDates(); const content = await this.zip.generateAsync(this.options); this.stream.end(content); this.emit('finish'); diff --git a/lib/xlsx/xform/book/workbook-xform.js b/lib/xlsx/xform/book/workbook-xform.js index 4826d5e3b..9c56befb3 100644 --- a/lib/xlsx/xform/book/workbook-xform.js +++ b/lib/xlsx/xform/book/workbook-xform.js @@ -59,7 +59,10 @@ class WorkbookXform extends BaseXform { }); } - if (sheet.pageSetup && (sheet.pageSetup.printTitlesRow || sheet.pageSetup.printTitlesColumn)) { + if ( + sheet.pageSetup && + (sheet.pageSetup.printTitlesRow || sheet.pageSetup.printTitlesColumn) + ) { const ranges = []; if (sheet.pageSetup.printTitlesColumn) { @@ -104,10 +107,7 @@ class WorkbookXform extends BaseXform { this.map.calcPr.render(xmlStream, model.calcProperties); // Render pivot caches (both programmatic and preserved) - const allPivotCaches = [ - ...(model.pivotTables || []), - ...(model.preservedPivotCaches || []), - ]; + const allPivotCaches = [...(model.pivotTables || []), ...(model.preservedPivotCaches || [])]; this.map.pivotCaches.render(xmlStream, allPivotCaches); xmlStream.closeNode(); @@ -199,7 +199,12 @@ class WorkbookXform extends BaseXform { _.each(model.definedNames, definedName => { if (definedName.name === '_xlnm.Print_Area') { worksheet = worksheets[definedName.localSheetId]; - if (worksheet) { + // An empty or unparseable Print_Area range decodes to `undefined` — the + // classic "print area cleared in Excel (or a third-party writer) which + // leaves the `_xlnm.Print_Area` defined name behind with no ref" artifact. + // Excel and our own streaming reader both treat that as "no print area"; + // skip it here rather than crash in colCache.decodeEx(undefined). (XLS-861) + if (worksheet && definedName.ranges && definedName.ranges[0]) { if (!worksheet.pageSetup) { worksheet.pageSetup = {}; } diff --git a/lib/xlsx/xlsx.js b/lib/xlsx/xlsx.js index 0d66a8c73..6f34ad812 100644 --- a/lib/xlsx/xlsx.js +++ b/lib/xlsx/xlsx.js @@ -1104,7 +1104,10 @@ class XLSX { async write(stream, options) { options = options || {}; const {model} = this.workbook; - const zip = new ZipStream.ZipWriter(options.zip); + // XLS-352: forward an optional `zipEntryDate` into the zip writer so every buffered entry's mtime + // is stamped with it (the buffered twin of the streaming writer's per-entry pin). Undefined leaves + // JSZip's default (the wall clock) untouched, so callers that do not pass it see no change. + const zip = new ZipStream.ZipWriter({...options.zip, entryDate: options.zipEntryDate}); zip.pipe(stream); this.prepareModel(model, options); diff --git a/package.json b/package.json index da5de1b5a..d513c900e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@protobi/exceljs", - "version": "4.4.0-protobi.10", + "version": "4.4.0-protobi.11", "description": "Excel Workbook Manager - Temporary fork with pivot table enhancements and bug fixes pending upstream merge", "private": false, "license": "MIT", diff --git a/spec/integration/issues/issue-xls352-zip-entry-mtime-pin.spec.js b/spec/integration/issues/issue-xls352-zip-entry-mtime-pin.spec.js new file mode 100644 index 000000000..ac662c4a4 --- /dev/null +++ b/spec/integration/issues/issue-xls352-zip-entry-mtime-pin.spec.js @@ -0,0 +1,138 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const JSZip = require('jszip'); + +const ExcelJS = verquire('exceljs'); + +// XLS-352 — teach the fork to pin every zip entry's mtime itself, so the server's +// `PinnedWorkbookWriter` subclass (which reached past the published surface to wrap `zip.append` +// from inside the base constructor) can be deleted. +// +// The defect: `date` is a PER-ENTRY zip option (archiver's `data.date` / JSZip's file `date`), +// defaulting to `new Date()` at append time. A module-level `zip: {date}` is a no-op — archiver +// never forwards it to an entry header. So each entry's local-file-header mtime carries the wall +// clock at the DOS format's two-second granularity, and two builds are byte-identical only when +// they happen to land in the same two-second bucket (XLS-350's coin-flip). +// +// The fix adds one option, `zipEntryDate`, threaded to EVERY `zip.append` on BOTH writer paths: +// - streaming: `stream.xlsx.WorkbookWriter` — via the `_append` choke point. +// - buffered: `Workbook.xlsx.writeBuffer/writeFile` — via `ZipWriter.append` (JSZip `.file`). +// +// This is pinned three ways per path, because each alone is a false green: +// 1. Byte-identity across a real DOS tick — the determinism property itself. +// 2. Every entry's parsed mtime equals the pin — proves the date reached the entry headers, not +// just that two runs happened to match. +// 3. POSITIVE CONTROL: WITHOUT the option the output differs from the pinned output AND the +// entries carry the wall clock (a recent year), so the option is demonstrably what pins them. +// Revert the fork fix and arms 1+2 stay green vacuously; this arm is what reddens. + +// A fixed pin on an even two-second boundary (DOS granularity) and at midday, so a CI timezone +// offset shifts the hour but never the calendar date the readback asserts. +const PIN = new Date(2000, 5, 15, 12, 0, 0); + +const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); + +let tmpCounter = 0; +function tmpFile() { + return path.join(os.tmpdir(), `xls352-stream-${process.pid}-${Date.now()}-${tmpCounter++}.xlsx`); +} + +// Build a workbook through the STREAMING writer and return its bytes. `extraOptions` carries (or +// omits) `zipEntryDate`. `created`/`modified` are pinned too so docProps/core.xml — entry CONTENT, +// the other half of byte-identity — does not itself carry the wall clock and mask the entry-mtime +// property under test. +async function buildStreaming(extraOptions) { + const file = tmpFile(); + const wb = new ExcelJS.stream.xlsx.WorkbookWriter({ + stream: fs.createWriteStream(file, {flags: 'w'}), + useStyles: true, + useSharedStrings: false, + ...extraOptions, + }); + wb.created = PIN; + wb.modified = PIN; + const ws = wb.addWorksheet('S'); + ws.addRow(['hello', 42]).commit(); + ws.commit(); + await wb.commit(); + const buffer = fs.readFileSync(file); + fs.unlinkSync(file); + return buffer; +} + +// Build the same workbook through the BUFFERED writer (`writeBuffer`) and return its bytes. +async function buildBuffered(extraOptions) { + const wb = new ExcelJS.Workbook(); + wb.created = PIN; + wb.modified = PIN; + const ws = wb.addWorksheet('S'); + ws.addRow(['hello', 42]); + return wb.xlsx.writeBuffer(extraOptions); +} + +// Every entry, INCLUDING the folder entries JSZip auto-creates — those default to `new Date()` and +// are exactly what made the buffered writer non-deterministic until the finalize-time pin. +async function entryDates(buffer) { + const zip = await JSZip.loadAsync(buffer); + return Object.values(zip.files).map(entry => entry.date); +} + +describe('github issues: XLS-352 fork pins every zip entry mtime (durable fix for XLS-350)', function() { + // Two full builds plus a real >2s delay per determinism arm. + this.timeout(30000); + + describe('streaming writer (stream.xlsx.WorkbookWriter)', () => { + it('two runs separated by more than one DOS tick are byte-identical when zipEntryDate is pinned', async () => { + const first = await buildStreaming({zipEntryDate: PIN}); + await sleep(2500); + const second = await buildStreaming({zipEntryDate: PIN}); + expect(first.equals(second)).to.equal(true); + }); + + it('stamps every zip entry mtime with zipEntryDate', async () => { + const dates = await entryDates(await buildStreaming({zipEntryDate: PIN})); + expect(dates.length).to.be.greaterThan(3); + dates.forEach(date => { + expect(date.getFullYear()).to.equal(2000); + expect(date.getMonth()).to.equal(5); + expect(date.getDate()).to.equal(15); + }); + }); + + it('POSITIVE CONTROL: without zipEntryDate the bytes differ and entries carry the wall clock', async () => { + const pinned = await buildStreaming({zipEntryDate: PIN}); + const unpinned = await buildStreaming({}); + expect(pinned.equals(unpinned)).to.equal(false); + const dates = await entryDates(unpinned); + expect(dates.some(date => date.getFullYear() >= 2020)).to.equal(true); + }); + }); + + describe('buffered writer (Workbook.xlsx.writeBuffer)', () => { + it('two runs separated by more than one DOS tick are byte-identical when zipEntryDate is pinned', async () => { + const first = await buildBuffered({zipEntryDate: PIN}); + await sleep(2500); + const second = await buildBuffered({zipEntryDate: PIN}); + expect(first.equals(second)).to.equal(true); + }); + + it('stamps every zip entry mtime with zipEntryDate', async () => { + const dates = await entryDates(await buildBuffered({zipEntryDate: PIN})); + expect(dates.length).to.be.greaterThan(3); + dates.forEach(date => { + expect(date.getFullYear()).to.equal(2000); + expect(date.getMonth()).to.equal(5); + expect(date.getDate()).to.equal(15); + }); + }); + + it('POSITIVE CONTROL: without zipEntryDate the bytes differ and entries carry the wall clock', async () => { + const pinned = await buildBuffered({zipEntryDate: PIN}); + const unpinned = await buildBuffered({}); + expect(pinned.equals(unpinned)).to.equal(false); + const dates = await entryDates(unpinned); + expect(dates.some(date => date.getFullYear() >= 2020)).to.equal(true); + }); + }); +}); diff --git a/spec/unit/xlsx/xform/book/workbook-xform.spec.js b/spec/unit/xlsx/xform/book/workbook-xform.spec.js index 47adc0675..8b8fa2169 100644 --- a/spec/unit/xlsx/xform/book/workbook-xform.spec.js +++ b/spec/unit/xlsx/xform/book/workbook-xform.spec.js @@ -11,10 +11,7 @@ const expectations = [ return new WorkbookXform(); }, preparedModel: require('./data/book.1.1.json'), - xml: fs - .readFileSync(`${__dirname}/data/book.1.2.xml`) - .toString() - .replace(/\r\n/g, '\n'), + xml: fs.readFileSync(`${__dirname}/data/book.1.2.xml`).toString().replace(/\r\n/g, '\n'), parsedModel: require('./data/book.1.3.json'), tests: ['render', 'renderIn', 'parse'], }, @@ -23,10 +20,7 @@ const expectations = [ create() { return new WorkbookXform(); }, - xml: fs - .readFileSync(`${__dirname}/data/book.2.2.xml`) - .toString() - .replace(/\r\n/g, '\n'), + xml: fs.readFileSync(`${__dirname}/data/book.2.2.xml`).toString().replace(/\r\n/g, '\n'), parsedModel: require('./data/book.2.3.json'), tests: ['parse'], }, @@ -34,4 +28,45 @@ const expectations = [ describe('WorkbookXform', () => { testXformHelper(expectations); + + // XLS-861: reconcile() must not crash on an `_xlnm.Print_Area` defined name + // that carries no ref (empty `ranges`). Excel and third-party writers leave + // this artifact behind when a print area is cleared; the streaming reader + // already tolerates it, but the full loader (workbook.xlsx.load) used to throw + // `TypeError: Cannot read properties of undefined (reading 'match')` in + // colCache.decodeEx(undefined). Reconcile should treat it as "no print area". + describe('reconcile print areas', () => { + function modelWith(definedNames) { + const worksheet = {}; + return { + model: { + workbookRels: [{Id: 'rId1', Target: 'worksheets/sheet1.xml'}], + sheets: [{rId: 'rId1', name: 'Sheet1', id: 1, state: 'visible'}], + worksheetHash: {'xl/worksheets/sheet1.xml': worksheet}, + definedNames, + media: [], + }, + worksheet, + }; + } + + it('tolerates an empty _xlnm.Print_Area (no ref) instead of throwing', () => { + const xform = new WorkbookXform(); + const {model, worksheet} = modelWith([ + {name: '_xlnm.Print_Area', localSheetId: 0, ranges: []}, + ]); + expect(() => xform.reconcile(model)).to.not.throw(); + // Treated as "no print area": pageSetup.printArea is never set. + expect(worksheet.pageSetup && worksheet.pageSetup.printArea).to.be.undefined(); + }); + + it('still reconciles a valid _xlnm.Print_Area into pageSetup.printArea', () => { + const xform = new WorkbookXform(); + const {model, worksheet} = modelWith([ + {name: '_xlnm.Print_Area', localSheetId: 0, ranges: ['Sheet1!$A$1:$B$2']}, + ]); + expect(() => xform.reconcile(model)).to.not.throw(); + expect(worksheet.pageSetup.printArea).to.equal('A1:B2'); + }); + }); });