BaseXdb / basex

BaseX Main Repository.
http://basex.org
BSD 3-Clause "New" or "Revised" License
661 stars 268 forks source link

Inconsistent behavior if TransformerFactory is set to the well-known Saxon implementation #2209

Closed brandes-pq closed 1 year ago

brandes-pq commented 1 year ago

Description of the Problem

When running basex with the system property javax.xml.transform.TransformerFactory set to one of the well-known saxon implementations, xslt:processor() and xslt:version() both report "unknown".

If the system property is not set, it will be as a side effect of running any XSLT-related code (e.g. xslt:version()), but now with proper values for xslt:processor() and xslt:version().

This is not only inconsistent but also a problem when using e.g. the newish Schematron module https://github.com/schxslt/schxslt, since that module rejects to process schematron files with @queryBinding="xslt2" if xslt:version() returns unknown.

Expected Behavior

If saxon is in the classpath and the system property javax.xml.transform.TransformerFactory is set to an appropriate value (e.g. net.sf.saxon.TransformerFactoryImpl, depending on the saxon version), xslt:processor() and xslt:version() both should return the same values as if the system property has not been set.

Steps to Reproduce the Behavior

  1. Copy e.g. Saxon-HE-11.5.jar into basex/lib
  2. Run basex -q 'xslt:version()', result is 3.0
  3. Run
BASEX_JVM=-Djavax.xml.transform.TransformerFactory=net.sf.saxon.TransformerFactoryImpl \
    basex -q 'xslt:version()'

result is unknown, but the XSLT implementation is the same.

Do you have an idea how to solve the issue?

I have a proposal for a change in the initialization logic in XsltFn.java that I can offer as a pull request.

Since the code runs as a static class initializer, it is a bit tricky to unit test, and I have not tried to add a test yet. It may be better to refactor the initialization into a function for better testing, what do you think?

What is your configuration?

BaseX 10.6

ChristianGruen commented 1 year ago

I have a proposal for a change in the initialization logic in XsltFn.java that I can offer as a pull request.

Would definitely be interesting, thanks.

Since the code runs as a static class initializer, it is a bit tricky to unit test

I think we can go without it; I assume the code in question won’t change often. Next, we don’t have a setup for optional components.

ChristianGruen commented 1 year ago

Thanks, appreciated! I’ve taken your pull request and done some more rewrites that slightly change the semantics of the original code:

https://github.com/BaseXdb/basex/blob/main/basex-core/src/main/java/org/basex/query/func/xslt/XsltFn.java

Feel free to close the issue if you think it’s resolved.

brandes-pq commented 1 year ago

I am fine with the changes, but

ChristianGruen commented 1 year ago
  • xslt:version() returning the empty string is not allowed according to the documentation.

True, I still need to revise the documentation (I noticed some other inconsistencies).

it still might be an issue when running with security manager policies.

Good point; has been changed.

brandes-pq commented 1 year ago

Great, thank you for the fast response! Should I keep the ticket open until the documentation has been revised?

ChristianGruen commented 1 year ago

Thanks as well. The documentation of xslt:processor and xslt:version has been updated. I’ll close the issue, just reopen it if you spot something dubious.