This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch release/2.21.0 in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit c971b2cc940fd39c310704b1e07ceedbb2e9ec7b Author: Ammar Awad <[email protected]> AuthorDate: Thu Oct 5 16:31:03 2023 +0300 Warn on incorrect highlight style (#1545, #1637) --- .../log4j/core/pattern/HighlightConverterTest.java | 20 +++++++++++++++ .../logging/log4j/core/pattern/AnsiEscape.java | 3 +++ .../log4j/core/pattern/HighlightConverter.java | 14 +++++------ ...e_of_incorrect_syntax_of_highlighting_style.xml | 29 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/HighlightConverterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/HighlightConverterTest.java index afbd03ad3d..f0f7266423 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/HighlightConverterTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/HighlightConverterTest.java @@ -171,6 +171,26 @@ public class HighlightConverterTest { assertEquals("INFO : message in a bottle", buffer.toString()); } + /* + This test ensure that a keyvalue pair separated by = sign must be provided in order to configure the highlighting style + */ + @Test + @UsingStatusListener + public void testBadStyleOption(final ListStatusListener listener) { + String defaultWarnColor = "yellow"; + String defaultInfoColor = "green"; + final String[] options = {"%5level", PatternParser.NO_CONSOLE_NO_ANSI + "=false, " + PatternParser.DISABLE_ANSI + + "=false, " + "LOGBACK"}; + final HighlightConverter converter = HighlightConverter.newInstance(null, options); + assertNotNull(converter); + + // As the default highlighting WARN color is Yellow while the LOGBACK color is Red + assertEquals(AnsiEscape.createSequence(defaultWarnColor), converter.getLevelStyle(Level.WARN)); + // As the default highlighting INFO color is Green while the LOGBACK color is Blue + assertEquals(AnsiEscape.createSequence(defaultInfoColor), converter.getLevelStyle(Level.INFO)); + assertThat(listener.findStatusData(Level.WARN)).hasSize(1); + } + private CharSequence toFormattedCharSeq(final HighlightConverter converter, final Level level) { final StringBuilder sb = new StringBuilder(); converter.format(Log4jLogEvent.newBuilder().setLevel(level).build(), sb); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java index 51e8c75b75..4aee2a2c3d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java @@ -347,6 +347,7 @@ public enum AnsiEscape { * Bright white background color. */ BG_BRIGHT_WHITE("107"); + private static final StatusLogger LOGGER = StatusLogger.getLogger(); private static final String DEFAULT_STYLE = CSI.getCode() + SUFFIX.getCode(); @@ -431,6 +432,8 @@ public enum AnsiEscape { final String value = keyValue[1]; final boolean escape = Arrays.binarySearch(sortedIgnoreKeys, key) < 0; map.put(key, escape ? createSequence(value.split("\\s")) : value); + } else { + LOGGER.warn("Syntax error, missing '=': Expected \"{KEY1=VALUE, KEY2=VALUE, ...}"); } } return map; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java index 439d0bc2a4..494305d696 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/HighlightConverter.java @@ -27,7 +27,6 @@ import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.util.PerformanceSensitive; -import org.apache.logging.log4j.util.Strings; import static org.apache.logging.log4j.util.Strings.toRootUpperCase; @@ -87,6 +86,10 @@ public final class HighlightConverter extends LogEventPatternConverter implement private static final String STYLE_KEY = "STYLE"; + private static final String DISABLE_ANSI_KEY = "DISABLEANSI"; + + private static final String NO_CONSOLE_NO_ANSI_KEY = "NOCONSOLENOANSI"; + private static final String STYLE_KEY_DEFAULT = "DEFAULT"; private static final String STYLE_KEY_LOGBACK = "LOGBACK"; @@ -146,11 +149,8 @@ public final class HighlightConverter extends LogEventPatternConverter implement return DEFAULT_STYLES; } // Feels like a hack. Should String[] options change to a Map<String,String>? - final String string = options[1] - .replaceAll(PatternParser.DISABLE_ANSI + "=(true|false)", Strings.EMPTY) - .replaceAll(PatternParser.NO_CONSOLE_NO_ANSI + "=(true|false)", Strings.EMPTY); - // - final Map<String, String> styles = AnsiEscape.createMap(string, new String[] {STYLE_KEY}); + final Map<String, String> styles = AnsiEscape.createMap(options[1], new String[]{STYLE_KEY, DISABLE_ANSI_KEY, + NO_CONSOLE_NO_ANSI_KEY}); final Map<String, String> levelStyles = new HashMap<>(DEFAULT_STYLES); for (final Map.Entry<String, String> entry : styles.entrySet()) { final String key = toRootUpperCase(entry.getKey()); @@ -163,7 +163,7 @@ public final class HighlightConverter extends LogEventPatternConverter implement } else { levelStyles.putAll(enumMap); } - } else { + } else if (!DISABLE_ANSI_KEY.equalsIgnoreCase(key) && !NO_CONSOLE_NO_ANSI_KEY.equalsIgnoreCase(key)) { final Level level = Level.toLevel(key, null); if (level == null) { LOGGER.warn("Setting style for yet unknown level name {}", key); diff --git a/src/changelog/.2.x.x/1545_add_warn_in_case_of_incorrect_syntax_of_highlighting_style.xml b/src/changelog/.2.x.x/1545_add_warn_in_case_of_incorrect_syntax_of_highlighting_style.xml new file mode 100644 index 0000000000..55cb7a656d --- /dev/null +++ b/src/changelog/.2.x.x/1545_add_warn_in_case_of_incorrect_syntax_of_highlighting_style.xml @@ -0,0 +1,29 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one or more + ~ contributor license agreements. See the NOTICE file distributed with + ~ this work for additional information regarding copyright ownership. + ~ The ASF licenses this file to you under the Apache License, Version 2.0 + ~ (the "License"); you may not use this file except in compliance with + ~ the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns="http://logging.apache.org/log4j/changelog" + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.0.xsd" + type="fixed"> + <issue id="1545" link="https://github.com/apache/logging-log4j2/issues/1545"/> + <author id="aawad6"/> + <author name="Ammar Awad"/> + <description format="asciidoc"> + Add a warning in case of incorrect syntax of highlighting style (without =). + e.g. %highlight{%5level}{LOGBACK} should be %highlight{%5level}{STYLE=LOGBACK} + </description> +</entry>
