Is your feature request related to a problem? Please describe.
The log4j:1.2.17 jar file is flagged as a potentially vulnerable jar file on a recent Nexus scan.
Describe the solution you'd like
Upgrade log4j to a more recent version.
Describe alternatives you've considered
None.
Additional context
None.
Would you like to help implementing this feature?
I thought this might be as simple as replacing the log4j:1.2.17 dependency in zap.gradle.kts with the following:
api("org.apache.logging.log4j:log4j-1.2-api:2.13.3")
api("org.apache.logging.log4j:log4j:2.13.3")
The above code upgrades log4j to the next major version (2) and uses the log4j-1.2-api as an adapter to ensure that usages in ZAP of the log4j 1.2 code do not need to change. I encountered an issue with this approach where ZapBootstrap uses the log4j NullAppender. This code doesn't compile with the above approach.
If you think this approach is worthy of pursuing, and you can roughly guide me on how to reproduce the issue this solved: // Nasty hack to prevent warning messages when running from the command line, then I'm happy to try to create a pull request with this fix/feature.
Always in favour of keeping our dependencies up to date, but as you've found out upgrading them can sometimes be painful :/
What compilation error(s) are you getting?
as you've found out upgrading them can sometimes be painful :/
Totally! I thought this change would be impossible until I found out about the log4j adapter. (it still might be impossible 馃槀)
What compilation error(s) are you getting?
org.apache.log4j.varia.NullAppender isn't exposed by the log4j-1.2-adapter, so compilation fails when ZapBootstrap imports a class that cannot be found. I don't think configuring appenders dynamically is supported by the adapter at all (which I suppose is something for you to consider when approving the suggested approach).
Do you know what warnings were being suppressed by the addition of the NullAppender in ZapBootstrap? It's possible that the warnings don't even show on the new version of log4j, or that there is an alternate way of removing them. I removed the NullAppender in the current version of ZAP (i.e. using log4j 1.2.7) and didn't notice any strange warnings when starting ZAP with the run task.
Do you know what warnings were being suppressed by the addition of the
NullAppenderin ZapBootstrap?
That's because of these:
> Task :zap:run
log4j:WARN No appenders could be found for logger (org.apache.commons.configuration.ConfigurationUtils).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
...
Regarding the above WARNs, indeed, that no longer happens with Log4j 2 (it seems the new version has/uses a default configuration, instead of warning if nothing is configured yet).
As already pointed out the Log4j 1.x bridge does not support everything, which causes the following problems/differences:
${zap.user.log}/zap.log);The good news is that we can change some parts to use Log4j 2 APIs and rely on the bridge for "normal logging" elsewhere (especially important for the add-ons).
I'll work on addressing the above problems using the new APIs, for ZAP 2.10.0.
Most helpful comment
Regarding the above WARNs, indeed, that no longer happens with Log4j 2 (it seems the new version has/uses a default configuration, instead of warning if nothing is configured yet).
As already pointed out the Log4j 1.x bridge does not support everything, which causes the following problems/differences:
${zap.user.log}/zap.log);The good news is that we can change some parts to use Log4j 2 APIs and rely on the bridge for "normal logging" elsewhere (especially important for the add-ons).
I'll work on addressing the above problems using the new APIs, for ZAP 2.10.0.