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

[BUG] No version for EXPath Crypto dependency #8

Closed
adamretter opened this issue May 3, 2023 · 11 comments · Fixed by #9
Closed

[BUG] No version for EXPath Crypto dependency #8

adamretter opened this issue May 3, 2023 · 11 comments · Fixed by #9
Labels
question Further information is requested

Comments

@adamretter
Copy link
Member

adamretter commented May 3, 2023

The existdb-saml EXPath Package (including the latest version 1.6.3) does not declare the version of the EXPath Crypto module that it depends on.

The expath-pkg.xml in the XAR file, only contains this:

<package xmlns="http://expath.org/ns/pkg" name="http://exist-db.org/xquery/exsaml" abbrev="existdb-saml" version="1.6.3" spec="1.0">
    <title>existdb-saml</title>
    <dependency package="http://expath.org/ns/crypto"/>
    <dependency processor="http://exist-db.org" semver-min="3.2.0"/>
</package>

The dependency on http://expath.org/ns/crypto should declare at least a semver-min.

There have been lots of differing version of the EXPath Crypto module (published for eXist-db), including crypto versions: 6.0.1, 6.0.0, 5.3.0, 1.0.0, 0.7, 0.6, 0.5, 0.3.5, and 0.3.4.

The existdb-saml module declares that it requires eXist-db version 3.2.0 or later. The version of EXPath Crypto for eXist-db 3.2.0 (i.e. 0.3.5) however does not appear to work with the existdb-saml module on eXist-db 6.1.0. See https://exist-db.org/exist/apps/public-repo/packages/crypto?eXist-db-min-version=3.2.0

Questions

  1. Which is the version that is meant to be used by existdb-saml please?
  2. Does the semver-min of eXist-db used by the existdb-saml module need to be updated perhaps?
@adamretter adamretter added bug Something isn't working question Further information is requested labels May 3, 2023
@adamretter
Copy link
Member Author

adamretter commented May 3, 2023

I have also just tested the following which also appears to fail:

  • eXist-db 6.1.0-SNAPSHOT
  • existdb-saml 1.6.3
  • EXPath Crypto 6.0.1

I get the following error:

Caused by: org.exist.xquery.XPathException: exerr:ERROR error found while loading module exsaml: Error while loading module xmldb:///db/apps/existdb-saml/content/exsaml.xqm: error found while loading module crypto: Cannot find module class from EXPath repository: org.expath.exist.crypto.ExistExpathCryptoModule
	at org.exist.repo.ExistRepository.getModule(ExistRepository.java:164)
	at org.exist.repo.ExistRepository.resolveJavaModule(ExistRepository.java:140)
	at org.exist.xquery.XQueryContext.resolveInEXPathRepository(XQueryContext.java:509)
	at org.exist.xquery.XQueryContext.importModule(XQueryContext.java:2450)
	at org.exist.xquery.parser.XQueryTreeParser.importDecl(XQueryTreeParser.java:6353)
	at org.exist.xquery.parser.XQueryTreeParser.prolog(XQueryTreeParser.java:5367)
	at org.exist.xquery.parser.XQueryTreeParser.libraryModule(XQueryTreeParser.java:4047)
	at org.exist.xquery.parser.XQueryTreeParser.module(XQueryTreeParser.java:3878)
	at org.exist.xquery.parser.XQueryTreeParser.xpath(XQueryTreeParser.java:3659)
	at org.exist.xquery.XQueryContext.compileModule(XQueryContext.java:2666)
	at org.exist.xquery.XQueryContext.compileOrBorrowModule(XQueryContext.java:2607)
	at org.exist.xquery.XQueryContext.importModuleFromLocation(XQueryContext.java:2536)
	at org.exist.xquery.XQueryContext.importModule(XQueryContext.java:2471)
	at org.exist.xquery.parser.XQueryTreeParser.importDecl(XQueryTreeParser.java:6353)
	at org.exist.xquery.parser.XQueryTreeParser.prolog(XQueryTreeParser.java:5367)
	at org.exist.xquery.parser.XQueryTreeParser.mainModule(XQueryTreeParser.java:4059)
	at org.exist.xquery.parser.XQueryTreeParser.module(XQueryTreeParser.java:4004)
	at org.exist.xquery.parser.XQueryTreeParser.xpath(XQueryTreeParser.java:3659)
	at org.exist.xquery.XQuery.compile(XQuery.java:251)
	at org.exist.xquery.XQuery.compile(XQuery.java:180)
	at org.exist.xquery.XQuery.compile(XQuery.java:138)
	at org.exist.http.urlrewrite.XQueryURLRewrite.runQuery(XQueryURLRewrite.java:671)
	at org.exist.http.urlrewrite.XQueryURLRewrite.service(XQueryURLRewrite.java:241)
	... 37 more
Caused by: java.lang.ClassNotFoundException: org.expath.exist.crypto.ExistExpathCryptoModule
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:587)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:467)
	at org.exist.repo.ExistRepository.getModule(ExistRepository.java:154)
	... 59 more

I believe the additional error in this comment may instead be related to eXist-db/exist#4901

@chakl
Copy link
Collaborator

chakl commented May 4, 2023

Which is the version that is meant to be used by existdb-saml please?

The latest EXPath crypto module v6.0.1

Does the semver-min of eXist-db used by the existdb-saml module need to be updated perhaps?

Definitely. This was planned once the crypto-module renaming and re-versioning would settle. I guess crypto-6.0.1 is stable for long enough now.

There have been lots of differing version of the EXPath Crypto module (published for eXist-db), including crypto versions: 6.0.1, 6.0.0, 5.3.0, 1.0.0, 0.7, 0.6, 0.5, 0.3.5, and 0.3.4.

Yes. The library had to deal with this mess since crypto-0.3.x

@chakl
Copy link
Collaborator

chakl commented May 4, 2023

I have also just tested the following which also appears to fail:
eXist-db 6.1.0-SNAPSHOT
existdb-saml 1.6.3
EXPath Crypto 6.0.1

I had installed a test system (that I no longer have access to) with exactly these versions, and I have not seen the error you posted. Quite frankly, I have no idea.

I maintain other systems that run exist v6.2.0 and crypto v6.0.1. Unfortunately, they forked existdb-saml and introduced an incompatble version numbering. But a quick look at the code suggests the code that does crypto import and crypto usage is not different from existdb-saml v1.6.x.

existdb-saml 1.6.x is expected to work with exist6 and crypto-6.0.1.

I very vaguely remember a confusing error like this when I installed and upgraded various crypto-lib versions in a row for testing. I suspected some confusion with package management, restarted with a fresh exist, and never bothered to debug this.

@adamretter
Copy link
Member Author

existdb-saml 1.6.x is expected to work with exist6 and crypto-6.0.1.

@chakl Thanks, To clarify... would that explicitly be eXist-db 6.0.0 then, or if not, which newer version please?

@chakl
Copy link
Collaborator

chakl commented May 4, 2023

I'd say >=5.3.0, you know that release better than me

@joewiz
Copy link
Member

joewiz commented May 4, 2023

I had thought that expath-crypto-module 6.0.1 required eXist 6, but I see from the pom that the dependency is actually eXist 5.3.0+: https://github.com/eXist-db/expath-crypto-module/blob/main/pom.xml#L97. But then again this issue says there are problems with 5.3.0: eXist-db/expath-crypto-module#64. Hmm...

@chakl
Copy link
Collaborator

chakl commented May 5, 2023

Hmm.. when following your link eXist-db/expath-crypto-module#64, it seems to refer to tests with exist v5.4.x versions. Tests failed with incompatible types: org.exist.storage.serializers.Serializer cannot be converted to com.evolvedbinary.j8fu.function.ConsumerE<com.evolvedbinary.j8fu.function.ConsumerE<org.exist.storage.serializers.Serializer,java.io.IOException>,java.io.IOException>

I don't think this 5.4.x release was put into production anywhere.

@chakl chakl removed the bug Something isn't working label May 9, 2023
@chakl
Copy link
Collaborator

chakl commented May 9, 2023

Closing this. Not a bug.

Not specifying a semver-min attribute for the crypto dependency implies "use latest available version", which is correct. An upcoming maintenance release will be more explicit on the expected crypto version.

@adamretter If you have more evidence of incorrect behavior, please open another issue.

@chakl chakl closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2023
@adamretter adamretter reopened this May 10, 2023
@adamretter
Copy link
Member Author

Not specifying a semver-min attribute for the crypto dependency implies "use latest available version"

Agreed.

which is correct

Well not always actually! It depends on which version of eXist-db you have. The Crypto module has a minimum supported version of eXist-db too! So the "latest" version of the Crypto module for eXist-db 6.1.0 will not be the same as the latest version of the Crypto module for previous versions of eXist-db.

So for example. The existdb-saml module will almost certainly not work on eXist-db 3.2.0 with the version of the Crypto module which supports eXist-db 3.2.0. That is to say that the following from the existdb-saml module's expath-pkg.xml file is incorrect and needs to be corrected:

<dependency package="http://expath.org/ns/crypto"/>
<dependency processor="http://exist-db.org" semver-min="3.2.0"/>

@chakl
Copy link
Collaborator

chakl commented May 10, 2023

@adamretter I hear you. As stated in the last Close these issues will be addressed in an upcoming maintenance release.

So for example. The existdb-saml module will almost certainly not work on eXist-db 3.2.0

Probably not, but you're fabricating a fictional scenario. You may want to hire consultants who know eXist-db and SAML to address this, if very old eXist-db need to be modernized.

Re-Closing

@adamretter
Copy link
Member Author

adamretter commented May 11, 2023

but you're fabricating a fictional scenario.

I am afraid I don't understand. How is this fictional?
The fact of the matter is that the existdb-saml application declares compatibility with eXist-db 3.2.0 but it is not compatible.

You may want to hire consultants who know eXist-db and SAML to address this

We are those consultants!

We will send a PR to fix this problem, and that will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants