Log4j 2
  1. Log4j 2
  2. LOG4J2-682

Special characters (tab and so on) in PatternLayout do not work

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0-rc1
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      I upgraded a log4j 1.2 layout pattern containing "\t" to log4j 2:

         <param name="ConversionPattern" value="%{ISO8601}\t%m%n"/>
         ...becomes...
         <PatternLayout pattern="%{ISO8601}\t%m%n"/>
      

      The resulting output contained "\t" instead of the expected tab character.

      It seems the old DOMConfigurator applied OptionConverter.convertSpecialChars to the "value" attribute of every "param" element.

      We can't convert special characters quite as broadly anymore because it would break things like the "%replace" pattern. I considered various conversion points including the PatternLayout constructor, in XmlConfiguration.processAttributes, or in AbstractPluginVisitor.convert.

      In fact, PatternLayout.setConversionPattern actually did convertSpecialChars even though the constructor did not.

      It seems the best place to replace the 'special chars' is now in LiteralPatternConverter (see my patch). I added a test to PatternLayoutTest to demonstrate.

      I studied all the other plugin attributes and couldn't find any others where special characters might be useful, except perhaps the ColumnConfig "pattern" attribute, which happens to use PatternLayout so it now supports the special character escapes as well.

      Side note: PatternLayout.setConversionPattern was not storing the new value in the conversionPattern field since it was final. I've made it non-final (see patch) so we can store it there, just in case anyone ever calls getConversionPattern (not likely).

        Issue Links

          Activity

          Show
          Gary Gregory added a comment - - edited Serendipity! See the discussion on the user ML: https://mail-archives.apache.org/mod_mbox/logging-log4j-user/201406.mbox/%3CCAKnbemWoAXryn7UH=qMmwr=ad24La1+qv+cyO9OXxCCCJAGV_g@mail.gmail.com%3E
          Hide
          Gary Gregory added a comment - - edited

          Patch applied to trunk with additional changes, thank you.

          NOTE: This is NOT in 2.0-rc2.

          Please verify and close.

          Show
          Gary Gregory added a comment - - edited Patch applied to trunk with additional changes, thank you. NOTE: This is NOT in 2.0-rc2. Please verify and close.
          Hide
          Scott Harrington added a comment -

          I'm rebuilding a fresh 2.0-SNAPSHOT now from r1605450; will close after I verify.

          Side note: when you cleaned up the convertSpecialChars from if/then/else to switch, did you notice that the old code was incorrectly looking for a '\b' (ASCII 0x02) character where it should have been looking for a 'b' (ASCII 0x62)? This bug was present in Log4j 1.2 as well. Guess we can conclude nobody ever tried to embed a backspace character before, but that's fixed now as well, in case someone is inspired by your ProgressConsoleTest to use BS and CR characters in ways an appender was never intended.

          Show
          Scott Harrington added a comment - I'm rebuilding a fresh 2.0-SNAPSHOT now from r1605450; will close after I verify. Side note: when you cleaned up the convertSpecialChars from if/then/else to switch, did you notice that the old code was incorrectly looking for a '\b' (ASCII 0x02) character where it should have been looking for a 'b' (ASCII 0x62)? This bug was present in Log4j 1.2 as well. Guess we can conclude nobody ever tried to embed a backspace character before, but that's fixed now as well, in case someone is inspired by your ProgressConsoleTest to use BS and CR characters in ways an appender was never intended.
          Hide
          Gary Gregory added a comment -

          Yes, I fixed it as well but I should make a JIRA first. I did now: LOG4J2-686.

          Show
          Gary Gregory added a comment - Yes, I fixed it as well but I should make a JIRA first. I did now: LOG4J2-686 .
          Hide
          Scott Harrington added a comment -

          Verified as complete.

          Show
          Scott Harrington added a comment - Verified as complete.
          Hide
          Gary Gregory added a comment -

          Great, thank you for the review.

          Show
          Gary Gregory added a comment - Great, thank you for the review.

            People

            • Assignee:
              Unassigned
              Reporter:
              Scott Harrington
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development