From bbecab6f6169e0f4198428b916b6998f6026ee71 Mon Sep 17 00:00:00 2001 From: Bob Senoff Date: Sat, 2 May 2026 05:19:46 -0500 Subject: [PATCH] fix: StreamBuf parser hang on workbook drawings with non-xdr:wsDr root element (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DrawingXform.parseClose() compared the closing tag against the hardcoded string "xdr:wsDr" and fell to `default: return true` for anything else. Because BaseXform.parse() treats `false` as the loop-exit signal, a drawing file whose root uses a non-standard namespace caused the for-await loop to consume all SAX events and then block waiting for stream data that StreamBuf never delivers — hanging the load indefinitely. Fix approach (b): capture the actual root tag name into this._rootTag during parseOpen() when the element is not the expected xdr:wsDr. parseClose() then returns false when that captured name closes, giving the loop-exit signal regardless of which namespace the drawing root uses. Regression test: spec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js rewrites xl/drawings/drawing1.xml in the test-issue-1575.xlsx fixture to use an unknown namespace root element and asserts that wb.xlsx.load() completes within 5 seconds. Also fixes .prettierrc trailingComma "all" → "es5" to resolve the prettier↔eslint comma-dangle conflict on function arguments that was blocking the pre-commit hook (same fix as fix/prettier-eslint-config-drift). --- .prettierrc | 2 +- lib/xlsx/xform/drawing/drawing-xform.js | 23 ++++++++-- .../issue-56-streambuf-non-xdr-root.spec.js | 46 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 spec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js diff --git a/.prettierrc b/.prettierrc index 9f4d9560f..d49c5c4f3 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,7 +1,7 @@ { "bracketSpacing": false, "printWidth": 100, - "trailingComma": "all", + "trailingComma": "es5", "arrowParens": "avoid", "singleQuote": true } diff --git a/lib/xlsx/xform/drawing/drawing-xform.js b/lib/xlsx/xform/drawing/drawing-xform.js index 6bc4c43d8..f77681580 100644 --- a/lib/xlsx/xform/drawing/drawing-xform.js +++ b/lib/xlsx/xform/drawing/drawing-xform.js @@ -53,14 +53,24 @@ class DrawingXform extends BaseXform { switch (node.name) { case this.tag: this.reset(); + this._rootTag = undefined; this.model = { anchors: [], }; break; default: - this.parser = this.map[node.name]; - if (this.parser) { - this.parser.parseOpen(node); + // If we have not yet opened a model, this is the root element of a + // drawing file with a non-standard namespace (e.g. not xdr:wsDr). + // Capture the actual root tag so parseClose can signal loop-exit when + // the corresponding close tag arrives, preventing a StreamBuf hang. + if (!this.model) { + this._rootTag = node.name; + this.model = {anchors: []}; + } else { + this.parser = this.map[node.name]; + if (this.parser) { + this.parser.parseOpen(node); + } } break; } @@ -85,6 +95,13 @@ class DrawingXform extends BaseXform { case this.tag: return false; default: + // Return false (loop-exit signal) when the actual root element closes, + // even if it used a non-standard namespace instead of xdr:wsDr. + // Without this, the StreamBuf for-await loop exhausts all SAX events + // and blocks waiting for stream data that never arrives (issue #56). + if (name === this._rootTag) { + return false; + } // could be some unrecognised tags return true; } diff --git a/spec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js b/spec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js new file mode 100644 index 000000000..651115f86 --- /dev/null +++ b/spec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js @@ -0,0 +1,46 @@ +const fs = require('fs'); +const JSZip = require('jszip'); + +const ExcelJS = verquire('exceljs'); + +// Test for issue #56: StreamBuf parser hangs when a workbook drawing file uses +// a root element other than (e.g. a proprietary or unknown namespace). +// +// Root cause: DrawingXform.parseClose() only returned the loop-exit signal (false) +// when the close tag was exactly "xdr:wsDr". Any other close tag fell to +// `default: return true`, keeping the BaseXform for-await loop alive until the +// underlying stream was exhausted — which never happens with StreamBuf unless the +// caller explicitly ends it. Fix: capture the actual root tag in parseOpen() and +// also return false when that tag closes. + +function buildFixture() { + const srcBuf = fs.readFileSync('./spec/integration/data/test-issue-1575.xlsx'); + const unknownTag = ''; + + return JSZip.loadAsync(srcBuf).then(zip => + zip.files['xl/drawings/drawing1.xml'].async('string').then(originalXml => { + // The fixture drawing is a self-closing element: + // Replace the entire tag with a minimal unknown-namespace tag using + // string indexing (avoids regex issues with long attribute strings). + const openStart = originalXml.indexOf('', openStart); + const modifiedXml = + originalXml.substring(0, openStart) + unknownTag + originalXml.substring(openEnd + 1); + + zip.file('xl/drawings/drawing1.xml', modifiedXml); + return zip.generateAsync({type: 'nodebuffer'}); + }) + ); +} + +describe('github issues', () => { + it('issue 56 - StreamBuf hang on drawing with non-xdr:wsDr root element', () => { + const wb = new ExcelJS.Workbook(); + return buildFixture() + .then(buf => wb.xlsx.load(buf)) + .then(() => { + // Arriving here means the parser exited cleanly — no StreamBuf hang. + expect(true).to.equal(true); + }); + }).timeout(5000); +});