Stop depending on ASF 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 #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 #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 1348a08#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 zhfeng/jdk11u-dev@7e6cad7 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?

[jamesnetherton]

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

Runtime unfortunately 😞

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 apache/xalan-java#186

[ppalaga]

Does anybody remember, what is different in Xalan, that the setting of the translet class name/package works there?

[ppalaga]

Does anybody remember, what is different in Xalan, that the setting of the translet class name/package works there?

ASF Xalan does not initialize XSLTC._packageName with any default value.

[zhfeng]

yeah, so the classname would be without package, see https://github.com/apache/xalan-java/blob/master/xalan/src/main/java/org/apache/xalan/xsltc/compiler/XSLTC.java#L547-L556

But in the JDK, the default value of packageName is die.verwandlung and it is invoking setClassName before setPackageName. Then it causes the class name is alway with the die_verwandlung.

I had https://github.com/zhfeng/jdk-xalan-issue-reproducer before.

[ppalaga]

I wrote down this report for OpenJDK. Could you guys please review it before we submit it?

---->8-----

translet-name ignored, when package-name is also set via TransformerFactory.setAttribute

Background

GraalVM native executables do not allow loading classes at runtime due to the closed world assumption. To make XSLT work with them, we generate XSLT Translet classes at build time and let native-image compile them into the native executable. For that to work reliably, we need to be able to set the name of the generated class so that we are then able to find the class and pass it to the native compiler.

Steps to reproduce

To generate a Translet class for a given XSL file, we perform steps similar to the following:

import javax.xml.transform.TransformerFactory;
import javax.xml.transform.stream.StreamSource;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;

public class Main {
        public static void main(String[] args) throws Exception {
            TransformerFactory tf = TransformerFactory.newInstance();
    
            tf.setAttribute("generate-translet", true);
            tf.setAttribute("translet-name", "MyTranslet");
            tf.setAttribute("package-name", "org.acme");
            tf.setAttribute("destination-directory", "test");
    
            Path test = Path.of("test");
            if (Files.exists(test)) {
                Files.walk(test).sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
            }

            File xslFile = new File(args[0]);
            tf.newTemplates(new StreamSource(Files.newInputStream(xslFile.toPath())));

            Files.list(Path.of("test/org/acme")).forEach(System.out::println);
        }
}

The reproducer code can be taken from https://github.com/zhfeng/jdk-xalan-issue-reproducer .

When this program is compiled through javac Main.java and run via java Main test.xsl, where test.xsl is any simple XSL file, such as

<?xml version="1.0" encoding="iso-8859-1"?>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
    <xsl:output method="xml" version="1.0" encoding="UTF-8"/>

    <xsl:template match="/request">
        <xsl:variable name="name" select="//name/text()"/>
        <response>
            <message>
                <xsl:value-of select="concat('Hello, ', $name)"/>
            </message>
        </response>
    </xsl:template>
</xsl:stylesheet>

then, we expect to find the generated translet file under test/org/acme/MyTranslet.class.

In reality, the generated translet is under test/org/acme/die_verwandlung.class.

Analysis

The execution flow goes via com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl.newTemplates(Source), where a new com.sun.org.apache.xalan.internal.xsltc.trax.XSLTC is created and its setClassName(String) and setPackageName(String) are called.

XSLTC.setClassName(String) does some sanitization of the passed className and if the _packageName field is set, it sets the _className field to _packageName + '.' + name.

Because XSLTC._packageName is initialized to "die.verwandlung", then, after the first call of setClassName("MyTranslet"), the value of _packageName is "die.verwandlung.MyTranslet".

The XSLTC.setPackageName("org.acme") called afterwards, first sets the _packageName field to the passed value and then, if _className != null, it calls setClassName(_className).

In our situation, it effectively means calling setClassName("die.verwandlung.MyTranslet").

The sanitization of the passed value done within this second setClassName() call transforms "die.verwandlung.MyTranslet" into "die_verwandlung".

Afterwards, the _className field is set to _packageName + '.' + name which is "org.acme" + '.' + "die_verwandlung" in our case.

Observation: the ASF Xalan does not initialize XSLTC._packageName to "die.verwandlung" and therefore the reproducer code works as expected there.

Possible solutions

A. In TransformerFactoryImpl.newTemplates(Source), call XSLTC.setPackageName(String) before XSLTC.setClassName(String).

B. In XSLTC.setPackageName(String), instead of calling setClassName(_className), pass only the simple class name extracted from the _className field to setClassName(String).

C. Make XSLTC.setClassName(String) throw an exception when it is called with a className containig ., / or \; document that it expects a simple name, document that it sets _className to a fully qualified name when _packageName is set; move all sanitization to callers of XSLTC.setClassName(String). (This implies solution B).

[zhfeng]

Thanks @ppalaga - It looks good to me!

So what is the next step? can we put the fix to the upstream jdk?

[ppalaga]

I have added solution C. Which is IMO the proper solution, but I am not sure how much bug-backwards-compatible the JDK aims to be. I guess, we should report the issue on [email protected] and maybe discuss the solution there, before sending a PR? WDYT @galderz?

[ppalaga]

@galderz I wonder how much is XSLTC.setClassName(String) held a part of the public API? It is in an internal package, so changing its contract via solution C could be OK?

[zhfeng]

I just open a PR

• 8344925: translet-name ignored, when package-name is also set via TransformerFactory.setAttribute openjdk/jdk#22425

[zhfeng]

The fix has been included openjdk/jdk@3989a19

[ppalaga]

The fix has been included openjdk/jdk@3989a19

That's a phenomenal news! Thanks @zhfeng @galderz and anybody else involded!

@galderz can you see any chance that the fix gets backported to JDK 21 and 17? Can we help somehow for it to happen?

[galderz]

@ppalaga I'll ask around and see what can be done

[galderz]

@ppalaga I asked around and the best approach would be to backport it first to JDK 24 then to JDK 21. I doubt it'll get backported to JDK 17 but if you really need that you'd have to explain why specifically.

[galderz]

I will take care of the JDK 24 and 21 backports

[galderz]

FYI JDK 24 backport: openjdk/jdk24u#73 Once that goes through I'll send the one for JDK 21

[ppalaga]

@ppalaga I asked around and the best approach would be to backport it first to JDK 24 then to JDK 21.

Having it in 21 will be awesome, thanks a lot!

I doubt it'll get backported to JDK 17 but if you really need that you'd have to explain why specifically.

The whole Quarkiverse is on Java baseline 17 currently. Not sure it is going to change to 21 any time soon. I asked about that on Quarkus Zulip Max replied, that Quarkus 3.x's baseline is going to stay Java 17.

[galderz]

The whole Quarkiverse is on Java baseline 17 currently. Not sure it is going to change to 21 any time soon. I asked about that on Quarkus Zulip Max replied, that Quarkus 3.x's baseline is going to stay Java 17.

It's unclear to me in this issue and I'm not familiar how this is being used, but could you when this xlt gets generated and therefore needs the fix? Is it during native image execution (e.g. something that gets generated via some kind of build time initialization that runs inside the native image JDK), or is it by Quarkus build before native image gets invoked (e.g. the JDK that compiles the source code and runs maven/gradle)? Or some other phase?

[galderz]

Having it in 21 will be awesome, thanks a lot!

JDK 24 backport has gone in, it'll be part in 24.0.2 (July).

[galderz]

JDK 21 backport PR: openjdk/jdk21u-dev#1412

[ppalaga]

Is it during native image execution (e.g. something that gets generated via some kind of build time initialization that runs inside the native image JDK), or is it by Quarkus build before native image gets invoked (e.g. the JDK that compiles the source code and runs maven/gradle)? Or some other phase?

We generate the translet class during Quarkus build phase (where only the standard JDK is involved). During that process we create a mapping from XLST URIs to translet FQ class names. That's where the unique and predictable name under which javax.xml.transform.TransformerFactory creates the translet class matters.
Further, at Quarkus build time we take the translet class bits and push it to Quarkus via GeneratedClassBuildItem.
Then, when the native executable is running, we always configure the TransformerFactory to use the translet class available in the
native image, so that native image does not need to generate and load it.

So there is no problem with native image generation or runtime. The only problem we have is with the vanilla JDK not storing the generated translet classes where we want it to.

[galderz]

The whole Quarkiverse is on Java baseline 17 currently. Not sure it is going to change to 21 any time soon. I asked about that on Quarkus Zulip Max replied, that Quarkus 3.x's baseline is going to stay Java 17.

I've had a chat with the team and JDK 17 is pretty much done and only very critical fixes get backported there. For the time being we will backport it to JDK 21 and we can reevaluate again, if necessary, once the backport there has had some time running in production for end users.