search

FasterXML / woodstox

The gold standard Stax XML API implementation. Now at Github.
Apache License 2.0
227 stars 82 forks source link

Implementation of XMLStreamReader.getLocation() inconsistent with javadoc #156

Open klease opened 2 years ago

klease commented 2 years ago

Hello, I'm contributing to the Apache Camel project where we use woodstox as the preferred Stax implementation. In particular we rely on XMLStreamReader.getLocation().getCharacterOffset() and the fact that it returns the position at the start of the event, for example at the beginning of the start or end tag. The built-in implementation of XMLStreamReader in the JDK returns the offset at the end of the tag. The javadoc for the JDK API doesn't seem explicit. But I notice that the javadoc for the Woodstox implementation class (com.ctc.wstx.sr.BasicStreamReader) states:

public final Location getLocation() Description copied from class: StreamScanner Returns location of last properly parsed token; as per StAX specs, apparently needs to be the end of current event, which is the same as the start of the following event (or EOF if that's next).

But the implementation of BasicStreamReader returns the start location.

I'm not suggesting you change the implementation, since Camel is currently relying on it, but the javadoc should be corrected in that case. -Karen Lease

cowtowncoder commented 2 years ago

I agree that Javadoc should explain behavior as it actually exists.

I think Stax2 extension API has different methods for "current" (which would be somewhere between start and end, depending on if lazy parsing enabled) and "start" location, making it bit more explicit.

I am happy to change wording but also if you or anyone else has time, PRs are always a good way to go :) (I can review things faster than write)

Calabor-Hoc commented 1 year ago

My code also rely on the precise value of XMLStreamReader.getLocation(). The different implementation between JDK and woodstox make me anxious.

It took me quite long to find out a bug caused by these inconsistent implementation.

cowtowncoder commented 1 year ago

Ok, 2 things:

  1. Javadoc update would be appreciated, if anyone wants to do a PR
  2. If at all possible, use of Stax2-api extension (XMLStreamReader2) methods is strongly recommended -- it is implemented by Woodstox and Aalto Stax implementations only, so might not be an option -- but it has explicit definition of exactly how accessor works

It is unfortunate that the original Stax API definition left many things ambiguous, and worse, the reference implementation did not necessarily implement it exactly correctly either. And JDK implementation (sjsxp) had its share of issues for quite a while too. Over the years some behavior has become documented as the expected behavior which makes sense, but unfortunately existing behavior of Woodstox (for example) can not necessarily be easily change either due to existing user base relying on it. So it's a tricky situation.