Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump xalan from version 2.7.2 to version 2.7.3 #4902

Open
wants to merge 1 commit into
base: 2.13.x
Choose a base branch
from

Conversation

turing85
Copy link
Contributor

Bump xalan from version 2.7.2 to version 2.7.3

@turing85 turing85 force-pushed the feature/2.13.x/bump-xalan-version branch from a3b174c to 44f7dc1 Compare May 15, 2023 21:29
@turing85
Copy link
Contributor Author

@oscerd could you re-approve the workflows? I had to force-push some changes in order to make the pipeline succeed.

@zhfeng
Copy link
Contributor

zhfeng commented May 16, 2023

It seem that there are java.lang.ClassNotFoundException: org.apache.xml.serializer.OutputPropertiesFactory

@jamesnetherton
Copy link
Contributor

It seem that there are java.lang.ClassNotFoundException: org.apache.xml.serializer.OutputPropertiesFactory

The 2.7.3 POM is a bit screwed up compared with 2.7.2. It is missing dependencies.

https://issues.apache.org/jira/browse/XALANJ-2649

@turing85
Copy link
Contributor Author

Oh nice... so... should we close this PR and wait for an update instead?

@jamesnetherton
Copy link
Contributor

Oh nice... so... should we close this PR and wait for an update instead?

It's not clear to me whether they are going to patch it or not....

I think we could work around it by adding xalan:serializer into poms/bom/pom.xml and also into extensions-support/xalan/runtime/pom.xml. Afterwards, if you run mvn clean install in poms it'll regenerate the BOM files.

@turing85
Copy link
Contributor Author

Oh nice... so... should we close this PR and wait for an update instead?

It's not clear to me whether they are going to patch it or not....

I think we could work around it by adding xalan:serializer into poms/bom/pom.xml and also into extensions-support/xalan/runtime/pom.xml. Afterwards, if you run mvn clean install in poms it'll regenerate the BOM files.

Okay, will update my PR as soon as I have some time. There is, however, a major concern here: CVE-2022-34169. For me, it is unclear whether the issue is actually fixed in 2.7.3, or whether it is just "gone" due to the missing dependencies.

@turing85 turing85 force-pushed the feature/2.13.x/bump-xalan-version branch from 44f7dc1 to 1d85d7a Compare May 16, 2023 17:20
@turing85
Copy link
Contributor Author

@jamesnetherton @oscerd @zhfeng Updated the PR, xalan:serializer is now in the dependencies. Could you re-approve to trigger the pipeline run?

- Add xalan:serializer dependency since it is missing from xalan's pom
@turing85 turing85 force-pushed the feature/2.13.x/bump-xalan-version branch from 1d85d7a to 1d1cf8e Compare May 16, 2023 17:47
@turing85
Copy link
Contributor Author

... welp...

@ppalaga
Copy link
Contributor

ppalaga commented May 17, 2023

There is, however, a major concern here: CVE-2022-34169. For me, it is unclear whether the issue is actually fixed in 2.7.3, or whether it is just "gone" due to the missing dependencies.

The fix commit seems to be this one apache/xalan-java@2e60d0a
That's definitely in Xalan.

@turing85
Copy link
Contributor Author

There is, however, a major concern here: CVE-2022-34169. For me, it is unclear whether the issue is actually fixed in 2.7.3, or whether it is just "gone" due to the missing dependencies.

The fix commit seems to be this one apache/xalan-java@2e60d0a That's definitely in Xalan.

Okay nice. Only question is now: what is missing so that the pipeline succeeds?

@ppalaga
Copy link
Contributor

ppalaga commented May 17, 2023

what is missing so that the pipeline succeeds?

Let me have a look

@ppalaga
Copy link
Contributor

ppalaga commented May 17, 2023

I was able to do some hacks to make the XML security test pass - see https://github.com/ppalaga/camel-quarkus/commits/pr4902
I unfortunately have no more time fix the failing XML test.

@zhfeng
Copy link
Contributor

zhfeng commented May 23, 2023

The XML test are weired. They are working in JVM mode but throwing the Exception during native building.

[ERROR] org/apache/camel/quarkus/component/xslt/generated/ClasspathTransform (wrong name: org/apache/xalan/xsltc/runtime/AbstractTranslet)
javax.xml.transform.TransformerException: org/apache/camel/quarkus/component/xslt/generated/ClasspathTransform (wrong name: org/apache/xalan/xsltc/runtime/AbstractTranslet)
    at org.apache.xalan.xsltc.trax.TransformerFactoryImpl.passErrorsToListener (TransformerFactoryImpl.java:661)
    at org.apache.xalan.xsltc.trax.TransformerFactoryImpl.newTemplates (TransformerFactoryImpl.java:836)
    at org.apache.camel.quarkus.support.xalan.XalanTransformerFactory.newTemplates (XalanTransformerFactory.java:70)
    at org.apache.camel.quarkus.component.xslt.deployment.XsltProcessor.xsltResources (XsltProcessor.java:119)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:568)
    at io.quarkus.deployment.ExtensionLoader$3.execute (ExtensionLoader.java:909)
    at io.quarkus.builder.BuildContext.run (BuildContext.java:281)
    at org.jboss.threads.ContextHandler$1.runWith (ContextHandler.java:18)
    at org.jboss.threads.EnhancedQueueExecutor$Task.run (EnhancedQueueExecutor.java:2449)
    at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run (EnhancedQueueExecutor.java:1478)
    at java.lang.Thread.run (Thread.java:833)
    at org.jboss.threads.JBossThread.run (JBossThread.java:501)
[ERROR] Could not compile stylesheet
javax.xml.transform.TransformerConfigurationException: Could not compile stylesheet
    at org.apache.xalan.xsltc.trax.TransformerFactoryImpl.newTemplates (TransformerFactoryImpl.java:832)
    at org.apache.camel.quarkus.support.xalan.XalanTransformerFactory.newTemplates (XalanTransformerFactory.java:70)
    at org.apache.camel.quarkus.component.xslt.deployment.XsltProcessor.xsltResources (XsltProcessor.java:119)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:568)
    at io.quarkus.deployment.ExtensionLoader$3.execute (ExtensionLoader.java:909)
    at io.quarkus.builder.BuildContext.run (BuildContext.java:281)
    at org.jboss.threads.ContextHandler$1.runWith (ContextHandler.java:18)
    at org.jboss.threads.EnhancedQueueExecutor$Task.run (EnhancedQueueExecutor.java:2449)
    at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run (EnhancedQueueExecutor.java:1478)
    at java.lang.Thread.run (Thread.java:833)
    at org.jboss.threads.JBossThread.run (JBossThread.java:501)

I will take a look.

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

Well, it seems that xalan 2.7.3 added a new checking before dumping a generated translet class. See https://github.com/apache/xalan-java/blame/master/src/org/apache/xalan/xsltc/compiler/XSLTC.java#L860-L865

 byte[] classByteArray = clazz.getBytes();
 ByteArrayClassLoader classLoader = new ByteArrayClassLoader(classByteArray);
 Class clz = classLoader.findClass(clazz.getClassName());

And ByteArrayClassLoader can not find org.apache.xalan.xsltc.runtime.AbstractTranslet during the quarkus-maven-plugin runs AugmentAction. I guess that it uses a different class loader in quarkus-maven-plugin and ByteArrayClassLoader is inherited from SystemClassLoader.

I have no idea how to fix it and don't undertand the motivation for the checking in XSLTC.

@turing85
Copy link
Contributor Author

turing85 commented Jun 2, 2023

Sooo... what's our course of action now? Xalan 2.7.2 is vulnerable to https://cve.circl.lu/cve/CVE-2022-34169 and 2.7.3 does not seem to work.

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

@zhfeng could you please try changing the constructor of ByteArrayClassLoader so that it uses something like super(Thread.currentThread().getContextClassLoader() != null ? Thread.currentThread().getContextClassLoader() : ClassLoader.getSystemClassLoader())? If it works for us, we should file a Xalan issue and send a PR.

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

something like super(Thread.currentThread().getContextClassLoader() != null ? Thread.currentThread().getContextClassLoader() : ClassLoader.getSystemClassLoader())

Or even better super(Thread.currentThread().getContextClassLoader() != null ? XSLTC.class.getClassLoader() : ClassLoader.getSystemClassLoader())
super(Thread.currentThread().getContextClassLoader() != null ? Thread.currentThread().getContextClassLoader() : XSLTC.class.getClassLoader())

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

@ppalaga yeah, I tried such changes and it works with TCCL.

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

Let's propose it in Xalan then. Should I help with formulating why we need it?

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

Yeah, please file an issue on XALANJ ?

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

You mean I should file an issue on Xalan? - no problem, let me do it.

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

super(Thread.currentThread().getContextClassLoader() != null ? Thread.currentThread().getContextClassLoader() : XSLTC.class.getClassLoader())

@ppalaga I'm just curious in what case the TCCL is null?

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

@ppalaga I'm just curious in what case the TCCL is null?

When it is not set via Thread.currentThread().setContextClassLoader(...) - it is quite a normal situation.

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

@ppalaga
Copy link
Contributor

ppalaga commented Jun 2, 2023

I filed https://issues.apache.org/jira/browse/XALANJ-2664

Would you like to care for sending the PR, @zhfeng ?

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

Thanks @ppalaga and I will prepare a PR.

@zhfeng
Copy link
Contributor

zhfeng commented Jun 2, 2023

@ppalaga I'm just curious in what case the TCCL is null?

When it is not set via Thread.currentThread().setContextClassLoader(...) - it is quite a normal situation.

IIRC, the default value of TCCL is System Class Loader. So if there is no setContextClassLoader(null) explicitly, it should be always not empty?

@zhfeng
Copy link
Contributor

zhfeng commented Oct 28, 2024

We revisit the following issue and will try to put the fix to the upstream jdk

@zhfeng zhfeng added the do-not-merge/hold Indicates that a PR should not be merged because there are unresolved issues pending label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not be merged because there are unresolved issues pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants