ofbiz-framework
ofbiz-framework copied to clipboard
Updated several (transitive) dependencies (OFBIZ-13123)
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.
@JacquesLeRoux Feel free to review
Thanks @dtrunk90,
That's quite a help, much appreciated :+1: I'll review and test all that
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
Hey,
weird. For me there were some about simplifying expressions. And after upgrading the checkstyle toolVersion there were some more.
Ah, maybe it's the reason... Will check that... Anyway so far all your fixes seems good to me ;)
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)?
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/[email protected]:gte=1d:%22tomcat%2010%22 Most of the time, for a such task we discuss on dev ML before...
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.
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!
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.
That's why it's in separate commits. I see no reason in doing another PR just for the checkstyle corrections.
OK, so it's only about (in order of appearance): https://github.com/apache/ofbiz-framework/pull/819/commits/b6ed6a626ef94f6a93a87aedbca251fce074fc50 https://github.com/apache/ofbiz-framework/pull/819/commits/e5464886765d7c25ff340a54ee29d32752ee62d4 right ? With the new checkstyle toolVersion of course ie https://github.com/apache/ofbiz-framework/pull/819/commits/759c8752542a89b7229e232634bb5ec731cbc0d3
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.
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.
While reverting the patch, I saw that you renamed GroovyScriptTestCase to GroovyScriptAssert and GroovyTestCase to GroovyAssert. Not a big deal, just to say.
Yes, I've only tested framework. I'll have a look at the plugins later that day. Didn't thought about them tbh.
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.
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.
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?
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.
Great, I now understand better why you wanted to keep OWASP dependencies check.
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
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.
@JacquesLeRoux @mbrohl Were you able to review/test the rest yet?
Hi Danny,
I did not review nor tested since your commits 2 days ago. Before all was OK with me as much as I was able to test. Do you know what are the differences in these last commits?
Hi Danny,
I did not review nor tested since your commits 2 days ago. Before all was OK with me as much as I was able to test. Do you know what are the differences in these last commits?
I just dropped the tomcat upgrade and resolved the conflicts.
Then it's OK with me
Any news when this will be merged?
Hi @dtrunk90,
Please update Apache PDFBox to Version 2.0.32. No CVEs but several bug fixes: https://lists.apache.org/thread/p1vg0j8pg23lptlwvmjybm1l7lbbdzjn TIA
Hi @dtrunk90,
Please update Apache PDFBox to Version 2.0.32. No CVEs but several bug fixes: https://lists.apache.org/thread/p1vg0j8pg23lptlwvmjybm1l7lbbdzjn TIA
Done. Would be nice to get this merged today so I can finally merge it into our project. We need this tomorrow. Otherwise I have to merge my fork which I actually wanted to avoid.