Move off of dangerous log4j dependency #7
Conversation
| @@ -31,6 +34,7 @@ dependencies { | |||
| } | |||
|
|
|||
| test { | |||
| jvmArgs "-Xshare:off" | |||
There was a problem hiding this comment.
Slightly unrelated change, just removing a dumb warning when running tests.
| @@ -49,7 +50,7 @@ | |||
| public class SamlGenerator { | |||
|
|
|||
| /* Constants */ | |||
| private static final Logger logger = Logger.getLogger(SamlGenerator.class); | |||
| private static final Logger logger = LogManager.getLogger(SamlGenerator.class); | |||
There was a problem hiding this comment.
This Logger.getLogger(... -> LogManager.getLogger(... change is part of the upgrade
There was a problem hiding this comment.
Notice above how the imports change from:
org.apache.log4j... to:
org.apache.logging.log4j...
This is a really annoying way they try to be backwards compatible (I guess?) but upgrade their code >.<
|
Thanks for the review! I have pushed this change into prod, but I kinda want to leave this open until I get a response from GitHub about Dependabot. Not sure if this link works for others, but my GitHub ticket is here. |
casewalker
force-pushed
the
fix-logger-setup
branch
from
ddd1b0c to
129ef95
Compare
|
I rebased this review on Main since there were a lot of changes to how dependencies are defined, I am going to use the sticky-accept since none of the other parts of this review changed. |
The dependency that came from Serverless's Java Lambda template (
com.amazonaws:aws-lambda-java-log4j:1.0.1) is so old, it is still subject to log4j bugs! See vulnerabilities here. And it is also the newest version of this dependency, having been released in 2020.This change removes that dependency and follows guidance from AWS docs here to get logging setup correctly with other dependencies.