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/04/08 18:55:27 UTC

[logging-log4j2] branch release-2.x updated: [LOG4J2-2872] Normalizes level names

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

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


The following commit(s) were added to refs/heads/release-2.x by this push:
     new 7f0e350fe8 [LOG4J2-2872] Normalizes level names
7f0e350fe8 is described below

commit 7f0e350fe8aba6b5eaa33ff2bcb6e682955b0ad9
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Thu Apr 7 22:49:32 2022 +0200

    [LOG4J2-2872] Normalizes level names
    
    Calling `Level.forName` with a non upper case argument, can create a
    custom level that can not be retrieved using `Level.toLevel`.
    
    This PR fixes to upper case the keys in the `Level.LEVELS` map.
---
 .../apache/log4j/helpers/UtilLoggingLevelTest.java    |  7 +++++--
 .../src/main/java/org/apache/logging/log4j/Level.java | 18 +++++++++++++-----
 .../test/java/org/apache/logging/log4j/LevelTest.java | 19 +++++++++++++++++--
 3 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/UtilLoggingLevelTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/UtilLoggingLevelTest.java
index 2d49b43b1c..3b53aa668d 100644
--- a/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/UtilLoggingLevelTest.java
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/helpers/UtilLoggingLevelTest.java
@@ -18,6 +18,7 @@
 package org.apache.log4j.helpers;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
 import java.util.stream.Stream;
@@ -49,9 +50,11 @@ public class UtilLoggingLevelTest {
     @ParameterizedTest
     @MethodSource("namesAndLevels")
     public void testOptionConverterToLevel(final String name, final UtilLoggingLevel level) {
-        assertEquals(level, OptionConverter.toLevel(name, Level.ALL));
+        assertTrue(level == OptionConverter.toLevel(name, Level.ALL), "get v1 level by name");
         // Comparison of Log4j 2.x levels
-        assertEquals(level.getVersion2Level(), org.apache.logging.log4j.Level.getLevel(name));
+        assertTrue(level.getVersion2Level() == org.apache.logging.log4j.Level.getLevel(name), "get v2 level by name");
+        // Test convertLevel
+        assertTrue(level == OptionConverter.convertLevel(level.getVersion2Level()), "convert level v2 -> v1");
     }
 }
 
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/Level.java b/log4j-api/src/main/java/org/apache/logging/log4j/Level.java
index 48d36d1292..9f9af36b08 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/Level.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/Level.java
@@ -17,7 +17,6 @@
 package org.apache.logging.log4j;
 
 import java.io.Serializable;
-import java.util.Collection;
 import java.util.Locale;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
@@ -139,7 +138,7 @@ public final class Level implements Comparable<Level>, Serializable {
         this.name = name;
         this.intLevel = intLevel;
         this.standardLevel = StandardLevel.getStandardLevel(intLevel);
-        if (LEVELS.putIfAbsent(name, this) != null) {
+        if (LEVELS.putIfAbsent(toUpperCase(name.trim()), this) != null) {
             throw new IllegalStateException("Level " + name + " has already been defined.");
         }
     }
@@ -259,15 +258,20 @@ public final class Level implements Comparable<Level>, Serializable {
      * @throws java.lang.IllegalArgumentException if the name is null or intValue is less than zero.
      */
     public static Level forName(final String name, final int intValue) {
-        final Level level = LEVELS.get(name);
+        if (Strings.isEmpty(name)) {
+            throw new IllegalArgumentException("Illegal null or empty Level name.");
+        }
+        final String normalizedName = toUpperCase(name.trim());
+        final Level level = LEVELS.get(normalizedName);
         if (level != null) {
             return level;
         }
         try {
+            // use original capitalization
             return new Level(name, intValue);
         } catch (final IllegalStateException ex) {
             // The level was added by something else so just return that one.
-            return LEVELS.get(name);
+            return LEVELS.get(normalizedName);
         }
     }
 
@@ -276,9 +280,13 @@ public final class Level implements Comparable<Level>, Serializable {
      *
      * @param name The name of the Level.
      * @return The Level or null.
+     * @throws java.lang.IllegalArgumentException if the name is null.
      */
     public static Level getLevel(final String name) {
-        return LEVELS.get(name);
+        if (Strings.isEmpty(name)) {
+            throw new IllegalArgumentException("Illegal null or empty Level name.");
+        }
+        return LEVELS.get(toUpperCase(name.trim()));
     }
 
     /**
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/LevelTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/LevelTest.java
index 0e464882e8..6a85e357dd 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/LevelTest.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/LevelTest.java
@@ -16,9 +16,14 @@
  */
 package org.apache.logging.log4j;
 
-import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import static org.junit.jupiter.api.Assertions.*;
+import org.junit.jupiter.api.Test;
 
 public class LevelTest {
 
@@ -37,9 +42,19 @@ public class LevelTest {
         assertNotNull(level);
         assertEquals(level, Level.forName(name, intValue));
         assertEquals(level, Level.getLevel(name));
+        assertEquals(level, Level.toLevel(name));
         assertEquals(intValue, Level.getLevel(name).intLevel());
     }
 
+    @Test
+    public void testThrowsOnNull() {
+        assertThrowsExactly(IllegalArgumentException.class, () -> Level.forName(null, 100));
+        assertThrowsExactly(IllegalArgumentException.class, () -> Level.getLevel(null));
+        // the intLevel should be checked only if we create a new level
+        assertNull(Level.getLevel("Bar"));
+        assertThrowsExactly(IllegalArgumentException.class, () -> Level.forName("Bar", -1));
+    }
+
     @Test
     public void testGoodLevels() {
         final Level level = Level.toLevel("INFO");