Use FQDN in RFC5424 Layout (+review #2853)

ppkarwasz · ppkarwasz · commit b593be7d881b · 2024-08-26T20:53:09.000+02:00
Since RFC5424 specifies that the [`HOSTNAME`](https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.4) field should contain the FQDN of the current host, we replace replace the usage of `InetAddress.getHostName()` with `InetAddress.getCanonicalHostName()`. Closes #1740.
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java
@@ -16,13 +16,17 @@
  */
 package org.apache.logging.log4j.core.layout;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -751,7 +755,7 @@ public void testLayoutBuilderDefaultValues() {
                 .withConfigurationNode(new Node())
                 .withConfiguration(new DefaultConfiguration())
                 .build();
-        assertTrue(obj instanceof Rfc5424Layout);
+        assertInstanceOf(Rfc5424Layout.class, obj);
         checkDefaultValues((Rfc5424Layout) obj);
     }
 
@@ -781,4 +785,11 @@ void testLayoutBuilderInvalidEids(final String eid) {
 
         assertNull(layout);
     }
+
+    @Test
+    void testFQDN() throws UnknownHostException {
+        final String fqdn = InetAddress.getLocalHost().getCanonicalHostName();
+        final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build();
+        assertThat(layout.getLocalHostName()).isEqualTo(fqdn);
+    }
 }
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/NetUtilsTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/NetUtilsTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.logging.log4j.core.util;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
@@ -70,4 +71,10 @@ public void testToUriWindowsAbsolutePath() {
         assertNotNull(uri, "The URI should not be null.");
         assertEquals("file:/D:/path/to/something/on/windows", uri.toString(), "The URI is not correct.");
     }
+
+    @Test
+    public void testCanonicalHostName() {
+        // If this fails the host might be misconfigured
+        assertThat(NetUtils.getCanonicalLocalHostname()).contains(".");
+    }
 }
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
@@ -131,7 +131,7 @@ public SyslogAppender build() {
                                 .setExceptionPattern(exceptionPattern)
                                 .setUseTLSMessageFormat(useTlsMessageFormat)
                                 .setLoggerFields(loggerFields)
-                                .setConfig(configuration)
+                                .setConfiguration(configuration)
                                 .build()
                         :
                         // @formatter:off
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java
@@ -36,10 +36,9 @@
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.Node;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
-import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
-import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
+import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
+import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
 import org.apache.logging.log4j.core.config.plugins.PluginElement;
-import org.apache.logging.log4j.core.config.plugins.PluginFactory;
 import org.apache.logging.log4j.core.layout.internal.ExcludeChecker;
 import org.apache.logging.log4j.core.layout.internal.IncludeChecker;
 import org.apache.logging.log4j.core.layout.internal.ListChecker;
@@ -167,7 +166,7 @@ private Rfc5424Layout(
         this.appName = appName;
         this.messageId = messageId;
         this.useTlsMessageFormat = useTLSMessageFormat;
-        this.localHostName = NetUtils.getLocalHostname();
+        this.localHostName = NetUtils.getCanonicalLocalHostname();
         ListChecker checker = null;
         if (excludes != null) {
             final String[] array = excludes.split(Patterns.COMMA_SEPARATOR);
@@ -642,80 +641,122 @@ public String toString() {
      * @return An Rfc5424Layout.
      * @deprecated Use {@link Rfc5424LayoutBuilder instead}
      */
-    @PluginFactory
+    @Deprecated
     public static Rfc5424Layout createLayout(
-            // @formatter:off
-            @PluginAttribute(value = "facility", defaultString = "LOCAL0") final Facility facility,
-            @PluginAttribute("id") final String id,
-            @PluginAttribute(value = "enterpriseNumber", defaultInt = DEFAULT_ENTERPRISE_NUMBER)
-                    final int enterpriseNumber,
-            @PluginAttribute(value = "includeMDC", defaultBoolean = true) final boolean includeMDC,
-            @PluginAttribute(value = "mdcId", defaultString = DEFAULT_MDCID) final String mdcId,
-            @PluginAttribute("mdcPrefix") final String mdcPrefix,
-            @PluginAttribute("eventPrefix") final String eventPrefix,
-            @PluginAttribute(value = "newLine") final boolean newLine,
-            @PluginAttribute("newLineEscape") final String escapeNL,
-            @PluginAttribute("appName") final String appName,
-            @PluginAttribute("messageId") final String msgId,
-            @PluginAttribute("mdcExcludes") final String excludes,
-            @PluginAttribute("mdcIncludes") String includes,
-            @PluginAttribute("mdcRequired") final String required,
-            @PluginAttribute("exceptionPattern") final String exceptionPattern,
-            // RFC 5425
-            @PluginAttribute(value = "useTlsMessageFormat") final boolean useTlsMessageFormat,
-            @PluginElement("LoggerFields") final LoggerFields[] loggerFields,
-            @PluginConfiguration final Configuration config) {
-        // @formatter:on
+            final Facility facility,
+            final String id,
+            final int enterpriseNumber,
+            final boolean includeMDC,
+            final String mdcId,
+            final String mdcPrefix,
+            final String eventPrefix,
+            final boolean newLine,
+            final String escapeNL,
+            final String appName,
+            final String msgId,
+            final String excludes,
+            String includes,
+            final String required,
+            final String exceptionPattern,
+            final boolean useTlsMessageFormat,
+            final LoggerFields[] loggerFields,
+            final Configuration config) {
         if (includes != null && excludes != null) {
             LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored");
             includes = null;
         }
 
-        return new Rfc5424Layout(
-                config,
-                facility,
-                id,
-                String.valueOf(enterpriseNumber),
-                includeMDC,
-                newLine,
-                escapeNL,
-                mdcId,
-                mdcPrefix,
-                eventPrefix,
-                appName,
-                msgId,
-                excludes,
-                includes,
-                required,
-                StandardCharsets.UTF_8,
-                exceptionPattern,
-                useTlsMessageFormat,
-                loggerFields);
-    }
-
-    public static class Rfc5424LayoutBuilder {
-        private Configuration config;
+        return newBuilder()
+                .setConfiguration(config)
+                .setFacility(facility)
+                .setId(id)
+                .setEin(String.valueOf(enterpriseNumber))
+                .setIncludeMDC(includeMDC)
+                .setIncludeNL(newLine)
+                .setEscapeNL(escapeNL)
+                .setMdcId(mdcId)
+                .setMdcPrefix(mdcPrefix)
+                .setEventPrefix(eventPrefix)
+                .setAppName(appName)
+                .setMessageId(msgId)
+                .setExcludes(excludes)
+                .setIncludes(includes)
+                .setRequired(required)
+                .setCharset(StandardCharsets.UTF_8)
+                .setExceptionPattern(exceptionPattern)
+                .setUseTLSMessageFormat(useTlsMessageFormat)
+                .setLoggerFields(loggerFields)
+                .build();
+    }
+
+    @PluginBuilderFactory
+    public static Rfc5424LayoutBuilder newBuilder() {
+        return new Rfc5424LayoutBuilder();
+    }
+
+    public static class Rfc5424LayoutBuilder extends AbstractStringLayout.Builder<Rfc5424LayoutBuilder>
+            implements org.apache.logging.log4j.core.util.Builder<Rfc5424Layout> {
+
+        @PluginBuilderAttribute
         private Facility facility = Facility.LOCAL0;
+
+        @PluginBuilderAttribute
         private String id;
+
+        @PluginBuilderAttribute
         private String ein = String.valueOf(DEFAULT_ENTERPRISE_NUMBER);
+
+        @PluginBuilderAttribute
+        private Integer enterpriseNumber;
+
+        @PluginBuilderAttribute
         private boolean includeMDC = true;
+
+        @PluginBuilderAttribute
         private boolean includeNL;
+
+        @PluginBuilderAttribute
         private String escapeNL;
+
+        @PluginBuilderAttribute
         private String mdcId = DEFAULT_MDCID;
+
+        @PluginBuilderAttribute
         private String mdcPrefix;
+
+        @PluginBuilderAttribute
         private String eventPrefix;
+
+        @PluginBuilderAttribute
         private String appName;
+
+        @PluginBuilderAttribute
         private String messageId;
+
+        @PluginBuilderAttribute
         private String excludes;
+
+        @PluginBuilderAttribute
         private String includes;
+
+        @PluginBuilderAttribute
         private String required;
-        private Charset charset;
+
+        @PluginBuilderAttribute
         private String exceptionPattern;
+
+        @PluginBuilderAttribute
         private boolean useTLSMessageFormat;
+
+        @PluginElement(value = "loggerFields")
         private LoggerFields[] loggerFields;
 
+        /**
+         * @deprecated Since 2.24.0 use {@link #setConfiguration} instead.
+         */
+        @Deprecated
         public Rfc5424LayoutBuilder setConfig(final Configuration config) {
-            this.config = config;
+            setConfiguration(config);
             return this;
         }
 
@@ -789,9 +830,9 @@ public Rfc5424LayoutBuilder setRequired(final String required) {
             return this;
         }
 
+        // Kept for binary compatibility
         public Rfc5424LayoutBuilder setCharset(final Charset charset) {
-            this.charset = charset;
-            return this;
+            return super.setCharset(charset);
         }
 
         public Rfc5424LayoutBuilder setExceptionPattern(final String exceptionPattern) {
@@ -809,19 +850,24 @@ public Rfc5424LayoutBuilder setLoggerFields(final LoggerFields[] loggerFields) {
             return this;
         }
 
+        @Override
         public Rfc5424Layout build() {
             if (includes != null && excludes != null) {
                 LOGGER.error("mdcIncludes and mdcExcludes are mutually exclusive. Includes wil be ignored");
                 includes = null;
             }
 
+            if (enterpriseNumber != null) {
+                ein = String.valueOf(enterpriseNumber);
+            }
             if (ein != null && !ENTERPRISE_ID_PATTERN.matcher(ein).matches()) {
                 LOGGER.warn(String.format("provided EID %s is not in valid format!", ein));
                 return null;
             }
+            final Charset charset = getCharset();
 
             return new Rfc5424Layout(
-                    config,
+                    getConfiguration(),
                     facility,
                     id,
                     ein,
@@ -836,7 +882,7 @@ public Rfc5424Layout build() {
                     excludes,
                     includes,
                     required,
-                    charset,
+                    charset != null ? charset : StandardCharsets.UTF_8,
                     exceptionPattern,
                     useTLSMessageFormat,
                     loggerFields);
@@ -926,4 +972,9 @@ public boolean isIncludeMdc() {
     public String getMdcId() {
         return mdcId;
     }
+
+    // Used in tests
+    String getLocalHostName() {
+        return localHostName;
+    }
 }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/package-info.java
@@ -22,7 +22,7 @@
  * {@link org.apache.logging.log4j.core.Layout#ELEMENT_TYPE layout}.
  */
 @Export
-@Version("2.20.1")
+@Version("2.24.0")
 package org.apache.logging.log4j.core.layout;
 
 import org.osgi.annotation.bundle.Export;
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
@@ -30,6 +30,7 @@
 import java.util.Arrays;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.function.Function;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.status.StatusLogger;
 
@@ -52,23 +53,35 @@ private NetUtils() {
      * @return String the name of the local host
      */
     public static String getLocalHostname() {
+        return getHostname(InetAddress::getHostName);
+    }
+
+    /**
+     * This method gets the FQDN of the machine we are running on.
+     * It returns {@value UNKNOWN_LOCALHOST} if the host name cannot be found.
+     *
+     * @return The canonical name of the local host; or {@value UNKNOWN_LOCALHOST}, if cannot be found.
+     */
+    public static String getCanonicalLocalHostname() {
+        return getHostname(InetAddress::getCanonicalHostName);
+    }
+
+    private static String getHostname(final Function<? super InetAddress, ? extends String> callback) {
         try {
-            final InetAddress addr = InetAddress.getLocalHost();
-            return addr == null ? UNKNOWN_LOCALHOST : addr.getHostName();
+            final InetAddress address = InetAddress.getLocalHost();
+            return address == null ? UNKNOWN_LOCALHOST : callback.apply(address);
         } catch (final UnknownHostException uhe) {
             try {
                 final Enumeration<NetworkInterface> interfaces = NetworkInterface.getNetworkInterfaces();
-                if (interfaces != null) {
-                    while (interfaces.hasMoreElements()) {
-                        final NetworkInterface nic = interfaces.nextElement();
-                        final Enumeration<InetAddress> addresses = nic.getInetAddresses();
-                        while (addresses.hasMoreElements()) {
-                            final InetAddress address = addresses.nextElement();
-                            if (!address.isLoopbackAddress()) {
-                                final String hostname = address.getHostName();
-                                if (hostname != null) {
-                                    return hostname;
-                                }
+                while (interfaces.hasMoreElements()) {
+                    final NetworkInterface nic = interfaces.nextElement();
+                    final Enumeration<InetAddress> addresses = nic.getInetAddresses();
+                    while (addresses.hasMoreElements()) {
+                        final InetAddress address = addresses.nextElement();
+                        if (!address.isLoopbackAddress()) {
+                            final String hostname = callback.apply(address);
+                            if (hostname != null) {
+                                return hostname;
                             }
                         }
                     }
diff --git a/src/changelog/.2.x.x/1740_rfc5424_use_fqdn.xml b/src/changelog/.2.x.x/1740_rfc5424_use_fqdn.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="added">
+  <issue id="1740" link="https://github.com/apache/logging-log4j2/issues/1740"/>
+  <description format="asciidoc">Add `useFqdn` configuration attribute to RFC5424 Layout.</description>
+</entry>

Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ public SyslogAppender build() {`
`131`	`131`	`.setExceptionPattern(exceptionPattern)`
`132`	`132`	`.setUseTLSMessageFormat(useTlsMessageFormat)`
`133`	`133`	`.setLoggerFields(loggerFields)`
`134`		`- .setConfig(configuration)`
	`134`	`+ .setConfiguration(configuration)`
`135`	`135`	`.build()`
`136`	`136`	`:`
`137`	`137`	`// @formatter:off`