[OPENAM-15776] Push Registration fails (QR code invalid) to register Created: 13/Dec/19 Updated: 05/May/20 Resolved: 20/Apr/20
|Fix Version/s:||126.96.36.199, 5.5.2, 7.0.0, 6.5.3|
|Reporter:||C-Weng C||Assignee:||Adam Heath|
|Remaining Estimate:||Not Specified|
|Time Spent:||Not Specified|
|Original Estimate:||Not Specified|
|Attachments:||Video MP4 (1680x898).mp4 pushissue.mp4|
|Sprint:||AM Sustaining Sprint 70, AM Sustaining Sprint 71|
Push registration fails to work after upgrading to 188.8.131.52 (working fine on 184.108.40.206). With message debug enable and when the QR code is registered this is seen in CoreSystem
or pay attention to
Details steps outlining how to recreate the issue (remove this text)
Maybe remove openam/WEB-INF/lib/woodstox-core-asl-4.3.0.jar
Cause by the Jackson dependencies changes upgrade and the Woodstox dependencies due to Jackson upgrade. See to https://github.com/FasterXML/woodstox/issues/46 if there is old woodstox.
|Comment by Adam Heath [ 13/Dec/19 ]|
I am unable to reproduce this when configuring Push auth as per the steps in https://backstage.forgerock.com/knowledge/cs/article/a42796832 (see attached video) - C-Weng C is there anything else you have configured to reproduce this issue?
re: the referenced https://github.com/FasterXML/woodstox/issues/46 - I think we should have already covered this as part of the changes noted and discussed in https://bugster.forgerock.org/jira/browse/OPENAM-15000 - which pin the version of woodstox-core to 5.3.0
|Comment by Jonathan Thomas [ 13/Dec/19 ]|
With NoSuchMethodError one thing to check is clearing the expanded war and container caches before the upgrade .
|Comment by C-Weng C [ 13/Dec/19 ]|
The issue is that a clean install will also show. Anyway, i suppose it depends how the classloading woodstox get loaded first on which is resolved.
Maybe you can dump this to the openam and test this to see what loaded? The issue is not about pinning to woodstox-5.3.0 but that there are duplicate woodstox in the openam.war and in my case the 4.3.0 version is loaded.
|Comment by Adam Heath [ 16/Dec/19 ]|
Thanks for the additional info C-Weng C - I can confirm I loaded the test.jsp you provided in your last update and noticed no errors (just a `class com.ctc.wstx.evt.CompactStartElement Stax Event #1` message)
I think I understand a little more about the issue now though given your notes above. It appears that there were some changes introduce in woodstox-core versions after 5.1.0 that have resulted in APIs that are non-binary compatible with earlier versions (see https://github.com/FasterXML/woodstox/issues/46 which you referenced earlier)
For now, the only way I can see us resolving this would be to pin this version back to a pre 5.1.0 version (5.0.3 is the latest stable) so that all the related libraries (woodstox-core 5.0.3 and woodstox-core-asl 4.3.0) remain binary compatible. However, this would mean that we miss out on all the bugfixes which have been provided in later 5.x releases https://github.com/FasterXML/woodstox/wiki/Woodstox-Release-5.3 and this may be something that we need to monitor in the future should other items require later versions of this dependency.
|Comment by Adam Heath [ 19/Dec/19 ]|
Having done a good amount of investigation into this am going to add a summary of the issue here so that it is hopefully clearer for anyone else looking at this PR or coming from the JIRA.
The exceptions being observed are essentially boiling down to incompatibilities between the versions of woodstox (either woodstox-core (covering 5.x and above, or woodstox-core-asl - legacy covering 4.x and below) and the stax2-api libraries.
Since the update to jackson 2.9.10/2.10 we have had to bump and pin the versions to the following in all branches where restlet is still present (so 6.5 and earlier):
This has then caused issues in the instances where the woodstox-core-asl library is still in use within the project (e.g. currently within restlet, and via transitive dependencies in jaxws-rt and wss4j.policy), as woodstox-core-asl is expecting to use stax2-api 3.1.4 which has now been bumped by the above to 4.2...and so causing the exception currently being experienced.
4.x of stax2-api was introduced as required by woodstox core from version 5.1.0 onwards, and woodstox core >5.1.0 was introduced in the changes involved in updating jackson to 2.9.10 and above.
It has not been possible to pin the versions to the older 5.0.3 and 3.1.4 versions respectively, as despite these being compatible with one another there will potentially be issues for restlet which is now expecting the newer stax2-api (4.2) via a transitive dependency pulled in from jackson 2.10, see:
Leaving the versions at 5.3.0 and 4.2 will require us to add an exclusion against the org.restlet.ext.jackson dependency (as seen in this attached PR) which will then prevent the conflicting versions of stax2-api from being referenced. However, there are still other libraries which pull in this dependency (mentioned above: jaxws-rt and wss4j.policy) and so at the moment, it is unclear if having the remaining inclusions of woodstox-core-asl will still cause these issues to be present. C-Weng C is this something you may be able to help confirm with your set-up? (I am still unable to reproduce this here)
The underlying issue is summed up well (eventually) in a github issue raised against jackson-data-format here: https://github.com/FasterXML/jackson-dataformat-xml/issues/340 - with the key point being highlighted as the non-binary compatible change to the EmptyIterator.getInstance() method - and quite frankly, it all looks to be a bit of a mess caused by these version updates in jackson
|Comment by Adam Heath [ 09/Jan/20 ]|
During testing, this issue can not be avoided whenever the woodstox-core-asl dependency is present in the resulting WEB-INF/lib - because of this the approach to fix this has now been to exclude the legacy woodstox-core-asl dependency where it is still currently being referenced in the project. This should be possible due to us already including the updated woodstox-core 5.2.x throughout alongside the existing bump to the compatible stax2-api 4.2 mentioned above - this is also similar to the approach taken in
|Comment by Ľubomír Mlích [ 10/Feb/20 ]|
No verification as I'm not able to reproduce
|Comment by Adam Heath [ 20/Apr/20 ]|
Re-opening to clarify the bug title and not explicitly mention AM 220.127.116.11 (As other versions are impacted) - this title appears in release notes