SOLR-12154: Disallow explicit usage of Log4j2 logger via forbidden APIs

lucene/tools/forbiddenApis/solr.txt

@@ -30,6 +30,7 @@ java.util.concurrent.ThreadPoolExecutor#<init>(int,int,long,java.util.concurrent
 
 @defaultMessage Use slf4j classes instead
 org.apache.log4j.**
+org.apache.logging.log4j.**
 java.util.logging.**
 
 @defaultMessage Use RTimer/TimeOut/System.nanoTime for time comparisons, and `new Date()` output/debugging/stats of timestamps. If for some miscellaneous reason, you absolutely need to use this, use a SuppressForbidden.

solr/CHANGES.txt

@@ -161,6 +161,8 @@ Other Changes
 
 * SOLR-12165: Ref Guide: DisMax default mm param value is improperly documented as 100%. (Steve Rowe)
 
+* SOLR-12154: Disallow explicit usage of Log4j2 logger via forbidden APIs. (Varun Thacker, Tomás Fernández Löbbe)
+
 ==================  7.3.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java

@@ -23,28 +23,31 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.google.common.base.Throwables;
 import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.core.Filter;
+import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.LoggerConfig;
-import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.filter.ThresholdFilter;
 import org.apache.logging.log4j.message.Message;
 import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.logging.CircularList;
 import org.apache.solr.logging.ListenerConfig;
 import org.apache.solr.logging.LogWatcher;
 import org.apache.solr.logging.LoggerInfo;
 
-import com.google.common.base.Throwables;
-
+@SuppressForbidden(reason = "class is specific to log4j2")
 public class Log4j2Watcher extends LogWatcher<LogEvent> {
 
   private final static String LOG4J2_WATCHER_APPENDER = "Log4j2WatcherAppender";
+
+  @SuppressForbidden(reason = "class is specific to log4j2")
   protected class Log4j2Appender extends AbstractAppender {
 
     private Log4j2Watcher watcher;
@@ -74,6 +77,7 @@ public class Log4j2Watcher extends LogWatcher<LogEvent> {
     }
   }
 
+  @SuppressForbidden(reason = "class is specific to log4j2")
   protected class Log4j2Info extends LoggerInfo {
     final Level level;
 

solr/core/src/java/org/apache/solr/util/SolrLogLayout.java

@@ -45,7 +45,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
 
-@SuppressForbidden(reason = "class is specific to log4j")
+@SuppressForbidden(reason = "class is specific to log4j2")
 @Plugin(name = "SolrLogLayout", category = "Core", elementType = "layout", printObject = true)
 public class SolrLogLayout extends AbstractStringLayout {
 
@@ -110,23 +110,9 @@ public class SolrLogLayout extends AbstractStringLayout {
   
   Map<Integer,CoreInfo> coreInfoMap = new WeakHashMap<>();
   
-  public Map<String,String> classAliases = new HashMap<>();
-  
-  public void appendThread(StringBuilder sb, LogEvent event) {
+  public void appendThread(StringBuilder sb) {
     Thread th = Thread.currentThread();
     
-    /******
-     * sb.append(" T="); sb.append(th.getName()).append(' ');
-     * 
-     * // NOTE: tried creating a thread group around jetty but we seem to lose
-     * it and request // threads are in the normal "main" thread group
-     * ThreadGroup tg = th.getThreadGroup(); while (tg != null) {
-     * sb.append("(group_name=").append(tg.getName()).append(")");
-     * 
-     * if (tg instanceof TG) { sb.append(((TG)tg).getTag()); sb.append('/'); }
-     * try { tg = tg.getParent(); } catch (Throwable e) { tg = null; } }
-     ******/
-    
     // NOTE: LogRecord.getThreadID is *not* equal to Thread.getId()
     sb.append(" T");
     sb.append(th.getId());
@@ -146,10 +132,8 @@ public class SolrLogLayout extends AbstractStringLayout {
     
     long now = event.getTimeMillis();
     long timeFromStart = now - startTime;
-    long timeSinceLast = now - lastTime;
     lastTime = now;
-    String shortClassName = getShortClassName(event.getSource().getClassName(),
-        event.getSource().getMethodName());
+    String shortClassName = getShortClassName(event.getSource().getClassName(), event.getSource().getMethodName());
     
     /***
      * sb.append(timeFromStart).append(' ').append(timeSinceLast);
@@ -165,7 +149,7 @@ public class SolrLogLayout extends AbstractStringLayout {
     try (SolrQueryRequest req = (requestInfo == null) ? null : requestInfo.getReq()) {
       core = (req == null) ? null : req.getCore();
     }
-    ZkController zkController = null;
+    ZkController zkController;
     CoreInfo info = null;
     
     if (core != null) {
@@ -206,7 +190,7 @@ public class SolrLogLayout extends AbstractStringLayout {
     // sb.append("\nL").append(record.getSequenceNumber()); // log number is
     // useful for sequencing when looking at multiple parts of a log file, but
     // ms since start should be fine.
-    appendThread(sb, event);
+    appendThread(sb);
 
     appendMDC(sb);
 

solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java

@@ -57,11 +57,11 @@ public final class StartupLoggingUtils {
   }
 
   /**
-   * Disables all log4j ConsoleAppender's by modifying log4j configuration dynamically.
+   * Disables all log4j2 ConsoleAppender's by modifying log4j configuration dynamically.
    * Must only be used during early startup
-   * @return true if ok or else false if something happened, e.g. log4j classes were not in classpath
+   * @return true if ok or else false if something happened, e.g. log4j2 classes were not in classpath
    */
-  @SuppressForbidden(reason = "Legitimate log4j access")
+  @SuppressForbidden(reason = "Legitimate log4j2 access")
   public static boolean muteConsole() {
     try {
       if (!isLog4jActive()) {
@@ -90,7 +90,7 @@ public final class StartupLoggingUtils {
    * @param logLevel String with level, should be one of the supported, e.g. TRACE, DEBUG, INFO, WARN, ERROR...
    * @return true if ok or else false if something happened, e.g. log4j classes were not in classpath
    */
-  @SuppressForbidden(reason = "Legitimate log4j access")
+  @SuppressForbidden(reason = "Legitimate log4j2 access")
   public static boolean changeLogLevel(String logLevel) {
     try {
       if (!isLog4jActive()) {

solr/core/src/test/org/apache/solr/handler/RequestLoggingTest.java

@@ -35,7 +35,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-@SuppressForbidden(reason = "test is specific to log4j")
+@SuppressForbidden(reason = "test is specific to log4j2")
 public class RequestLoggingTest extends SolrTestCaseJ4 {
   private StringWriter writer;
   private Appender appender;

solr/core/src/test/org/apache/solr/handler/admin/LoggingHandlerTest.java

@@ -25,12 +25,11 @@ import org.apache.logging.log4j.core.config.LoggerConfig;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.SuppressForbidden;
-
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 
-@SuppressForbidden(reason = "test uses log4j because it tests output at a specific level")
+@SuppressForbidden(reason = "test uses log4j2 because it tests output at a specific level")
 public class LoggingHandlerTest extends SolrTestCaseJ4 {
 
   // TODO: This only tests Log4j at the moment, as that's what's defined

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -106,6 +106,7 @@ import org.apache.solr.common.params.UpdateParams;
 import org.apache.solr.common.util.ContentStream;
 import org.apache.solr.common.util.ContentStreamBase;
 import org.apache.solr.common.util.ObjectReleaseTracker;
+import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.common.util.XML;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoresLocator;
@@ -397,6 +398,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     return result;
   }
 
+  @SuppressForbidden(reason = "Using the Level class from log4j2 directly")
   private static Map<String, Level> savedClassLogLevels = new HashMap<>();
 
   public static void initClassLogLevels() {

solr/test-framework/src/java/org/apache/solr/util/LogLevel.java

@@ -52,7 +52,7 @@ public @interface LogLevel {
    */
   public String value();
 
-  @SuppressForbidden(reason="Specific to Log4J")
+  @SuppressForbidden(reason="Specific to Log4J2")
   public static class Configurer {
 
     private static Map<String, Level> parseFrom(String input) {

solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java

@@ -23,7 +23,7 @@ import org.apache.solr.common.util.SuppressForbidden;
 import org.apache.solr.util.LogLevel;
 import org.junit.Test;
 
-@SuppressForbidden(reason="We need to use log4J classes to access the log levels")
+@SuppressForbidden(reason="We need to use log4J2 classes to access the log levels")
 @LogLevel("org.apache.solr.ClassLogLevel=error;org.apache.solr.MethodLogLevel=warn")
 public class TestLogLevelAnnotations extends SolrTestCaseJ4 {