You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2022/03/28 22:10:55 UTC

[logging-log4j2] 03/05: Log4j 1.2 bridge creates a SocketAppender instead of a SyslogAppender.

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit a035a73a7cfd2b96db97465f248d2dd091c04401
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Jan 22 11:51:36 2022 -0500

    Log4j 1.2 bridge creates a SocketAppender instead of a SyslogAppender.
    
    - Test fixtures incorrectly used an extra property for port instead of
    the host:port pair supported by Log4j 1.
    - Remove unused @SuppressWarnings value.
    - Make sure we use TCP as the default protocol in XML.
    
    Conflicts:
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java
    	src/changes/changes.xml
---
 .../org/apache/log4j/builders/AbstractBuilder.java |  6 +++++-
 .../builders/appender/SyslogAppenderBuilder.java   |  8 ++++----
 .../config/SyslogAppenderConfigurationTest.java    | 24 ++++++++++++++++------
 .../log4j1-syslog-protocol-default.properties      |  3 +--
 .../log4j1-syslog-protocol-tcp.properties          |  3 +--
 .../log4j1-syslog-protocol-udp.properties          |  3 +--
 .../log4j/core/appender/SyslogAppender.java        |  2 +-
 .../log4j/core/net/AbstractSocketManager.java      | 21 ++++++++++++++++++-
 src/changes/changes.xml                            | 10 ++++++++-
 9 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
index c6118cf..f7a482b 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
@@ -155,9 +155,13 @@ public abstract class AbstractBuilder implements Builder {
     }
 
     protected String getValueAttribute(final Element element) {
-        return substVars(element.getAttribute(VALUE_ATTR));
+        return getValueAttribute(element, null);
     }
 
+    protected String getValueAttribute(final Element element, final String defaultValue) {
+        final String attribute = element.getAttribute(VALUE_ATTR);
+        return substVars(attribute != null ? attribute : defaultValue);
+    }
 
     protected String substVars(final String value) {
         return OptionConverter.substVars(value, properties);
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java
index 31618a4..93faf26 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/SyslogAppenderBuilder.java
@@ -39,7 +39,7 @@ import org.apache.log4j.config.PropertiesConfiguration;
 import org.apache.log4j.spi.Filter;
 import org.apache.log4j.xml.XmlConfiguration;
 import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.core.appender.SocketAppender;
+import org.apache.logging.log4j.core.appender.SyslogAppender;
 import org.apache.logging.log4j.core.layout.SyslogLayout;
 import org.apache.logging.log4j.core.net.Facility;
 import org.apache.logging.log4j.core.net.Protocol;
@@ -77,7 +77,7 @@ public class SyslogAppenderBuilder extends AbstractBuilder implements AppenderBu
         AtomicReference<String> facility = new AtomicReference<>();
         AtomicReference<String> level = new AtomicReference<>();
         AtomicReference<String> host = new AtomicReference<>();
-        AtomicReference<Protocol> protocol = new AtomicReference<>();
+        AtomicReference<Protocol> protocol = new AtomicReference<>(Protocol.TCP);
         forEachElement(appenderElement.getChildNodes(), currentElement -> {
             switch (currentElement.getTagName()) {
                 case LAYOUT_TAG:
@@ -105,7 +105,7 @@ public class SyslogAppenderBuilder extends AbstractBuilder implements AppenderBu
                             break;
                         }
                         case PROTOCOL_PARAM:
-                            protocol.set(Protocol.valueOf(getValueAttribute(currentElement)));
+                            protocol.set(Protocol.valueOf(getValueAttribute(currentElement, Protocol.TCP.name())));
                             break;
                     }
                     break;
@@ -148,7 +148,7 @@ public class SyslogAppenderBuilder extends AbstractBuilder implements AppenderBu
         }
 
         org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
-        return new AppenderWrapper(SocketAppender.newBuilder()
+        return new AppenderWrapper(SyslogAppender.newSyslogAppenderBuilder()
                 .setName(name)
                 .setConfiguration(configuration)
                 .setLayout(appenderLayout)
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java
index 93eee35..a63d808 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/SyslogAppenderConfigurationTest.java
@@ -16,16 +16,21 @@
  */
 package org.apache.log4j.config;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
 import java.util.Map;
 
+import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.Appender;
-import org.apache.logging.log4j.core.appender.SocketAppender;
+import org.apache.logging.log4j.core.appender.SyslogAppender;
 import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.filter.ThresholdFilter;
+import org.apache.logging.log4j.core.layout.Rfc5424Layout;
 import org.apache.logging.log4j.core.net.AbstractSocketManager;
+import org.apache.logging.log4j.core.net.Facility;
 import org.apache.logging.log4j.core.net.Protocol;
 import org.junit.Test;
 
@@ -34,25 +39,32 @@ import org.junit.Test;
  */
 public class SyslogAppenderConfigurationTest {
 
-    private void checkProtocol(final Protocol expected, final Configuration configuration) {
+    private void check(final Protocol expected, final Configuration configuration) {
         final Map<String, Appender> appenders = configuration.getAppenders();
         assertNotNull(appenders);
         final String appenderName = "syslog";
         final Appender appender = appenders.get(appenderName);
         assertNotNull(appender, "Missing appender " + appenderName);
-        final SocketAppender socketAppender = (SocketAppender) appender;
+        final SyslogAppender syslogAppender = (SyslogAppender) appender;
         @SuppressWarnings("resource")
-        final AbstractSocketManager manager = socketAppender.getManager();
+        final AbstractSocketManager manager = syslogAppender.getManager();
         final String prefix = expected + ":";
         assertTrue(manager.getName().startsWith(prefix), () -> String.format("'%s' does not start with '%s'", manager.getName(), prefix));
+        // Threshold
+        final ThresholdFilter filter = (ThresholdFilter) syslogAppender.getFilter();
+        assertEquals(Level.DEBUG, filter.getLevel());
+        // Host
+        assertEquals("localhost", manager.getHost());
+        // Port
+        assertEquals(9999, manager.getPort());
     }
 
     private void checkProtocolPropertiesConfig(final Protocol expected, final String xmlPath) throws IOException {
-        checkProtocol(expected, TestConfigurator.configure(xmlPath).getConfiguration());
+        check(expected, TestConfigurator.configure(xmlPath).getConfiguration());
     }
 
     private void checkProtocolXmlConfig(final Protocol expected, final String xmlPath) throws IOException {
-        checkProtocol(expected, TestConfigurator.configure(xmlPath).getConfiguration());
+        check(expected, TestConfigurator.configure(xmlPath).getConfiguration());
     }
 
     @Test
diff --git a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-default.properties b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-default.properties
index 5a140e5..2bfda78 100644
--- a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-default.properties
+++ b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-default.properties
@@ -18,8 +18,7 @@
 log4j.rootLogger=DEBUG,syslog
 log4j.appender.syslog=org.apache.log4j.net.SyslogAppender
 log4j.appender.syslog.Threshold=DEBUG
-log4j.appender.syslog.syslogHost=localhost
-log4j.appender.syslog.port=9999
+log4j.appender.syslog.syslogHost=localhost:9999
 log4j.appender.syslog.header=true
 log4j.appender.syslog.Facility=LOCAL3
 log4j.appender.syslog.layout=org.apache.log4j.PatternLayout
diff --git a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-tcp.properties b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-tcp.properties
index 57ece1b..9a95669 100644
--- a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-tcp.properties
+++ b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-tcp.properties
@@ -18,8 +18,7 @@
 log4j.rootLogger=DEBUG,syslog
 log4j.appender.syslog=org.apache.log4j.net.SyslogAppender
 log4j.appender.syslog.Threshold=DEBUG
-log4j.appender.syslog.syslogHost=localhost
-log4j.appender.syslog.port=9999
+log4j.appender.syslog.syslogHost=localhost:9999
 log4j.appender.syslog.protocol=TCP
 log4j.appender.syslog.header=true
 log4j.appender.syslog.Facility=LOCAL3
diff --git a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-udp.properties b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-udp.properties
index 07ce88c..b7ab6a7 100644
--- a/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-udp.properties
+++ b/log4j-1.2-api/src/test/resources/log4j1-syslog-protocol-udp.properties
@@ -18,8 +18,7 @@
 log4j.rootLogger=DEBUG,syslog
 log4j.appender.syslog=org.apache.log4j.net.SyslogAppender
 log4j.appender.syslog.Threshold=DEBUG
-log4j.appender.syslog.syslogHost=localhost
-log4j.appender.syslog.port=9999
+log4j.appender.syslog.syslogHost=localhost:9999
 log4j.appender.syslog.protocol=UDP
 log4j.appender.syslog.header=true
 log4j.appender.syslog.Facility=LOCAL3
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
index 777cf1c..9f151d1 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
@@ -102,7 +102,7 @@ public class SyslogAppender extends SocketAppender {
         @PluginElement("LoggerFields")
         private LoggerFields[] loggerFields;
 
-        @SuppressWarnings({"resource", "unchecked"})
+        @SuppressWarnings({"resource"})
         @Override
         public SyslogAppender build() {
             final Protocol protocol = getProtocol();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/AbstractSocketManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/AbstractSocketManager.java
index 76d3e46..253143e 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/AbstractSocketManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/AbstractSocketManager.java
@@ -46,7 +46,8 @@ public abstract class AbstractSocketManager extends OutputStreamManager {
     protected final int port;
 
     /**
-     * The Constructor.
+     * Constructs a new instance.
+     *
      * @param name The unique name of this connection.
      * @param os The OutputStream to manage.
      * @param inetAddress The Internet address.
@@ -79,4 +80,22 @@ public abstract class AbstractSocketManager extends OutputStreamManager {
         result.put("address", inetAddress.getHostAddress());
         return result;
     }
+
+    /**
+     * Gets the host.
+     *
+     * @return the host.
+     */
+    public String getHost() {
+        return host;
+    }
+
+    /**
+     * Gets the port.
+     *
+     * @return the port.
+     */
+    public int getPort() {
+        return port;
+    }
 }
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index be44ffe..03d4f36 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -315,7 +315,15 @@
       </action>
       <action dev="ggregory" type="fix" due-to="Gary Gregory, Piotr P. Karwasz">
         Log4j 1.2 bridge throws ClassCastException when using SimpleLayout and others #708.
-      </action>      
+      </action>
+      <action dev="ggregory" type="fix" due-to="Gary Gregory">
+        Log4j 1.2 bridge creates a SocketAppender instead of a SyslogAppender.
+      </action>
+      <action dev="ggregory" type="fix" due-to="Gary Gregory, Piotr P. Karwasz">
+      </action>
+      <action dev="ggregory" type="fix">
+        JndiManager reverts to 2.17.0 behavior: Read the system property for each call.
+      </action>
       <action dev="ggregory" type="fix" issue="LOG4J2-3330" due-to="Mircea Lemnaru, Gary Gregory">
         Configurator.setLevel not fetching the correct LoggerContext.
       </action>