Remove use of Xalan

[JiriOndrusek]

Follows https://issues.apache.org/jira/browse/CAMEL-18346

Java has built-in support for TransformerFactory and XPathFactory. This means most apps that use Xalan-J can readily switch away. Saxon-HE is another well maintained alternative.

Xalan is directly used in camel-quarkus-support-xalan. It should be possible to remove xalan dependency and use java support instead.

[lburgazzoli]

I recall that the provided transformer factory had issues for which it was not possible to leverage java code generation hence the need to bring that dependency.

I don't recall the issue in detail but there were an issue with the generated classes names.

It may have been fixed.

[JiriOndrusek]

@lburgazzoli I removed xalan and then built whole Camel-quarkus without any profile. Build finished successfully. I created PR https://github.com/apache/camel-quarkus/pull/4066 and we will see whether there is a build failure or not.

[lburgazzoli]

good to know, so things have been fixed finally :)

[JiriOndrusek]

Ci build failed (in xml tests) with:

Caused by: javax.xml.transform.TransformerConfigurationException: Cannot find class 'org.apache.camel.quarkus.component.xslt.generated.HtmlTransform'.
2022-09-02T10:44:40.1852572Z    at java.xml/com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl.newTemplates(TransformerFactoryImpl.java:903)
2022-09-02T10:44:40.1853455Z    at org.apache.camel.quarkus.support.xalan.XalanTransformerFactory.newTemplates(XalanTransformerFactory.java:70)
2022-09-02T10:44:40.1854276Z    at org.apache.camel.component.xslt.XsltBuilder.setTransformerSource(XsltBuilder.java:345)
2022-09-02T10:44:40.1855004Z    at org.apache.camel.component.xslt.XsltEndpoint.loadResource(XsltEndpoint.java:326)
2022-09-02T10:44:40.1855675Z    at org.apache.camel.component.xslt.XsltEndpoint.doInit(XsltEndpoint.java:342)
2022-09-02T10:44:40.1856270Z    at org.apache.camel.support.service.BaseService.init(BaseService.java:83)

@lburgazzoli It seems to be the issue mentioned by you in your previous comment. Unfortunately I did't run locally this extension tests (xmlsecurity is successful). I'll try to search a fix.

[ppalaga]

Removing Xalan might be tricky. Not sure it is worth the effort. Note that @zhfeng works on https://github.com/apache/camel-quarkus/pull/4018 - so please sync with him to avoid conflicts

[JiriOndrusek]

Removing Xalan might be tricky. Not sure it is worth the effort. Note that @zhfeng works on #4018 - so please sync with him to avoid conflicts

Thanks for the information, I'm asking @zhfeng about this problem.

[JiriOndrusek]

I don't understand a lot of the code behind the xslt, but I found this commit https://github.com/apache/camel-quarkus/commit/1348a08fc4c389a5ebaf79da0ee61e4333931ab2#diff-3f81d95136449165870ef59323dfe2d77527e7ff78ba506f4a81b404d608611f which seems to be doing the required functionality with the xalan library.

I debugged a little bit the same processes with jdk library. I made the TransformFactoryImpl to create a jar file with the generated classes and then looked at it and found weird thing. The class generated in the factory was org.apache.camel.quarkus.component.xslt.generated.HtmlTransform, but in the jar in tmp folder there is a some kind of placeholder instead of the name:

@ppalaga @lburgazzoli I just wanted to share this. I might be missing something, but from the debugging it seems like there is a placeholder, which is replaced by the real value, but it does't work correctly. If my observation is correct, then it explains why there is a ClassNotFoundException. If the behavior could be changed to generate a correct name, then it might be working correctly.

[lburgazzoli]

I don't recall all the details but that is pretty much where I stopped and I switched to xalan, I think I were able to get down to the real issue but it's been long time. I recall that there where a bad handling of translet-name and package-name that led to one value overriding/resetting the other.

[JiriOndrusek]

I don't recall all the details but that is pretty much where I stopped and I switched to xalan, I think I were able to get down to the real issue but it's been long time. I recall that there where a bad handling of translet-name and package-name that led to one value overriding/resetting the other.

Exactly! The issue is still there:

When package is provided, class name is overwritten. Into *.die_verwadlung (because the local _className contanins die_verwandling prefix)

[lburgazzoli]

I guess we need to open an openjdk bug or stay with xalan

[JiriOndrusek]

Therefore it might not be possible to remove xalan. @ppalaga should I define the version of xalan in camel-qaurkes and keep it there? (the version is inherited from the Camel which removed it). To make camel-main work.

EDITED:

I'm going to fix camel-main by defining the version of xalan in camel-quarkus.

[zhfeng]

Nice investigating! The XSLTC has the defualt _packageName which is die.verwandlung https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/compiler/XSLTC.java#L114

And when generating the translet class, TransformerFactoryImpl invokes xsltc.setClassName before xsltc.setPackageName https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/TransformerFactoryImpl.java#L1006-L1024 That causes the className contains die_verwandlung prefix. So a possible fix in openjdk could be to do setPackagesName before setClassName.

[zhfeng]

I just test with https://github.com/zhfeng/jdk11u-dev/commit/7e6cad7737d15644600efbbdd8fb1f6be46a672e locally and it works. Is there any way we can do to fix in upstream openjdk?

[JiriOndrusek]

I just test with zhfeng/jdk11u-dev@7e6cad7 locally and it works. Is there any way we can do to fix in upstream openjdk?

Great news! My locally built java did not work successfully with this problem, but I probably miss-configured something.

EDITED:

Fix works, I verified it.

[ppalaga]

I just test with zhfeng/jdk11u-dev@7e6cad7 locally and it works. Is there any way we can do to fix in upstream openjdk?

@galderz any hint how to proceed?

[galderz]

@ppalaga Any upstream openjdk changes will require a bug and someone to sponsor it, unless @zhfeng you're already an openjdk committer. @zhfeng go ahead and create a bug in the openjdk bug system and then I can ask around to see who could sponsor it.

[zhfeng]

Thanks @galderz and I'm trying to create a simple reproducer of this issue. Btw, https://bugs.openjdk.org/ is the right place to report?

[galderz]

@zhfeng Yeah that's the right place.

[zhfeng]

@galderz But it needs login to create a issue, I have no such credential unfortunately.

[galderz]

@zhfeng Ah yes. Could you please write up a bug report here? I can then ask someone to verify it and we can create the OpenJDK issue as well as help with proposing a fix.

[zhfeng]

@galderz I just revisit this issue and write a simple reproducer https://github.com/zhfeng/jdk-xalan-issue-reproducer. Can you take a look?

[galderz]

Thanks @zhfeng for the reproducer. I'll have a look.

[galderz]

@zhfeng Thanks a lot for building the reproducer. It worked for me.

I also checked the proposed fix and it looks good to me but I'm in no way an expert in this area. Both XSLTC.setClassName() and XSLTC.setPackageName() depend on each other so at a first glance either could be called first, but in their current impl, if setClassName is called first, it wrongly sets the class name with the package name to the hardcoded default in XSLTC itself (die.verwandlung). You could then undo things when calling setPackageName but that would complicate things. Instead calling XSLTC.setPackageName() first makes things behave more naturally. You change the default to the set package and then you set the class name in setClassName.

At this stage, you can do either of two things: you could first ask in the core-libs-dev OpenJDK list or if you prefer you could go open a PR in github.com/openjdk/jdk. To open the PR you'll need OCA clearance. You can find information about OCA clearance and other topics in the OpenJDK contributing guide.

I would also recommend that you run the tier1 tests as well as the jaxp/xml tests (they are not part of tier1). You can run these extra tests via the :jaxp_all test group. See Testing the JDK for more details.

[zhfeng]

Thanks @galderz I will run these tests at first.

[jamesnetherton]

Out of curiosity, I was able to work around this issue by transforming the XSLT generated classes to update the incorrect class & package names. I got the XSLT JVM and native tests passing without the xalan dependency.

The downside is that you need some additional --add-exports for packages in the java.xml module.

[zhfeng]

@jamesnetherton it seems good. --add-exports is needed at build time?

[zhfeng]

Can you share your solution?

[jamesnetherton]

@jamesnetherton it seems good. --add-exports is needed at build time?

Runtime unfortunately :disappointed:

Can you share your solution?

I'll try to polish it up today and link to my branch.

[jamesnetherton]

Here's a branch with a rough POC:

https://github.com/jamesnetherton/camel-quarkus/tree/xalan-remove

Not sure if a lot of the stuff in the xalan extension could just be removed or folded into the xslt extension.

[zhfeng]

Thanks @jamesnetherton !

I'm wonder why we need --add-exports and if we get a fix in jdk in the future, will it still be needed?

[jamesnetherton]

will it still be needed?

I think so, because the XSLTC generated classes refer to internal bits from javax.xml, and they are not exported by default.

[turing85]

Hi! Maybe this if off-topic, but if we cannot remove Xalan, can we at least upgrade Xalan to version 2.7.3?

[zhfeng]

@turing85 Unfortunately the fix of XALANJ-2664 had not been included in 2.7.3. see https://github.com/apache/xalan-java/pull/186