search

FasterXML / jackson-dataformat-xml

Extension for Jackson JSON processor that adds support for serializing POJOs as XML (and deserializing from XML) as an alternative to JSON
Apache License 2.0
567 stars 221 forks source link

`UnsupportedOperationException` from `createXMLStreamReader` ⇒ fall back to overloads guaranteed to work #481

Closed jglick closed 3 years ago

jglick commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.