Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
klease opened this issue Sep 8, 2022 · 3 comments
Open
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project pr-welcome Issue for which progress most likely if someone submits a Pull Request

Comments

@klease
Copy link

klease commented Sep 8, 2022

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
Copy link
Member

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
Copy link

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 cowtowncoder added good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project pr-welcome Issue for which progress most likely if someone submits a Pull Request labels Dec 4, 2022
@cowtowncoder
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project pr-welcome Issue for which progress most likely if someone submits a Pull Request
Projects
None yet
Development

No branches or pull requests

3 participants