-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[NO-ISSUE] Use reload4j instead of log4j #4719
Changes from all commits
1315eb3
250c24e
431257c
9404384
f493fe7
8d2554e
168c167
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,8 +114,8 @@ | |
<plugin.frontend.version>1.12.1</plugin.frontend.version> | ||
|
||
<!-- common library versions --> | ||
<slf4j.version>1.7.30</slf4j.version> | ||
<log4j.version>1.2.17</log4j.version> | ||
<slf4j.version>1.7.35</slf4j.version> | ||
<reload4j.version>1.2.25</reload4j.version> | ||
<libthrift.version>0.13.0</libthrift.version> | ||
<flexmark.all.version>0.62.2</flexmark.all.version> | ||
<gson.version>2.8.9</gson.version> | ||
|
@@ -229,8 +229,20 @@ | |
|
||
<dependency> | ||
<groupId>org.slf4j</groupId> | ||
<artifactId>slf4j-log4j12</artifactId> | ||
<artifactId>slf4j-reload4j</artifactId> | ||
<version>${slf4j.version}</version> | ||
<exclusions> | ||
<exclusion> | ||
<groupId>ch.qos.reload4j</groupId> | ||
<artifactId>reload4j</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
|
||
<dependency> | ||
<groupId>ch.qos.reload4j</groupId> | ||
<artifactId>reload4j</artifactId> | ||
<version>${reload4j.version}</version> | ||
</dependency> | ||
|
||
<!-- Use jcl-over-slf4j instead of commons-logging --> | ||
|
@@ -240,12 +252,6 @@ | |
<version>${slf4j.version}</version> | ||
</dependency> | ||
|
||
<dependency> | ||
<groupId>log4j</groupId> | ||
<artifactId>log4j</artifactId> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removing the definition from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll have a better look over the next day or 2 to see what transitive dependencies could leaking log4j dependencies into zeppelin. There already appears to be issues (pre-existing issues unrelated to this change) where log4j and log4j-1.2-api are conflicting. log4j-1.2-api is a log4jv2 jar that pretends to be log4jv1 and you shouldn't have this on the classpath at the same time as you have the legacy log4j jar. |
||
<version>${log4j.version}</version> | ||
</dependency> | ||
|
||
<dependency> | ||
<groupId>org.apache.thrift</groupId> | ||
<artifactId>libthrift</artifactId> | ||
|
@@ -649,6 +655,10 @@ | |
<groupId>commons-logging</groupId> | ||
<artifactId>commons-logging</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>log4j</groupId> | ||
<artifactId>log4j</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
|
||
|
@@ -1117,6 +1127,14 @@ | |
<groupId>commons-logging</groupId> | ||
<artifactId>commons-logging</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>log4j</groupId> | ||
<artifactId>log4j</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>org.slf4j</groupId> | ||
<artifactId>slf4j-log4j12</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>org.ow2.asm</groupId> | ||
<artifactId>asm</artifactId> | ||
|
@@ -1332,6 +1350,10 @@ | |
<groupId>commons-logging</groupId> | ||
<artifactId>commons-logging</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>log4j</groupId> | ||
<artifactId>log4j</artifactId> | ||
</exclusion> | ||
<exclusion> | ||
<groupId>com.fasterxml.jackson.core</groupId> | ||
<artifactId>jackson-databind</artifactId> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not 1.7.36?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
causes a maven convergence issue - due to other dependencies of zeppelin using 1.7.35
I can try further changes but it could take a few days.
The key thing is to get agreement that keeping log4jv1 is not a good thing and that reload4j is a reasonable short term change to avoid using a completely insecure logging framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the elaboration, that makes sense.