Skip to content

Conversation

@jglick
Copy link

@jglick jglick commented Jul 3, 2021

Alternative to #480. Rather than throwing an error, or forcing clients to use a new API, automatically and silently fall back to the provided StAX implementation (presumably from the JRE) even if it is not as nice as Woodstox.

Untested.

@basil
Copy link

basil commented Jul 3, 2021

As detailed in FasterXML/jackson-dataformat-xml#480 (comment) I tested the ByteArrayInputStream change in a realistic environment: Jenkins core 2.300, where a plugin's thread context classloader (which is core's web application class loader) does not have Woodstox, but the plugin's classloader does have Woodstox. Before this change, I got the UnsupportedOperationException. After this change, it worked. I verified by stepping through the change in the debugger that we were successfully catching the UnsupportedOperationException and that retrying against the JRE's built-in class worked.

@cowtowncoder
Copy link
Member

I think I could work with this; I think it may be possible to dynamically detect likelihood of support.
Stax2 api has wrapper used for non-native implementations, and such wrapping probably means stax2 byte source won't work...

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 4, 2021

Thank you @jglick and @basil! I think that this behavior (exception) was a flaw -- although it is strongly recommended Woodstox or other Stax2-implementing library is used, I'd like to avoid unnecessary failures for other implementations, esp. one bundled in JDK. So good catch, thank you reporting it.

I changed implementation slightly via #482, to avoid throwing exception, since I figured an alternative based on checking for XMLInputFactory2 (stax2 api); the effect should be same here. Also added a test to verify this.

I guess there is still open question on whether #480 (or something along those lines) should be added too, so I'll leave that open. But I think I can close this PR.

Fix should go in 2.12.4 whenever I release that (probably to after 2.13.0-rc1), hopefully within a month -- I need to release a full set and it's likely the last 2.12 patch.

@jglick jglick deleted the UnsupportedOperationException branch July 6, 2021 13:39
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.

3 participants