Uploaded image for project: 'Log4j 2'
  1. Log4j 2
  2. LOG4J2-922

Parameter of mdcId in SyslogAppender has no default value.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0
    • Component/s: Appenders
    • Labels:
      None

      Description

      I don't need MDC information, but must setting mdcId. if not set mdcId, throw a IllegalArgumentException;

      Configruation
      <Syslog name="RFC5424" charset="UTF-8" format="RFC5424" host="192.168.65.181" port="13514" protocol="UDP"
                      appName="testApp" includeMDC="false" facility="USER"
                      newline="true" messageId="Audit" "/>
      
      exception info
      Caused by: java.lang.IllegalArgumentException: No structured id name was supplied
      	at org.apache.logging.log4j.message.StructuredDataId.<init>(StructuredDataId.java:92)
      	at org.apache.logging.log4j.core.layout.Rfc5424Layout.<init>(Rfc5424Layout.java:139)
      	at org.apache.logging.log4j.core.layout.Rfc5424Layout.createLayout(Rfc5424Layout.java:657)
      	at org.apache.logging.log4j.core.appender.SyslogAppender.createAppender(SyslogAppender.java:133)
      	... 25 more
      

        Activity

        Hide
        garydgregory Gary Gregory added a comment - - edited

        This has been fixed for a long time ago:

        commit a2d8c1dac0f3ce6534ee44f1c6cbfba720cd9164
        Author: Ralph Goers <rgoers@apache.org> 2011-08-06 15:44:31
        Committer: Ralph Goers <rgoers@apache.org> 2011-08-06 15:44:31
        Parent: e9a6ec8929ccaf84cbd23e358c8f97c3a9863d4d (Add JMS appenders)
        Child: 22a54040eb087ff8ace1a91d10ddc3759f7481d4 (ThreadContext Map and Stack can be removed. Correct places where MDC was returning an Object)
        Branches: AutoCloseableLock, LOG4J-1181, LOG4J2-1136, LOG4J2-1161, LOG4J2-1278-gc-free-level-logger, LOG4J2-1395, LOG4J2-599-MoveLogger2ToLogger, LOG4J2-952, StyledMessage, StyledXException, ggregory-casandra, level-logger, master, origin/2.0-beta1, origin/AutoCloseableLock, origin/HEAD, origin/LOG4J-1012, origin/LOG4J-1181, origin/LOG4J2-1116, origin/LOG4J2-1121A, origin/LOG4J2-1121B-ReliabilityStrategy, origin/LOG4J2-1136 and 35 more branches
        
        Allow some MDC fields to be required
        
        git-svn-id: https://svn.apache.org/repos/asf/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers@1154618 13f79535-47bb-0310-9956-ffa450edef68
        
        diff --git a/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java b/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java
        index f591254..73d9749 100644
        --- a/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java
        +++ b/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java
        @@ -19,6 +19,7 @@
         import org.apache.logging.log4j.LogManager;
         import org.apache.logging.log4j.core.LogEvent;
         import org.apache.logging.log4j.core.LoggerContext;
        +import org.apache.logging.log4j.core.LoggingException;
         import org.apache.logging.log4j.core.config.plugins.Plugin;
         import org.apache.logging.log4j.core.config.plugins.PluginAttr;
         import org.apache.logging.log4j.core.config.plugins.PluginFactory;
        @@ -58,9 +59,12 @@
             private final String configName;
             private final List<String> mdcExcludes;
             private final List<String> mdcIncludes;
        +    private final List<String> mdcRequired;
             private final ListChecker checker;
             private final ListChecker noopChecker = new NoopChecker();
             private final boolean includeNewLine;
        +
        +    private static final String DEFAULT_MDCID = "mdc";
         
             private long lastTimestamp = -1;
             private String timestamppStr = null;
        @@ -73,7 +77,8 @@
             public static final String DEFAULT_ID = "Audit";
         
             public RFC5424Layout(Facility facility, String id, int ein, boolean includeMDC, boolean includeNL, String mdcId,
        -                         String appName, String messageId, String excludes, String includes, Charset charset) {
        +                         String appName, String messageId, String excludes, String includes, String required,
        +                         Charset charset) {
                 super(charset);
                 this.facility = facility;
                 this.defaultId = id == null ? DEFAULT_ID : id;
        @@ -112,6 +117,20 @@
                     }
                 } else {
                     mdcIncludes = null;
        +        }
        +        if (required != null) {
        +            String[] array = required.split(",");
        +            if (array.length > 0) {
        +                mdcRequired = new ArrayList<String>(array.length);
        +                for (String str : array) {
        +                    mdcRequired.add(str.trim());
        +                }
        +            } else {
        +                mdcRequired = null;
        +            }
        +
        +        } else {
        +            mdcRequired = null;
                 }
                 this.checker = c != null ? c : noopChecker;
                 LoggerContext ctx = (LoggerContext) LogManager.getContext();
        @@ -153,24 +172,33 @@
                     buf.append("-");
                 }
                 buf.append(" ");
        -        if (isStructured) {
        -            StructuredDataMessage data = (StructuredDataMessage) msg;
        -            Map map = data.getData();
        -            StructuredDataId id = data.getId();
        -            formatStructuredElement(id, map, buf, noopChecker);
        +        if (isStructured || includeMDC) {
        +            StructuredDataId id = null;
        +            String text = "";
        +            if (isStructured) {
        +                StructuredDataMessage data = (StructuredDataMessage) msg;
        +                Map map = data.getData();
        +                id = data.getId();
        +                formatStructuredElement(id, map, buf, noopChecker);
        +                text = data.getMessageFormat();
        +            } else {
        +                text = msg.getFormattedMessage();
        +            }
                     if (includeMDC)
                     {
        -                int ein = id.getEnterpriseNumber() < 0 ? enterpriseNumber : id.getEnterpriseNumber();
        +                if (mdcRequired != null) {
        +                    checkRequired(event.getContextMap());
        +                }
        +                int ein = id == null || id.getEnterpriseNumber() < 0 ? enterpriseNumber : id.getEnterpriseNumber();
                         StructuredDataId mdcSDID = new StructuredDataId(mdcId, ein, null, null);
                         formatStructuredElement(mdcSDID, event.getContextMap(), buf, checker);
                     }
        -            String text = data.getMessageFormat();
                     if (text != null && text.length() > 0) {
                         buf.append(" ").append(text);
                     }
                 } else {
                     buf.append("- ");
        -            buf.append(event.getMessage().getFormattedMessage());
        +            buf.append(msg.getFormattedMessage());
                 }
                 if (includeNewLine) {
                     buf.append("\n");
        @@ -197,6 +225,14 @@
                     logger.error("Could not determine local host name", uhe);
                     return "UNKNOWN_LOCALHOST";
                 }
        +    }
        +
        +    public List<String> getMdcExcludes() {
        +        return mdcExcludes;
        +    }
        +
        +    public List<String> getMdcIncludes() {
        +        return mdcIncludes;
             }
         
             private String computeTimeStampString(long now) {
        @@ -294,6 +330,15 @@
                 return sb.toString();
             }
         
        +    private void checkRequired(Map<String, Object> map) {
        +        for (String key : mdcRequired) {
        +            Object value = map.get(key);
        +            if (value == null) {
        +                throw new LoggingException("Required key " + key + " is missing from the " + mdcId);
        +            }
        +        }
        +    }
        +
             private void appendMap(Map<String, Object> map, StringBuilder sb, ListChecker checker)
             {
                 SortedMap<String, Object> sorted = new TreeMap<String, Object>(map);
        @@ -338,7 +383,8 @@
                                                      @PluginAttr("appName") String appName,
                                                      @PluginAttr("messageId") String msgId,
                                                      @PluginAttr("mdcExcludes") String excludes,
        -                                             @PluginAttr("mdcINcludes") String includes,
        +                                             @PluginAttr("mdcIncludes") String includes,
        +                                             @PluginAttr("mdcRequired") String required,
                                                      @PluginAttr("charset") String charset) {
                 Charset c = Charset.isSupported("UTF-8") ? Charset.forName("UTF-8") : Charset.defaultCharset();
                 if (charset != null) {
        @@ -356,8 +402,11 @@
                 int enterpriseNumber = ein == null ? DEFAULT_ENTERPRISE_NUMBER : Integer.parseInt(ein);
                 boolean isMdc = includeMDC == null ? true : Boolean.valueOf(includeMDC);
                 boolean includeNewLine = includeNL == null ? false : Boolean.valueOf(includeNL);
        +        if (mdcId == null) {
        +            mdcId = DEFAULT_MDCID;
        +        }
         
                 return new RFC5424Layout(f, id, enterpriseNumber, isMdc, includeNewLine, mdcId, appName, msgId, excludes,
        -                                 includes, c);
        +                                 includes, required, c);
             }
         }
        
        Show
        garydgregory Gary Gregory added a comment - - edited This has been fixed for a long time ago: commit a2d8c1dac0f3ce6534ee44f1c6cbfba720cd9164 Author: Ralph Goers <rgoers@apache.org> 2011-08-06 15:44:31 Committer: Ralph Goers <rgoers@apache.org> 2011-08-06 15:44:31 Parent: e9a6ec8929ccaf84cbd23e358c8f97c3a9863d4d (Add JMS appenders) Child: 22a54040eb087ff8ace1a91d10ddc3759f7481d4 (ThreadContext Map and Stack can be removed. Correct places where MDC was returning an Object) Branches: AutoCloseableLock, LOG4J-1181, LOG4J2-1136, LOG4J2-1161, LOG4J2-1278-gc-free-level-logger, LOG4J2-1395, LOG4J2-599-MoveLogger2ToLogger, LOG4J2-952, StyledMessage, StyledXException, ggregory-casandra, level-logger, master, origin/2.0-beta1, origin/AutoCloseableLock, origin/HEAD, origin/LOG4J-1012, origin/LOG4J-1181, origin/LOG4J2-1116, origin/LOG4J2-1121A, origin/LOG4J2-1121B-ReliabilityStrategy, origin/LOG4J2-1136 and 35 more branches Allow some MDC fields to be required git-svn-id: https://svn.apache.org/repos/asf/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers@1154618 13f79535-47bb-0310-9956-ffa450edef68 diff --git a/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java b/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java index f591254..73d9749 100644 --- a/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java +++ b/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/RFC5424Layout.java @@ -19,6 +19,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.LoggingException; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttr; import org.apache.logging.log4j.core.config.plugins.PluginFactory; @@ -58,9 +59,12 @@ private final String configName; private final List<String> mdcExcludes; private final List<String> mdcIncludes; + private final List<String> mdcRequired; private final ListChecker checker; private final ListChecker noopChecker = new NoopChecker(); private final boolean includeNewLine; + + private static final String DEFAULT_MDCID = "mdc"; private long lastTimestamp = -1; private String timestamppStr = null; @@ -73,7 +77,8 @@ public static final String DEFAULT_ID = "Audit"; public RFC5424Layout(Facility facility, String id, int ein, boolean includeMDC, boolean includeNL, String mdcId, - String appName, String messageId, String excludes, String includes, Charset charset) { + String appName, String messageId, String excludes, String includes, String required, + Charset charset) { super(charset); this.facility = facility; this.defaultId = id == null ? DEFAULT_ID : id; @@ -112,6 +117,20 @@ } } else { mdcIncludes = null; + } + if (required != null) { + String[] array = required.split(","); + if (array.length > 0) { + mdcRequired = new ArrayList<String>(array.length); + for (String str : array) { + mdcRequired.add(str.trim()); + } + } else { + mdcRequired = null; + } + + } else { + mdcRequired = null; } this.checker = c != null ? c : noopChecker; LoggerContext ctx = (LoggerContext) LogManager.getContext(); @@ -153,24 +172,33 @@ buf.append("-"); } buf.append(" "); - if (isStructured) { - StructuredDataMessage data = (StructuredDataMessage) msg; - Map map = data.getData(); - StructuredDataId id = data.getId(); - formatStructuredElement(id, map, buf, noopChecker); + if (isStructured || includeMDC) { + StructuredDataId id = null; + String text = ""; + if (isStructured) { + StructuredDataMessage data = (StructuredDataMessage) msg; + Map map = data.getData(); + id = data.getId(); + formatStructuredElement(id, map, buf, noopChecker); + text = data.getMessageFormat(); + } else { + text = msg.getFormattedMessage(); + } if (includeMDC) { - int ein = id.getEnterpriseNumber() < 0 ? enterpriseNumber : id.getEnterpriseNumber(); + if (mdcRequired != null) { + checkRequired(event.getContextMap()); + } + int ein = id == null || id.getEnterpriseNumber() < 0 ? enterpriseNumber : id.getEnterpriseNumber(); StructuredDataId mdcSDID = new StructuredDataId(mdcId, ein, null, null); formatStructuredElement(mdcSDID, event.getContextMap(), buf, checker); } - String text = data.getMessageFormat(); if (text != null && text.length() > 0) { buf.append(" ").append(text); } } else { buf.append("- "); - buf.append(event.getMessage().getFormattedMessage()); + buf.append(msg.getFormattedMessage()); } if (includeNewLine) { buf.append("\n"); @@ -197,6 +225,14 @@ logger.error("Could not determine local host name", uhe); return "UNKNOWN_LOCALHOST"; } + } + + public List<String> getMdcExcludes() { + return mdcExcludes; + } + + public List<String> getMdcIncludes() { + return mdcIncludes; } private String computeTimeStampString(long now) { @@ -294,6 +330,15 @@ return sb.toString(); } + private void checkRequired(Map<String, Object> map) { + for (String key : mdcRequired) { + Object value = map.get(key); + if (value == null) { + throw new LoggingException("Required key " + key + " is missing from the " + mdcId); + } + } + } + private void appendMap(Map<String, Object> map, StringBuilder sb, ListChecker checker) { SortedMap<String, Object> sorted = new TreeMap<String, Object>(map); @@ -338,7 +383,8 @@ @PluginAttr("appName") String appName, @PluginAttr("messageId") String msgId, @PluginAttr("mdcExcludes") String excludes, - @PluginAttr("mdcINcludes") String includes, + @PluginAttr("mdcIncludes") String includes, + @PluginAttr("mdcRequired") String required, @PluginAttr("charset") String charset) { Charset c = Charset.isSupported("UTF-8") ? Charset.forName("UTF-8") : Charset.defaultCharset(); if (charset != null) { @@ -356,8 +402,11 @@ int enterpriseNumber = ein == null ? DEFAULT_ENTERPRISE_NUMBER : Integer.parseInt(ein); boolean isMdc = includeMDC == null ? true : Boolean.valueOf(includeMDC); boolean includeNewLine = includeNL == null ? false : Boolean.valueOf(includeNL); + if (mdcId == null) { + mdcId = DEFAULT_MDCID; + } return new RFC5424Layout(f, id, enterpriseNumber, isMdc, includeNewLine, mdcId, appName, msgId, excludes, - includes, c); + includes, required, c); } }
        Hide
        jvz Matt Sicker added a comment -

        That commit is from before 2.0 was ever released (probably from before 2.0-alpha1 even). Strange that this issue would have been reported in the first place, then.

        Show
        jvz Matt Sicker added a comment - That commit is from before 2.0 was ever released (probably from before 2.0-alpha1 even). Strange that this issue would have been reported in the first place, then.
        Hide
        pburrowesOC Paul Burrowes added a comment -

        This should be reopened and fixed. The commit above, described as fixing the bug, does not touch the faulty line in question.
        The DEFAULT_MDCID is only applied when createLayout() is called via the configuration parser. Creating a SyslogAppender calls this as programmatic configuration and thus fails. The constructor needs to also set the default as is done for defaultId:

          private Rfc5424Layout(final Configuration config, final Facility facility, final String id, final int ein,
                    final boolean includeMDC, final boolean includeNL, final String escapeNL, final String mdcId,
                    final String mdcPrefix, final String eventPrefix, final String appName, final String messageId,
                    final String excludes, final String includes, final String required, final Charset charset,
                    final String exceptionPattern, final boolean useTLSMessageFormat, final LoggerFields[] loggerFields) {
                super(charset);
                final PatternParser exceptionParser = createPatternParser(config, ThrowablePatternConverter.class);
                exceptionFormatters = exceptionPattern == null ? null : exceptionParser.parse(exceptionPattern);
                this.facility = facility;
                this.defaultId = id == null ? DEFAULT_ID : id;
                this.enterpriseNumber = ein;
                this.includeMdc = includeMDC;
                this.includeNewLine = includeNL;
                this.escapeNewLine = escapeNL == null ? null : Matcher.quoteReplacement(escapeNL);
        -        this.mdcId = mdcId;
        +        this.mdcId = id == null ? DEFAULT_MDCID : id;
                this.mdcSdId = new StructuredDataId(mdcId, enterpriseNumber, null, null);
        Show
        pburrowesOC Paul Burrowes added a comment - This should be reopened and fixed. The commit above, described as fixing the bug, does not touch the faulty line in question. The DEFAULT_MDCID is only applied when createLayout() is called via the configuration parser. Creating a SyslogAppender calls this as programmatic configuration and thus fails. The constructor needs to also set the default as is done for defaultId : private Rfc5424Layout( final Configuration config, final Facility facility, final String id, final int ein, final boolean includeMDC, final boolean includeNL, final String escapeNL, final String mdcId, final String mdcPrefix, final String eventPrefix, final String appName, final String messageId, final String excludes, final String includes, final String required, final Charset charset, final String exceptionPattern, final boolean useTLSMessageFormat, final LoggerFields[] loggerFields) { super (charset); final PatternParser exceptionParser = createPatternParser(config, ThrowablePatternConverter.class); exceptionFormatters = exceptionPattern == null ? null : exceptionParser.parse(exceptionPattern); this .facility = facility; this .defaultId = id == null ? DEFAULT_ID : id; this .enterpriseNumber = ein; this .includeMdc = includeMDC; this .includeNewLine = includeNL; this .escapeNewLine = escapeNL == null ? null : Matcher.quoteReplacement(escapeNL); - this .mdcId = mdcId; + this .mdcId = id == null ? DEFAULT_MDCID : id; this .mdcSdId = new StructuredDataId(mdcId, enterpriseNumber, null , null );
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit ce1183629fe89625a77872c7153853e7774502a6 in logging-log4j2's branch refs/heads/master from Paul Burrowes
        [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=ce11836 ]

        LOG4J2-922 Parameter of mdcId in SyslogAppender has no default value.

        Show
        jira-bot ASF subversion and git services added a comment - Commit ce1183629fe89625a77872c7153853e7774502a6 in logging-log4j2's branch refs/heads/master from Paul Burrowes [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=ce11836 ] LOG4J2-922 Parameter of mdcId in SyslogAppender has no default value.
        Hide
        garydgregory Gary Gregory added a comment -

        Fixed in git master with Paul Burrowes patch. Please verify and close.

        Show
        garydgregory Gary Gregory added a comment - Fixed in git master with Paul Burrowes patch. Please verify and close.

          People

          • Assignee:
            garydgregory Gary Gregory
            Reporter:
            angus.aqlu angus.aqlu
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development