`DOMWrappingReader.getDepth()` is one too low at `START_ELEMENT`, and typed array reads lose a level

Open
  • Is something wrong?
#62 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
58/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
java
Domain
api

Research direction

Start in DOMWrappingReader, where getDepth() returns _depth as is and readElementAsArray() moves into children or the virtual END_ELEMENT without the ++_depth that next() applies. Compare with Stax2ReaderAdapter.getDepth(), which is the reference for the expected values, and reproduce the two tables in the issue with a DOMSource. Done when getDepth() matches the expected values at START_ELEMENT, END_ELEMENT and after array reads, and the existing tests still pass. The compatibility impact on WstxDOMWrappingReader users needs a maintainer decision.

Written by the indexing model from the issue text.

Description

Bug found when fixing #60, I'm afraid Claude is right here, too.
I can provide the PR if you wish.

XMLStreamReader2.getDepth() is documented as 1 at the root element itself, and the same for a start element and its matching end element. DOMWrappingReader breaks this in two ways:

  1. At every START_ELEMENT, getDepth() returns one less than it should. _depth is only incremented when next() moves past the start element, and getDepth() returns it as is. Values at END_ELEMENT and inside elements are correct.
  2. readElementAsArray() (and so readElementAsIntArray() etc.) permanently lowers the depth by one. Starting at START_ELEMENT, it moves into the children (or to the virtual END_ELEMENT of an empty element) without the ++_depth that next() does there, but leaving the END_ELEMENT still does --_depth. This happens for every array read, whether partial, complete or of an empty element. Each one lowers all later depths by one more.

This affects Woodstox too: WstxDOMWrappingReader, returned for any DOMSource, inherits both methods without overriding them. So its depths differ from Woodstox's own stream reader for the same document (TestXMLStreamReader2 in Woodstox checks the stream reader only; no DOM test checks getDepth()). Stax2ReaderAdapter reports the correct values in all cases below.

Repro (depth after each event):

Document, action Expected DOMWrappingReader
<root><a><b/></a>x<c/></root>, plain next() loop <root>1 <a>2 <b>3 </b>3 </a>2 x:1 <c>2 </c>2 </root>1 <root>0 <a>1 <b>2 </b>3 </a>2 x:1 <c>1 </c>2 </root>1
<root><a>1 2 3</a><b/></root>, <a> read with readElementAsIntArray() to the end <root>1 <a>2 </a>2 <b>2 </b>2 </root>1 <root>0 <a>1 </a>1 <b>0 </b>1 </root>0

Coalescing or not makes no difference. Same results for a partial read followed by next(), and for an empty <a/>.

Fix:

  1. getDepth(): return _depth+1 when the current event is START_ELEMENT. This mirrors the END_ELEMENT adjustment in Stax2ReaderAdapter.getDepth().
  2. readElementAsArray(): ++_depth when starting at START_ELEMENT, before either branch (children or empty element).

Checked with a temporary patch: all cases above then match Stax2ReaderAdapter, and the existing tests pass.

Compatibility: fix 1 changes the value callers get at every START_ELEMENT from a DOM reader, including Woodstox users with a DOMSource. Callers that adjusted for the old value will be off by one in the other direction. Fix 2 only changes behaviour after array reads, where the old values were wrong in any case.

Dominant language
Java
Stars
42
Forks
21
Avg merge
3h 50m
Merged PRs (30d)
2

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from FasterXML/stax2-api

All issues in FasterXML/stax2-api

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.