Skip to content

fix: read XLSX tags independent of namespace prefix - #1166

Open
Aias00 wants to merge 3 commits into
apache:mainfrom
Aias00:fix/xlsx-namespace-prefix-1156
Open

Aias00 wants to merge 3 commits into
apache:mainfrom
Aias00:fix/xlsx-namespace-prefix-1156

Conversation

@Aias00

@Aias00 Aias00 commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Closed: #1156

Valid OOXML namespace prefixes are arbitrary, but the XLSX SAX handlers previously dispatched only unprefixed, x:, and ns2: tag names.

What's changed?

  • Enable namespace-aware SAX parsing.
  • Normalize worksheet and shared-string tags through localName, with a qualified-name fallback.
  • Add a compatibility test that rewrites worksheet and shared-string XML with a p: prefix and verifies model reading.

Verification:

CompatibilityTest#readXlsxWithArbitraryNamespacePrefix
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
spotless:check: passed

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Signed-off-by: liuhy <liuhongyu@apache.org>
@nkuprins

nkuprins commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Dispatching on localName alone matches elements from other namespaces. Before, only unprefixed names and the x: and ns2: prefixes matched.

POI's XSSFSheetXMLHandler checks the namespace first:

if (uri != null && !uri.equals(NS_SPREADSHEETML)) {
    return;
}

Should we add the same guard, plus a test?

Signed-off-by: liuhy <liuhongyu@apache.org>
@Aias00

Aias00 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Done in cfde3ec. Both worksheet and shared-string SAX handlers now ignore elements outside the SpreadsheetML namespace. The integration fixture also includes foreign-namespace row/c/v and si/t elements, so both paths are covered. CompatibilityTest passes all 10 tests, and spotless:check passes.

@skytin1004 skytin1004 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Aias00, while testing nested namespace content, I noticed that the namespace guard skips the foreign element events, but characters() still receives their text while the outer <t> is active. I tested an outer <t> containing a foreign <ext:t> element, and Name + foreign + 0 was read as Nameforeign0. I saw the same issue inside <v>, where the extra text can change the shared-string index and cause an IndexOutOfBoundsException.

I think both handlers should keep an ignored-element depth: increment it when entering a foreign element, ignore all character data while the depth is nonzero, and decrement it on the matching end event. Adding nested foreign-content cases for both <t> and <v> would cover this path.

Signed-off-by: liuhy <liuhongyu@apache.org>
@Aias00

Aias00 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Done in d9ea4c2. Both SAX handlers now track ignored-subtree depth, so character data inside a foreign namespace is discarded until the matching end element. The integration fixture covers nested foreign <ext:t> inside SpreadsheetML <t> and nested <ext:v> inside <v>, in addition to sibling foreign elements. The targeted regression test and all 10 CompatibilityTest cases pass, and spotless:check passes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] XLSX reader ignores valid worksheet tags with arbitrary namespace prefixes

3 participants