jmeter icon indicating copy to clipboard operation
jmeter copied to clipboard

remove xalan dependency due to it being end of life

Open asfimport opened this issue 3 years ago • 14 comments

PJ Fanning (Bug 66171): 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

Severity: normal OS: All

asfimport avatar Jul 19 '22 19:07 asfimport

@FSchumacher (migrated from Bugzilla): Care to provide a patch/PR?

asfimport avatar Jul 20 '22 14:07 asfimport

PJ Fanning (migrated from Bugzilla): 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.

asfimport avatar Jul 20 '22 14:07 asfimport

@FSchumacher (migrated from Bugzilla): Fair enough.

asfimport avatar Jul 20 '22 14:07 asfimport

@vlsi (migrated from Bugzilla): 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"

asfimport avatar Jul 20 '22 14:07 asfimport

PJ Fanning (migrated from Bugzilla): 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.

asfimport avatar Jul 21 '22 16:07 asfimport

@FSchumacher (migrated from Bugzilla): 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 :)

asfimport avatar Jul 21 '22 18:07 asfimport

@FSchumacher (migrated from Bugzilla): 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.

asfimport avatar Jul 22 '22 18:07 asfimport

PJ Fanning (migrated from Bugzilla): 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).

asfimport avatar Jul 22 '22 19:07 asfimport

@vlsi (migrated from Bugzilla): 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.

asfimport avatar Jul 22 '22 19:07 asfimport

PJ Fanning (migrated from Bugzilla): 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.

asfimport avatar Jul 22 '22 20:07 asfimport

PJ Fanning (migrated from Bugzilla): https://lists.apache.org/thread/2qvl7r43wb4t8p9dd9om1bnkssk07sn8

asfimport avatar Jul 22 '22 20:07 asfimport

@vlsi (migrated from Bugzilla):

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

asfimport avatar Jul 23 '22 09:07 asfimport

PJ Fanning (migrated from Bugzilla): 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.

asfimport avatar Jul 23 '22 12:07 asfimport

@FSchumacher (migrated from Bugzilla): 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.

asfimport avatar Jul 23 '22 13:07 asfimport

Hi team, any update/progress on this?

ahuertas-ms avatar Oct 27 '22 02:10 ahuertas-ms

There's a new Xalan release pending, so I guess we'll update Xalan in JMeter soon: https://lists.apache.org/thread/1pmwtqy1wf6wh96h9d1cdwn8qsdy42zh

vlsi avatar Oct 27 '22 08:10 vlsi

https://github.com/HtmlUnit/htmlunit-xpath Can this be an alternative?

I checked the same problem with htmlunit and saw this ticket: https://github.com/HtmlUnit/htmlunit/issues/493

firatkucuk avatar Mar 23 '23 12:03 firatkucuk

I updated Xalan to 2.7.3 in https://github.com/apache/jmeter/pull/5880

vlsi avatar May 05 '23 14:05 vlsi