Updated several (transitive) dependencies (OFBIZ-13123)

[dtrunk90]

Improved:

• Update Apache PDFBox to 2.0.32
• Update Apache CXF Runtime JAX-RS Frontend to 3.6.4
• Update Asciidoctor Gradle Plugin to 4.0.2
• Update transitive dependency testng to 7.7.0
• Update Groovy to 4.0.22 ¹
• Update Apache MINA sshd to 2.13.1
• Update poi to 5.3.0
• Update ez-vcard to 0.12.1
• Update jdom to 2.0.6.1
• Update Apache CXF Runtime JAX-RS Frontend to 3.6.3
• Update transitive dependency bcprov-jdk18on to 1.78
• Update tika parsers to 2.9.2
• Update fop to 2.9
• Update transitive dependency mime4j to 0.8.10
• Update clojure to 1.11.3
• Update derby to 10.16.1.1 ²
• Update jackson-databind to 2.17.1
• Update esapi to 2.5.4.0
• Add guava as dependency
• Set checkstyle.toolVersion
• Update org.owasp.dependencycheck to 10.0.2
• Upgrade to gradle 8.8

Reverted:

• Improved: Abandon the Gradle Owasp dependencycheck task (OFBIZ-13121) 0a9ee32 ³

Fixed:

• Corrections based on Checkstyle errors

I've updated several (transitive) dependencies. For the transitive dependencies see the because clause in their respective constraint.

¹ Maven coordinates have changed for Groovy 4+ (see https://groovy-lang.org/releasenotes/groovy-4.0.html).

² org.apache.derby.jdbc.EmbeddedDriver is now in derbytools.

³ The new REST API from NVD isn't stable (currently) because it's under massive load and returning HTTP 503 Service Unavailable sometimes. On a clean/purged CVE DB I had to wait ~1h 30m for dependencyCheckAnalyze to finish. But it worked and I think DependencyCheck is a good tool for finding at least some reasonable CVEs. This shouldn't be abandoned imho.

[dtrunk90]

@JacquesLeRoux Feel free to review

[JacquesLeRoux]

Thanks @dtrunk90,

That's quite a help, much appreciated 👍 I'll review and test all that

[JacquesLeRoux]

Hi Dan,

You say

Fixed:

• Corrections based on Checkstyle errors

But there are currently no errors in trunk: https://nightlies.apache.org/ofbiz/trunk/checkstyle.html

[dtrunk90]

Hey,

weird. For me there were some about simplifying expressions. And after upgrading the checkstyle toolVersion there were some more.

[JacquesLeRoux]

Ah, maybe it's the reason... Will check that... Anyway so far all your fixes seems good to me ;)

[dtrunk90]

Sounds great. More to come. I'm planning on updating Tomcat to v10 and do all the javax to jakarta namespace changes so libraries depending on jakarta namespace can be updated too and we're able to use Spring 6 by then. Do you want me to put them in this PR or do another one after this got merged (because the changes will affect loads of files and could cause conflicts)?

[JacquesLeRoux]

Do you want me to put them in this PR or do another one after this got merged (because the changes will affect loads of files and could cause conflicts)?

Please create a new PR, TIA
Also I have created https://issues.apache.org/jira/browse/OFBIZ-13123 for this PR
For the new one https://issues.apache.org/jira/browse/OFBIZ-12989 exists already
This is also interesting: https://lists.apache.org/list?dev@ofbiz.apache.org:gte=1d:%22tomcat%2010%22
Most of the time, for a such task we discuss on dev ML before...

[dtrunk90]

Aight. I had to force push again because I forgot to add derbytools. Now I was able to boot up OFBiz and click around the backend without anything popping up in the error log.

[mbrohl]

Please do not merge this PR or a new version of it until it is checked thorougly. I will have a look at it also after it is cleaned up and tested but not before mid/end of next week.

Thanks!

[JacquesLeRoux]

Also it would be better to separate in another PR the work done for fixing corrections based on Checkstyle errors. That would allow to easily revert one part or another if necessary.

[dtrunk90]

That's why it's in separate commits. I see no reason in doing another PR just for the checkstyle corrections.

[JacquesLeRoux]

OK, so it's only about (in order of appearance):
b6ed6a6
e546488
right ?
With the new checkstyle toolVersion of course ie 759c875

[JacquesLeRoux]

After a review on GH UI, the checkstyle corrections work perfectly locally. 34 issues before b6ed6a6, 14 remaining before e546488, then none, +1 for this part.

[JacquesLeRoux]

Hi @dtrunk90,

I guess you only tested the framework. We have 2 failures with plugins:

1: Task failed with an exception.

• Where:
  Build file 'C:\projectsASF\Git\ofbiz-framework\plugins\solr\build.gradle' line: 30

• What went wrong:
  A problem occurred evaluating project ':plugins:solr'.

No signature of method: org.gradle.api.internal.artifacts.ivyservice.dependencysubstitution.DefaultDependencySubstitutions$1.with() is applicable for argument types: (org.gradle.internal.component.external.model.DefaultModuleComponentS
elector) values: [com.google.guava:guava:28.0-jre]
Possible solutions: with(groovy.lang.Closure), with(boolean, groovy.lang.Closure), wait(), wait(long), find(), wait(long, int)

2: Task failed with an exception.

• What went wrong:
  A problem occurred configuring root project 'ofbiz'.

No hooks found

When used with framework only, the last failure does not appear and OFBiz builds and runs. I guess the 2nd failure is only due to the Solr error.

I ran the ZAP spider on it, which is not much reliable for finding errors, and got no relevant errors in error.log

Disclaimer: I did not review this part yet. I mean I only reviewed the Checkstyle corrections.

[JacquesLeRoux]

While reverting the patch, I saw that you renamed GroovyScriptTestCase to GroovyScriptAssert and GroovyTestCase to GroovyAssert. Not a big deal, just to say.

[dtrunk90]

Yes, I've only tested framework. I'll have a look at the plugins later that day. Didn't thought about them tbh.

[dtrunk90]

While reverting the patch, I saw that you renamed GroovyScriptTestCase to GroovyScriptAssert and GroovyTestCase to GroovyAssert. Not a big deal, just to say.

Yes, because Groovy deprecated the GroovyTestCase and refers to GroovyAssert as a replacement. I thought it would be better to make that change clear in the OFBiz class as well.

[JacquesLeRoux]

I tried it this morning and indeed after 1h15 I got the result. It found 217 vulnerabilities!
I guess (did not check all!) most of them, if not all, are transitive dependencies that are currently not in dependencies.gradle.
Anyway I agree that it can still be usefull. So I reopened rOFBIZ-13121 to revert with this PR.

[JacquesLeRoux]

BTW @dtrunk90, the title speaks about transitive dependencies, how do you expect to fix them? If I understand well, they are embedded/used within OFBiz declared dependencies. Adding them in dependencies.gradle would not help, right?

[dtrunk90]

BTW @dtrunk90, the title speaks about transitive dependencies, how do you expect to fix them? If I understand well, they are embedded/used within OFBiz declared dependencies. Adding them in dependencies.gradle would not help, right?

Actually you can update transitive dependencies: https://docs.gradle.org/current/userguide/dependency_constraints.html

I've only updated to the smallest possible version (patch or minor) to not break anything.

[JacquesLeRoux]

Great, I now understand better why you wanted to keep OWASP dependencies check.

[JacquesLeRoux]

Hi Danny,

For security reason, with https://issues.apache.org/jira/browse/OFBIZ-13124 I have already upgraded Tomcat to 9.0.91

Also if you did not spot it, you might be interested by this dev ML thread:
https://lists.apache.org/thread/xoqcjo20moz3gt4v40k51h1ngoh44lor

[dtrunk90]

Hi Danny,

For security reason, with https://issues.apache.org/jira/browse/OFBIZ-13124 I have already upgraded Tomcat to 9.0.91

Also if you did not spot it, you might be interested by this dev ML thread: https://lists.apache.org/thread/xoqcjo20moz3gt4v40k51h1ngoh44lor

No problem. I've dropped the commit.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[mbrohl]

Sorry @JacquesLeRoux and @dtrunk90 for the delay.
I am currently occupied with other duties.

To respond to the questions: there is a lot of dependency changes in this PR which is quite unusual and cannot be compared with the update of some dependency changes now and then like we did in the past.

Those dependency changes might cause other transitive dependencies to change and this could lead to breaking changes. I would simply like to take my time to check all this, including to have a look at the changelogs of the libraries changed. They often give hints about deprecations or breaking changes.

I am wondering why this PR does not raise more attention, it worries me a bit.
I recommend to put a reference in the dev mailing list to give everyone a chance to recognize this initiative and give his opinions.

I hope that I find some time to check this soon but cannot give a reliable point in time for it.

[JacquesLeRoux]

Thanks @mbrohl for your clear answer. I agree that for major changes like this one, it's better to have a discussion on dev ML to indeed hopefully get more attention and opinions, the more brains and experiences the better.

On the other hand, reviewing all dependencies change logs can take some time. Before we start a discussion on dev ML, @dtrunk90 would that be a problem for you?

[dtrunk90]

Thanks @mbrohl for your clear answer. I agree that for major changes like this one, it's better to have a discussion on dev ML to indeed hopefully get more attention and opinions, the more brains and experiences the better.

On the orher hand, reviewing all dependencies change logs can take some time. Before we start a discussion on dev ML, @dtrunk90 would that be a problem for you?

Up to you how you want to handle these changes. Take your time. I've merged my fork already into our projects. So we're good about the tons of CVEs reported by those outdated dependencies.

[JacquesLeRoux]

Hi @dtrunk90,

We have a small conflict here: https://github.com/apache/ofbiz-framework/pull/819/conflicts
Can you confirm the comment

// NOTE: since 2.4 dependencies are messed up. See https://github.com/moqui/moqui-fop/blob/master/build.gradle

is now pointless?

TIA