You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/05/27 15:54:17 UTC
[GitHub] [activemq] jbonofre opened a new pull request #662: [AMQ-7426] Upgrade to log4j2
jbonofre opened a new pull request #662:
URL: https://github.com/apache/activemq/pull/662
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781757330
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Note:
1. ActiveMQ is not EOL, ActiveMQ 5.17.0 with at least log4j 2.17.1 will be available soon
2. log4j 1.x is NOT EOL, there's a discussion to restart the project and cut new releases. Reason is simply a bunch of projects (at Apache and elsewhere) still uses log4j 1.x
3. I'm sure you have a bunch of projects/products that use EOL dependencies without problem ;)
4. From an Apache standpoint, EOL doesn't exist, anyone can contribute even on old branches/projects and do new releases (see log4j 1.x example).
5. Please, stop to "pollute" the PR, use the mailing list or direct email, thanks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1046770937
I'd say its a comment about the update since you are creating all the new files now, and I dont think log4j 1 had that functionality.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812963288
##########
File path: pom.xml
##########
@@ -571,10 +572,71 @@
<version>${osgi-version}</version>
</dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-api</artifactId>
+ <version>${pax-logging-version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-service</artifactId>
+ <version>${pax-logging-version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>log4j</groupId>
+ <artifactId>apache-log4j-extras</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-log4j2</artifactId>
+ <version>${pax-logging-version}</version>
+ </dependency>
+
<dependency>
<groupId>org.apache.hadoop.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
<version>${zookeeper-version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>io.netty</groupId>
+ <artifactId>netty</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.zookeeper</groupId>
+ <artifactId>zookeeper</artifactId>
+ <version>${zookeeper-version}</version>
+ </dependency>
Review comment:
This is why the exclude isnt working - its not on the org.apache.zookeeper dep
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre merged pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre merged pull request #662:
URL: https://github.com/apache/activemq/pull/662
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781684921
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
@tallpsmith then presumably they won't be using any version of ActiveMQ "Classic" in any capacity anyway since it has always used Log4j 1.x. In any case, it's already been noted on this thread and on others that 5.17.0 will use the latest Log4j 2.x. _Hope_ isn't necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812935277
##########
File path: activemq-all/pom.xml
##########
@@ -109,9 +105,8 @@
<include>org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec</include>
<include>org.apache.geronimo.specs:geronimo-annotation_1.0_spec</include>
<include>org.slf4j:slf4j-api</include>
- <include>org.slf4j:slf4j-log4j12</include>
+ <include>org.apache.logging:log4j-slf4j-impl</include>
Review comment:
Yes the comment wasnt about slf4j-log4j12 (that removal is obvious), just the line below it as noted (which didnt change and so couldnt be commented on, which is explicitly including org.slf4j:jcl-over-slf4j, as is the assemby, yet the dependencyManagement for it was just removed in place of using the same property in the assembly. I dont see why you would remove the version management if still actually using it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048849416
Easy to check with a little grep :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048897917
Yep, no log4j 1 and no slf4j-log4j12 mentions now. The mentions of log4j look to all be 2.17.1 now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1050925000
@jbonofre - We all good to merge?Looks like the checks passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r779464291
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
And again it doesn't make sense to upgrade to AMQ 5.17 for log4j 2 as 5.15/5.16 are NOT impacted by log4shell vulnerability as they are using log4j 1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] luke-zhang commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
luke-zhang commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r779453129
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Log4j 2.17.1 is the latest version that includes the fix to https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-44832.
by the way is there a date of when this can be released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812922429
##########
File path: pom.xml
##########
@@ -442,29 +441,6 @@
<!-- =============================== -->
<!-- Required dependencies -->
<!-- =============================== -->
- <dependency>
- <groupId>commons-logging</groupId>
- <artifactId>commons-logging</artifactId>
- <version>${commons-logging-version}</version>
- <exclusions>
- <exclusion>
- <groupId>avalon-framework</groupId>
- <artifactId>avalon-framework</artifactId>
- </exclusion>
- <exclusion>
- <groupId>logkit</groupId>
- <artifactId>logkit</artifactId>
- </exclusion>
- <exclusion>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- </exclusion>
- <exclusion>
- <groupId>javax.servlet</groupId>
- <artifactId>servlet-api</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
Review comment:
I remember now, I removed from the dependency management because not used by ActiveMQ directly. However, indeed, it's important for dependency that use commons-logging by transitity (like activeio). I readding the commons-logging dependency management.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r768731691
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Yes, I shared this on mailing list and slack: I will update this PR with log4j 2.16.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-991675248
You'll definitely want to use something newer than 2.14.1 given the recently discovered [severe vulnerability](https://cve.circl.lu/cve/CVE-2021-44228).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] tallpsmith commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
tallpsmith commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781668550
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Some uses in higher-compliance environments (e.g. FedRAMP) will not accept EOL libraries like log4j 1.x in any use, but of course, don't want log4jshell either. They want their cake and to eat it too.
I hope 5.17 gets the latest log4j 2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1046738416
Test configs can/should use 'log4j2-test.properties' rather than 'log4j2.properties', it has a higher priority than regular files and makes it easier/quicker to distinguish the non-test config amongst the sea of files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812243638
##########
File path: activemq-all/pom.xml
##########
@@ -109,9 +105,8 @@
<include>org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec</include>
<include>org.apache.geronimo.specs:geronimo-annotation_1.0_spec</include>
<include>org.slf4j:slf4j-api</include>
- <include>org.slf4j:slf4j-log4j12</include>
+ <include>org.apache.logging:log4j-slf4j-impl</include>
Review comment:
Commenting about the line below, seems odd that it is including org.slf4j:jcl-over-slf4j when you just removed the explicit dependency on it, and later in the root pom the version-management entry for it. Since there remains commons-logging use, im not too clear why? Do some of the log4j2 bits bridge that directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048889854
I don't see log4j1 anymore so that part looks good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r779463613
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Hi as explained on the mailing list: 1 the PR will be updated with log4j 2.17.1. 2 vote should start end of Jan meaning release available on Feb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781350767
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
@oribendetcx sorry but it's really stupid ;) Log4j 1.x has no log4shell vulnerability. Anyway, I'm moving forward on 5.17.0 (planned for end of January as shared on the mailing list).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r782050230
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
@gemmellr I mean that from Apache standpoint, anyone can start a release or fork log4j as new Apache project. It's the discussion currently on going. If the PMC doesn't want to support log4j in logging Apache umbrella, a new community can create a new project in the incubator/Apache.
Anyway, I'm working on the PR today ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] oribendetcx commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
oribendetcx commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781347123
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
@jbonofre on paper, yes, but quite a lot of customers want to update to the latest log4j version as a precaution
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1047838946
@gemmellr most of them are coming from commons-logging dependency in activeio. Should I exclude it ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1047851751
I would say so, if the idea is to not use Log4J 1.x, then there should be no trace of it. Those bits should be using Log4J 2 (via the 1.2 bridge), or a less apealling alternative would be subbing in reload4j, but that doesnt really make sense given the context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1047854939
Alright, I add the exclude.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048991527
Think it needs a rebase after the recent force push on main. The Jenkins build fell over, and prodding a restart it doesnt like trying to merge it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-991675248
You'll definitely want to use something newer than 2.14.1 given the recently discovered [severe vulnerability](https://cve.circl.lu/cve/CVE-2021-44228).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] lucastetreault commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
lucastetreault commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r770230966
##########
File path: assembly/pom.xml
##########
@@ -431,16 +435,6 @@
<artifactId>json-simple</artifactId>
<version>${json-simple-version}</version>
</dependency>
- <dependency>
Review comment:
Good to see some old defunct stuff cleaned up at the same time!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r768730745
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
FWIW, this was noted on the PR a few days ago and discussed on the users mailing list as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1046783550
Catcha. Thanks. I'm updating the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812241660
##########
File path: activemq-shiro/pom.xml
##########
@@ -100,13 +94,19 @@
<scope>test</scope>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-log4j12</artifactId>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
<scope>test</scope>
</dependency>
<dependency>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>commons-logging</groupId>
+ <artifactId>commons-logging</artifactId>
+ <version>1.0</version>
Review comment:
The commons-logging version was managed by a dependencyManagement until the PR removed it, and this module previously specified a dep using that same property, which was set to 1.2...why specify a fixed version here, and one which is older than that used before on existing releases?
##########
File path: pom.xml
##########
@@ -442,29 +441,6 @@
<!-- =============================== -->
<!-- Required dependencies -->
<!-- =============================== -->
- <dependency>
- <groupId>commons-logging</groupId>
- <artifactId>commons-logging</artifactId>
- <version>${commons-logging-version}</version>
- <exclusions>
- <exclusion>
- <groupId>avalon-framework</groupId>
- <artifactId>avalon-framework</artifactId>
- </exclusion>
- <exclusion>
- <groupId>logkit</groupId>
- <artifactId>logkit</artifactId>
- </exclusion>
- <exclusion>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- </exclusion>
- <exclusion>
- <groupId>javax.servlet</groupId>
- <artifactId>servlet-api</artifactId>
- </exclusion>
- </exclusions>
- </dependency>
Review comment:
Removal of this very long standing set of (https://github.com/apache/activemq/commit/735dc7a230b15044c7fcefbe1f762b5ed13b3132) exclusions is presumably why all the log4j dependencies popped up for things referencing activeio. which uses commons-logging.
Removal of the dependencyManagement entry also explains why the instances of commons-logging are older than those present in 5.16.4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048964692
I think this is ready to merge at this point if the tests pass (i don't think i see any other changes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1049027252
Rebased
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812924093
##########
File path: activemq-all/pom.xml
##########
@@ -109,9 +105,8 @@
<include>org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec</include>
<include>org.apache.geronimo.specs:geronimo-annotation_1.0_spec</include>
<include>org.slf4j:slf4j-api</include>
- <include>org.slf4j:slf4j-log4j12</include>
+ <include>org.apache.logging:log4j-slf4j-impl</include>
Review comment:
slf4j-log4j12 is actually replaced by log4j-slf4j-impl, and commons-logging is not used directly in ActiveMQ.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048844817
> Still some remaining log4j 1 instances (from zookeeper and pax stuff, as on the reload4j PR):
This much should be expected given it was one of the main things in the reload4j PR, e.g https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R623-R656 and https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R491-R512
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1049826052
Last change breaks some tests, I'm fixing (probably due to dependency updates). I'm on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1049908875
All should be good now. I'm just waiting for Jenkins green light before merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812260211
##########
File path: activemq-all/pom.xml
##########
@@ -109,9 +105,8 @@
<include>org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec</include>
<include>org.apache.geronimo.specs:geronimo-annotation_1.0_spec</include>
<include>org.slf4j:slf4j-api</include>
- <include>org.slf4j:slf4j-log4j12</include>
+ <include>org.apache.logging:log4j-slf4j-impl</include>
Review comment:
I will check (I don't remember why I did that). Let me remember ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048885854
No log4j1 dep anymore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] LLCampos commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
LLCampos commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r768724623
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
We probably want `2.15.0` or higher :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] oribendetcx commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
oribendetcx commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781353004
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
It is what it is :-) thanks for confirming the timeline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1046760243
There remain a lot of dependencies on log4j 1.x even after these changes. I'd suggest you should take a look at some of the AMQ 8472 related reload4j changes again and incorporating related aspects to address them (e.g perhaps also using the log4j2 api bridge for them if needed).
> $ mvn dependency:tree | grep " log4j:log4j"
[INFO] | +- log4j:log4j:jar:1.2.17:test
[INFO] | +- log4j:log4j:jar:1.2.12:compile (optional)
[INFO] | +- log4j:log4j:jar:1.2.12:compile (optional)
[INFO] | +- log4j:log4j:jar:1.2.12:compile (optional)
[INFO] | +- log4j:log4j:jar:1.2.17:provided
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | | +- log4j:log4j:jar:1.2.12:compile
[INFO] | \- log4j:log4j:jar:1.2.17:compile
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | +- log4j:log4j:jar:1.2.17:compile
[INFO] | | +- log4j:log4j:jar:1.2.17:compile
[INFO] | +- log4j:log4j:jar:1.2.12:compile
[INFO] | +- log4j:log4j:jar:1.2.17:compile
> $ mvn dependency:tree | grep "slf4j-log4j12"
[INFO] | +- org.slf4j:slf4j-log4j12:jar:1.7.25:provided
[INFO] | +- org.slf4j:slf4j-log4j12:jar:1.7.25:compile
[INFO] | | +- org.slf4j:slf4j-log4j12:jar:1.7.25:compile
[INFO] | +- org.slf4j:slf4j-log4j12:jar:1.7.25:compile
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1046768945
I will review the dependencies. Regarding log4j properties, it uses the same as before. So I guess the comment is not related to log4j2 update but more general comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1047826619
Still seeing a bunch of log4j 1.x deps, less than before but still several. Same number of slf4j-log4j12 deps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812943999
##########
File path: activemq-all/pom.xml
##########
@@ -109,9 +105,8 @@
<include>org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec</include>
<include>org.apache.geronimo.specs:geronimo-annotation_1.0_spec</include>
<include>org.slf4j:slf4j-api</include>
- <include>org.slf4j:slf4j-log4j12</include>
+ <include>org.apache.logging:log4j-slf4j-impl</include>
Review comment:
OK, got it. Yes, dependencyManagement should stiff contain version for jcl-over-slf4j. I do it, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr edited a comment on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr edited a comment on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048844817
> Still some remaining log4j 1 instances (from zookeeper and pax stuff, as on the reload4j PR):
This much should be expected given it was one of the main things in the reload4j PR, e.g https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R623-R656 and https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R491-R512
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048848097
@gemmellr let me double check, I thought I did it already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] armandomiani commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
armandomiani commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r772648942
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Maybe 2.17 ? :) https://security-tracker.debian.org/tracker/CVE-2021-45105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] PSALINGUE commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
PSALINGUE commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r773003910
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Yes, it would be good to update to log4j 2.17
Is it the target for [ActiveMQ 5.17.0](https://activemq.apache.org/news/cve-2021-44228) ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r782036576
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Its perhaps a little imprecise to say Log4J 1.x isnt EOL or that EOL doesnt exist, when it hasnt seen a release in a decade, was specifically announced as being EOL over 6 years ago and noted as such on its website, and was just reaffirmed last week as still being EOL [1] after its PMC decided against [2] doing new releases for it. (Though I know there has been activity since around doing a new version/fork elsewhere for an update release of sorts).
[1] https://lists.apache.org/thread/dlz8nyrsvffmgq29d354s0l484lfc83w
[2] https://lists.apache.org/thread/qdspyw2khhyz887nsj9p16pb9c0c5qgm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] tallpsmith commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
tallpsmith commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781738258
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
Not every corporation allows their staff to contribute back to open source (tricky copyright), so I wouldn't say "great position" for all companies (certainly not mine).
The irony of me being a previous active log4j contributor many moons ago is not lost on me here either.
You are correct that companies could have chosen to upgrade/migrate to Artemis. But, if ActiveMQ classic is still "supported" in the sense that it is not _officially_ listed as EOL (like log4j 1.x is), then it would be _preferabble_ if dependencies are reviewed, and older ones migrated away from.
As a previous open-source contributor, I totally understand this is a community effort, and I personally value all the hard work being done by volunteers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r782451352
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
> Not every corporation allows their staff to contribute back to open source (tricky copyright), so I wouldn't say "great position" for all companies (certainly not mine).
@tallpsmith, that's fair enough. I should have said, "...any vendor/supplier _who chooses_ to participate in an open source project (e.g. ActiveMQ) is in a great position..." Lots of corporations have figured out how to participate in (i.e. not just _use_) open source, and they are in a great position to get exactly what they want from those projects, usually to the benefit of the whole community. Any company that doesn't participate has _chosen_ not to - for whatever reason. Those companies don't make particularly healthy members of the community. As noted in the recent [ASF position paper](https://cwiki.apache.org/confluence/display/COMDEV/Position+Paper), "Community is defined by those who show up and do the work." But I digress.
> ...it would be _preferabble_ if dependencies are reviewed, and older ones migrated away from.
That's exactly what this PR is about. Nobody on this thread is advocating otherwise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] tallpsmith commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
tallpsmith commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781691649
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
I don't want to beat a dead horse, and glad "hope" is not required but that's correct, FedRAMP wouldn't allow log4j 1.x.
It makes vendors/suppliers of products that use something like, say ActiveMQ "Classic" internally, to make some tough decisions.
If ActiveMQ "classic" is a supported distribution, it _should_ try to remove EOL dependencies (as you're doing).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbertram commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r781697904
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
@tallpsmith, do you want 5.17.0 to use the latest Log4j 2.x so you can use it in a FedRAMP environment and you haven't been able to so far? If you wanted to use ActiveMQ in such an environment you could have been using ActiveMQ Artemis for the last few years - at least as far as Log4j is concerned.
In any event, any vendor/supplier who uses an open source project (e.g. ActiveMQ) is in a _great_ position whether that project uses Log4j 1.x or not due to the simple fact that they can get involved in that project and implement the changes they need along with the rest of the community. The Jira to upgrade ActiveMQ "Classic" to Log4j 2.x has been open for almost 2 years now. _Anybody_ could have jumped in, implemented the necessary changes, and sent a PR.
I'm not sure what you mean by "supported distribution." You can find details about support on the [ActiveMQ website](https://activemq.apache.org/support).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] LLCampos commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
LLCampos commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r768724623
##########
File path: pom.xml
##########
@@ -82,7 +80,8 @@
<junit-version>4.13.2</junit-version>
<hamcrest-version>1.3</hamcrest-version>
<karaf-version>4.2.10</karaf-version>
- <log4j-version>1.2.17</log4j-version>
+ <slf4j-version>1.7.30</slf4j-version>
+ <log4j-version>2.14.1</log4j-version>
Review comment:
We probably want `2.15.0` or higher considering [Log4Shell](https://www.lunasec.io/docs/blog/log4j-zero-day/#log4j-v2) :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048820698
@gemmellr PR updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr edited a comment on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr edited a comment on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1048844817
> Still some remaining log4j 1 instances (from zookeeper and pax stuff, as on the reload4j PR):
This much should be expected given it was one of the main things in the reload4j PR, e.g https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R623-R656 and https://github.com/apache/activemq/commit/1f01a61426e1f783b0952bb4298c6aeeaa1e5011#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R491-R512
A likely difference in this case might be the addition of the log4j2 bridge for the 1.2 api.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812947821
##########
File path: assembly/pom.xml
##########
@@ -361,18 +351,18 @@
</dependency>
<!-- copied dependencies from activemq-web-console -->
- <!-- enable commons-logging when inside jetty6:run -->
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-log4j12</artifactId>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
+ <version>${slf4j-version}</version>
Review comment:
Redundant version declaration, it is (once again) managed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on a change in pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on a change in pull request #662:
URL: https://github.com/apache/activemq/pull/662#discussion_r812978267
##########
File path: pom.xml
##########
@@ -571,10 +572,71 @@
<version>${osgi-version}</version>
</dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-api</artifactId>
+ <version>${pax-logging-version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-service</artifactId>
+ <version>${pax-logging-version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>log4j</groupId>
+ <artifactId>apache-log4j-extras</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.ops4j.pax.logging</groupId>
+ <artifactId>pax-logging-log4j2</artifactId>
+ <version>${pax-logging-version}</version>
+ </dependency>
+
<dependency>
<groupId>org.apache.hadoop.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
<version>${zookeeper-version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>io.netty</groupId>
+ <artifactId>netty</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.zookeeper</groupId>
+ <artifactId>zookeeper</artifactId>
+ <version>${zookeeper-version}</version>
+ </dependency>
Review comment:
Yup, my bad, I'm fixing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #662: [AMQ-7426] Upgrade to log4j2
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #662:
URL: https://github.com/apache/activemq/pull/662#issuecomment-1016188305
Runtime/assembly works fine. Now I have to fix some tests which are using log4j (logger/appender).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org