logging-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cko...@apache.org
Subject logging-log4j2 git commit: [LOG4J2-2320] Fix NPE in AbstractLogger when another exception is thrown
Date Tue, 17 Apr 2018 19:22:40 GMT
Repository: logging-log4j2
Updated Branches:
  refs/heads/master e12defadf -> a58e1d5cc


[LOG4J2-2320] Fix NPE in AbstractLogger when another exception is thrown

This bug hides the original cause of the failure.


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/a58e1d5c
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/a58e1d5c
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/a58e1d5c

Branch: refs/heads/master
Commit: a58e1d5ccf856cb473b681a012b7d9d96f489916
Parents: e12defa
Author: Carter Kozak <ckozak@apache.org>
Authored: Tue Apr 17 14:43:11 2018 -0400
Committer: Carter Kozak <ckozak@apache.org>
Committed: Tue Apr 17 15:22:33 2018 -0400

----------------------------------------------------------------------
 .../logging/log4j/spi/AbstractLogger.java       |  3 +-
 .../logging/log4j/AbstractLoggerTest.java       | 81 ++++++++++++++++++++
 src/changes/changes.xml                         |  3 +
 3 files changed, 86 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a58e1d5c/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
index 012dcca..5b6ac1a 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java
@@ -2181,7 +2181,8 @@ public abstract class AbstractLogger implements ExtendedLogger, Serializable
{
             throw (LoggingException) exception;
         }
         final String format = msg.getFormat();
-        final StringBuilder sb = new StringBuilder(format.length() + 100);
+        final int formatLength = format == null ? 4 : format.length();
+        final StringBuilder sb = new StringBuilder(formatLength + 100);
         sb.append(fqcn);
         sb.append(" caught ");
         sb.append(exception.getClass().getName());

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a58e1d5c/log4j-api/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
index f4800fe..a152aad 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
@@ -16,12 +16,15 @@
  */
 package org.apache.logging.log4j;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.apache.logging.log4j.junit.StatusLoggerRule;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.ObjectMessage;
 import org.apache.logging.log4j.message.ParameterizedMessage;
@@ -29,10 +32,15 @@ import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.apache.logging.log4j.message.SimpleMessage;
 import org.apache.logging.log4j.spi.AbstractLogger;
 import org.apache.logging.log4j.spi.MessageFactory2Adapter;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.MessageSupplier;
 import org.apache.logging.log4j.util.Supplier;
+import org.junit.Rule;
 import org.junit.Test;
 
+import java.util.List;
+
 /**
  *
  */
@@ -59,6 +67,9 @@ public class AbstractLoggerTest {
     private static final Marker MARKER = MarkerManager.getMarker("TEST");
     private static final String MARKER_NAME = "TEST";
 
+    @Rule
+    public StatusLoggerRule status = new StatusLoggerRule(Level.WARN);
+
     private static final LogEvent[] EVENTS = new LogEvent[] {
         new LogEvent(null, simple, null),
         new LogEvent(MARKER_NAME, simple, null),
@@ -902,6 +913,73 @@ public class AbstractLoggerTest {
         logger.log(Level.INFO, MARKER, supplier);
     }
 
+    @Test
+    public void testMessageThrows() {
+        final ThrowableExpectingLogger logger = new ThrowableExpectingLogger(false);
+        logger.error(new TestMessage(new TestMessage.FormattedMessageSupplier() {
+            @Override
+            public String getFormattedMessage() {
+                throw new IllegalStateException("Oops!");
+            }
+        }, "Message Format"));
+        List<StatusData> statusDatalist = StatusLogger.getLogger().getStatusData();
+        StatusData mostRecent = statusDatalist.get(statusDatalist.size() - 1);
+        assertEquals(Level.WARN, mostRecent.getLevel());
+        assertThat(mostRecent.getFormattedStatus(), containsString(
+                "org.apache.logging.log4j.spi.AbstractLogger caught " +
+                        "java.lang.IllegalStateException logging TestMessage: Message Format"));
+    }
+
+    @Test
+    public void testMessageThrowsAndNullFormat() {
+        final ThrowableExpectingLogger logger = new ThrowableExpectingLogger(false);
+        logger.error(new TestMessage(new TestMessage.FormattedMessageSupplier() {
+            @Override
+            public String getFormattedMessage() {
+                throw new IllegalStateException("Oops!");
+            }
+        }, null /* format */));
+        List<StatusData> statusDatalist = StatusLogger.getLogger().getStatusData();
+        StatusData mostRecent = statusDatalist.get(statusDatalist.size() - 1);
+        assertEquals(Level.WARN, mostRecent.getLevel());
+        assertThat(mostRecent.getFormattedStatus(), containsString(
+                "org.apache.logging.log4j.spi.AbstractLogger caught " +
+                        "java.lang.IllegalStateException logging TestMessage: "));
+    }
+
+    private static final class TestMessage implements Message {
+        private final FormattedMessageSupplier formattedMessageSupplier;
+        private final String format;
+        TestMessage(FormattedMessageSupplier formattedMessageSupplier, String format) {
+            this.formattedMessageSupplier = formattedMessageSupplier;
+            this.format = format;
+        }
+
+        @Override
+        public String getFormattedMessage() {
+            return formattedMessageSupplier.getFormattedMessage();
+        }
+
+        @Override
+        public String getFormat() {
+            return format;
+        }
+
+        @Override
+        public Object[] getParameters() {
+            return new Object[0];
+        }
+
+        @Override
+        public Throwable getThrowable() {
+            return null;
+        }
+
+        interface FormattedMessageSupplier {
+            String getFormattedMessage();
+        }
+    }
+
     private static class CountingLogger extends AbstractLogger {
         private static final long serialVersionUID = -3171452617952475480L;
 
@@ -1229,6 +1307,9 @@ public class AbstractLoggerTest {
             } else {
                 assertNull("Expected null but received a Throwable! "+t, t);
             }
+            if (message != null) {
+                message.getFormattedMessage();
+            }
         }
 
         @Override

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a58e1d5c/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 1020d08..c3e36b7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -142,6 +142,9 @@
       <action issue="LOG4J2-2318" dev="ckozak" type="fix">
         Messages are no longer mutated when the asynchronous queue is full. A warning is
logged to the status logger instead.
       </action>
+      <action issue="LOG4J2-2320" dev="ckozak" type="fix">
+        Fix NPE in AbstractLogger when another exception is thrown, masking the root cause.
+      </action>
     </release>
     <release version="2.11.1" date="2018-MM-DD" description="GA Release 2.11.1">
       <action issue="LOG4J2-2268" dev="rgoers" type="fix" due-to="Tilman Hausherr">


Mime
View raw message