Fix #56: StreamBuf parser hang on non-xdr:wsDr drawing root#70
Open
senoff wants to merge 1 commit into
Open
Conversation
This was referenced May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Original Problem
Loading certain XLSX files caused the parser to hang indefinitely. Files with drawing parts whose XML uses a non-standard root element (not
<xdr:wsDr>) — for examplec:userShapes, vendor-specific drawing namespaces, or drawing files from protected workbooks — causedDrawingXform.parseClose()to never returnfalse. The SAXfor-awaitloop in the StreamBuf infrastructure kept waiting for stream data that never came. See #56.Cause
DrawingXform.parseClose()only returned the loop-exit signal (false) when the close tag exactly matchedthis.tag(xdr:wsDr). Any other close tag fell through todefault: return true, keeping the BaseXform SAX loop alive. When the drawing XML opened with a non-xdr:wsDrroot tag,parseOpendid not capture that root tag name, soparseClosehad no way to know when the document ended.Fix
Two changes to
lib/xlsx/xform/drawing/drawing-xform.js:parseOpen: when the first element is notxdr:wsDr, store its name inthis._rootTagand initialize a minimal model ({anchors: []}).parseClose: returnfalsewhen the closing tag matchesthis._rootTag.Files changed
lib/xlsx/xform/drawing/drawing-xform.js—parseOpenandparseClosespec/integration/issues/issue-56-streambuf-non-xdr-root.spec.js— new integration testTest Run
Test constructs a fixture from
test-issue-1575.xlsx(an existing integration fixture with a self-closing<xdr:wsDr ... />root) by replacing the drawing root with<some:other xmlns:some="http://example.com/unknown-drawing-ns"/>. Loading the modified buffer viawb.xlsx.load()completes within the 5-second timeout. Without the fix this hangs indefinitely.Cross-PR check
lib/xlsx/xform/drawing/drawing-xform.jsis not touched by any other open senoff PR. PR #57 (config drift) and PR #54 (defensive drawings guard) do not touch this file.Excel/soffice verification
sofficeis not installed in this worker environment. The fix only affects the read path (SAX parsing); it does not change how drawing XML is written. Files that previously hung the parser will now load cleanly (with an emptyanchorsarray, as before for any drawing with unknown root). Recommend maintainer verifies a realc:userShapesdrawing file in Excel before merge.grace-review summary
openai:gpt-5.5 — MEDIUM: potential state-reuse issue with
!this.modelsentinel ifDrawingXformis reused. Pre-existing:BaseXform.reset()does not clearthis.model; the normalxdr:wsDrpath callsthis.model = {anchors: []}explicitly on each new root. The non-standard path mirrors this. MEDIUM: test fixture concern (self-closing tag leaves orphaned content). Not applicable:test-issue-1575.xlsxdrawing1.xml is entirely self-closing (<xdr:wsDr ... />), no content follows. gemini:gemini-2.5-flash — CRITICAL: should delegate parsing tothis.mapfor non-standard roots. Out of scope: the fix addresses the hang (parser never exits), not content parsing for unsupported roots. For unknown roots, an emptyanchorsmodel is the correct safe-default behavior per the lenient-parser pattern.Note: committed with
--no-verifyper AGENTS.md Rule 1.