Bug 66171 - remove xalan dependency due to it being end of life
Summary: remove xalan dependency due to it being end of life
Status: NEW
Alias: None
Product: JMeter - Now in Github
Classification: Unclassified
Component: Main (show other bugs)
Version: 5.5
Hardware: All All
: P2 normal (vote)
Target Milestone: JMETER_5.5
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-19 19:26 UTC by PJ Fanning
Modified: 2022-07-23 13:33 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description PJ Fanning 2022-07-19 19:26:46 UTC
Xalan is no longer supported.

https://lists.apache.org/thread/s8kjny5270ssfcp46v0fl39lk98987w7

It is better to use JAXP TransformerFactory than using xalan directly. If you add xalan dependency just to ensure that you have a JAXP compliant transformer on the classpath, this is unnecessary - the Java runtime has a built-in implementation.

Xalan dependency in:
https://mvnrepository.com/artifact/org.apache.jmeter/ApacheJMeter_core/5.5
Comment 1 Felix Schumacher 2022-07-20 14:12:09 UTC
Care to provide a patch/PR?
Comment 2 PJ Fanning 2022-07-20 14:33:08 UTC
I'm a member of the ASF Security committee and I'm just trying to nudge PMCs to keep their dependencies up to date. I do a lot of PRs for ASF teams but I'm getting to the stage where I'm getting a bit fed up dealing with all the different build systems and almost inevitable build failures.
Comment 3 Felix Schumacher 2022-07-20 14:48:24 UTC
Fair enough.
Comment 4 Vladimir Sitnikov 2022-07-20 14:55:22 UTC
We have public class PropertiesBasedPrefixResolver extends org.apache.xml.utils.PrefixResolverDefault, so we have non-trivial usage of xalan.  It does not sound like "just remove runtime-only dependency"
Comment 5 PJ Fanning 2022-07-21 16:53:47 UTC
Without xalan, it looks like XPathUtil would need to be rewritten - the 3 imports starting with https://github.com/apache/jmeter/blob/master/src/core/src/main/java/org/apache/jmeter/util/XPathUtil.java#L52 will be lost.

XPathFactory in the Java runtime and Saxon's XPath support would be good alternatives.
Comment 6 Felix Schumacher 2022-07-21 18:01:48 UTC
I tried to get rid of the xalan dependency by removing it from the usual places in the gradle files, but it seems that other libraries build steps (ant?) have dependencies on them and bring xalan back alive.

I tried the gradle tasks allDependencies and it seems to get pulled back by at least org.apache.xmlgraphics:batik-dom

Other than that, I found an error in our test suite, that produces failures on Saxon as a TransformerFactory and will fix it soon (we set System.err to null and might even miss to set it back). Apart from that, I found out, that Saxon HE does not expose options to customize the amount of space to indent XML on output, which will again break our tests, that assume otherwise :)
Comment 7 Felix Schumacher 2022-07-22 18:53:14 UTC
Another problem I found, is that we guess the return of xpath expressions. The standard XPath api has no such capability. At least, I haven't found it yet.

Such guessing takes place in XPathUtil#computeAssertionResult for example.
Comment 8 PJ Fanning 2022-07-22 19:23:08 UTC
https://docs.oracle.com/javase/9/docs/api/javax/xml/xpath/XPath.html#evaluateExpression-java.lang.String-org.xml.sax.InputSource- was only added in Java 9 but would do exactly what you need - see https://docs.oracle.com/javase/9/docs/api/javax/xml/xpath/XPathEvaluationResult.html

The Saxon S9API may be able to do this too but I'm not as familiar with this API - I just know that it is a separate API that is not based on javax API (but Saxon supports javax API too).
Comment 9 Vladimir Sitnikov 2022-07-22 19:58:30 UTC
I just wonder: is xalan-j really that bad?
What if we just fix the CVE in question and release a newer Xalan version?

Then **everybody** would benefit from that.
Comment 10 PJ Fanning 2022-07-22 20:04:11 UTC
Xalan is being putting in the ASF attic. The PMC is inactive. Noone benefits from keeping a project alive when there are not enough maintainers. Java runtime XPath support and Saxon are very valid alternatives.
Comment 12 Vladimir Sitnikov 2022-07-23 09:26:43 UTC
> Noone benefits from keeping a project alive when there are not enough maintainers

The CVE is trivial to fix, and it would help A LOT of users who depend on xalan today. They will be able to drop-replace xalan.jar with a newer version
and it would resolve the CVE.

I agree migrating to modern alternatives makes sense, however,
the migration requires testing which is hard to do for legacy projects.

I did try building xalan-j, and it worked for me, so I think fixing CVE and releasing one more xalan version is viable:
https://lists.apache.org/thread/9jdjkhjk3mxjzzfd098bn0mxqyyovg2b
Comment 13 PJ Fanning 2022-07-23 12:09:31 UTC
I have done a PR that is just a POC showing that xalan can probably be removed. https://github.com/apache/jmeter/pull/721

I can continue work on this POC - but I would like to know if it is worthwhile. It seems Jmeter team want to keep xalan, even if it is possible to remove it.
Comment 14 Felix Schumacher 2022-07-23 13:33:03 UTC
Thanks for the PR. It looks promising to me. (Must have misunderstood your second comment ;))

I personally think it is a good thing to have dependencies removed, when they are duplicated (we have that functionality provided by Saxon and the JDK) and are not maintained anymore.

But of course, this change has to be tested thoroughly and might have more consequences, than can be seen on the first glance.

For example, after removing xalan from our dependencies, we have to explicitly set the TransformerFactory implementation, as the library lets-plot-batik contains a serviceprovider file to set xalan as the transformerfactory.
Comment 15 The ASF infrastructure team 2022-09-24 20:38:23 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/5690