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 2024/03/25 07:56:11 UTC

(logging-log4j2) branch 2.x updated (11209a3abf -> 4c2435976d)

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

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


    from 11209a3abf Rollback breaking change in `ReusableMessage`
     new c890ca0471 Fix issue #2380: Log messages with insufficient parameters should not throw exception.
     new f6df651de5 Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.
     new 76e1acc5d1 Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.
     new 529a672997 Fix issue 2380: use args.length to calculate noThrowableArgCount
     new 8209dfc1d6 Fix issue 2380: Log messages with sufficient parameters include exception should not give waring log
     new 4c2435976d Fix issue 2380: consider ReusableParameterizedMessage's situation

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../log4j/message/ParameterFormatterTest.java      |  35 +++++--
 .../log4j/message/ParameterizedMessageTest.java    | 101 +++++++++++++++++----
 .../logging/log4j/message/ParameterFormatter.java  |  21 +++--
 .../log4j/message/ParameterizedMessage.java        |  18 +---
 ...80_insufficient_args_in_ParameterFormatter.xml} |   5 +-
 5 files changed, 131 insertions(+), 49 deletions(-)
 copy src/changelog/{2.20.0/1197_preserve_location.xml => .2.x.x/fix_2380_insufficient_args_in_ParameterFormatter.xml} (51%)


(logging-log4j2) 06/06: Fix issue 2380: consider ReusableParameterizedMessage's situation

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 4c2435976df4877220aac965a8c6ae243140d4b6
Author: PanLongfei <24...@qq.com>
AuthorDate: Sun Mar 24 12:35:07 2024 +0800

    Fix issue 2380: consider ReusableParameterizedMessage's situation
---
 .../java/org/apache/logging/log4j/message/ParameterFormatter.java   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
index cdc0e1de69..ac96a1736f 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
@@ -246,13 +246,13 @@ final class ParameterFormatter {
 
         // #2380: check if the count of placeholder is not equal to the count of arguments
         if (analysis.placeholderCount != argCount) {
-            final int realArgCount = args.length;
-            final int noThrowableArgCount = realArgCount - ((args[realArgCount - 1] instanceof Throwable) ? 1 : 0);
+            final int noThrowableArgCount =
+                    argCount < 1 ? 0 : argCount - ((args[argCount - 1] instanceof Throwable) ? 1 : 0);
             if (analysis.placeholderCount != noThrowableArgCount) {
                 STATUS_LOGGER.warn(
                         "found {} argument placeholders, but provided {} for pattern `{}`",
                         analysis.placeholderCount,
-                        Math.min(realArgCount, argCount),
+                        argCount,
                         pattern);
             }
         }


(logging-log4j2) 03/06: Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 76e1acc5d1334b0ad78aa9d604d34489750446d5
Author: PanLongfei <24...@qq.com>
AuthorDate: Fri Mar 22 09:49:49 2024 +0800

    Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.
---
 .../org/apache/logging/log4j/message/ParameterFormatterTest.java    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
index 164aedf2ec..638ab5c0dd 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
@@ -78,10 +78,10 @@ class ParameterFormatterTest {
     }
 
     @ParameterizedTest
-    @CsvSource({"2,pan {} {},a,pan a {}", "3,pan {}{}{},a-b,pan ab{}", "1,pan {},a-b-c,pan a"})
-    void format_should_fail_on_insufficient_args(
+    @CsvSource({"2,pan {} {},a,pan a {}", "3,pan {}{}{},a b,pan ab{}", "1,pan {},a b c,pan a"})
+    void format_should_warn_on_insufficient_args(
             final int placeholderCount, final String pattern, final String argsStr, final String expectedMessage) {
-        final String[] args = argsStr.split("-");
+        final String[] args = argsStr.split(" ");
         final int argCount = args.length;
 
         String actualMessage = ParameterFormatter.format(pattern, args, argCount);


(logging-log4j2) 01/06: Fix issue #2380: Log messages with insufficient parameters should not throw exception.

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit c890ca04710a7705492786c9d81a1db670062f84
Author: PanLongfei <24...@qq.com>
AuthorDate: Tue Mar 19 22:02:31 2024 +0800

    Fix issue #2380: Log messages with insufficient parameters should not throw exception.
---
 .../log4j/message/ParameterFormatterTest.java      | 16 +++---
 .../log4j/message/ParameterizedMessageTest.java    | 62 +++++-----------------
 .../logging/log4j/message/ParameterFormatter.java  |  8 ---
 ...380_insufficient_args_in_ParameterFormatter.xml | 11 ++++
 4 files changed, 31 insertions(+), 66 deletions(-)

diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
index e31796368b..6fe4f2d519 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
@@ -17,7 +17,6 @@
 package org.apache.logging.log4j.message;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -68,15 +67,12 @@ class ParameterFormatterTest {
         }
     }
 
-    @ParameterizedTest
-    @CsvSource({"1,foo {}", "2,bar {}{}"})
-    void format_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) {
-        final int argCount = placeholderCount - 1;
-        assertThatThrownBy(() -> ParameterFormatter.format(pattern, new Object[argCount], argCount))
-                .isInstanceOf(IllegalArgumentException.class)
-                .hasMessage(
-                        "found %d argument placeholders, but provided %d for pattern `%s`",
-                        placeholderCount, argCount, pattern);
+    @Test
+    void formatWithInsufficientArgs() {
+        final String pattern = "Test message {}-{} {}";
+        final Object[] args = {"a", "b"};
+        final String message = ParameterFormatter.format(pattern, args, args.length);
+        assertThat(message).isEqualTo("Test message a-b {}");
     }
 
     @ParameterizedTest
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
index 39a5fe3994..149709a6a4 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
@@ -19,12 +19,7 @@ package org.apache.logging.log4j.message;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.math.BigDecimal;
-import java.util.List;
-import java.util.function.Supplier;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.status.StatusData;
 import org.apache.logging.log4j.test.ListStatusListener;
 import org.apache.logging.log4j.test.junit.Mutable;
 import org.apache.logging.log4j.test.junit.SerialUtil;
@@ -185,50 +180,21 @@ class ParameterizedMessageTest {
         assertThat(actual.getFormattedMessage()).isEqualTo(expected.getFormattedMessage());
     }
 
-    static Stream<Object[]> testCasesForInsufficientFormatArgs() {
-        return Stream.of(new Object[] {1, "foo {}"}, new Object[] {2, "bar {}{}"});
-    }
-
-    @ParameterizedTest
-    @MethodSource("testCasesForInsufficientFormatArgs")
-    void formatTo_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) {
-        final int argCount = placeholderCount - 1;
-        verifyFormattingFailureOnInsufficientArgs(placeholderCount, pattern, argCount, () -> {
-            final ParameterizedMessage message = new ParameterizedMessage(pattern, new Object[argCount]);
-            final StringBuilder buffer = new StringBuilder();
-            message.formatTo(buffer);
-            return buffer.toString();
-        });
+    @Test
+    void formatToWithInsufficientArgs() {
+        final String pattern = "Test message {}-{} {}";
+        final Object[] args = {"a", "b"};
+        final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
+        final StringBuilder buffer = new StringBuilder();
+        message.formatTo(buffer);
+        assertThat(buffer.toString()).isEqualTo("Test message a-b {}");
     }
 
-    @ParameterizedTest
-    @MethodSource("testCasesForInsufficientFormatArgs")
-    void format_should_fail_on_insufficient_args(final int placeholderCount, final String pattern) {
-        final int argCount = placeholderCount - 1;
-        verifyFormattingFailureOnInsufficientArgs(
-                placeholderCount, pattern, argCount, () -> ParameterizedMessage.format(pattern, new Object[argCount]));
-    }
-
-    private void verifyFormattingFailureOnInsufficientArgs(
-            final int placeholderCount,
-            final String pattern,
-            final int argCount,
-            final Supplier<String> formattedMessageSupplier) {
-
-        // Verify the formatted message
-        final String formattedMessage = formattedMessageSupplier.get();
-        assertThat(formattedMessage).isEqualTo(pattern);
-
-        // Verify the logged failure
-        final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
-        assertThat(statusDataList).hasSize(1);
-        final StatusData statusData = statusDataList.get(0);
-        assertThat(statusData.getLevel()).isEqualTo(Level.ERROR);
-        assertThat(statusData.getMessage().getFormattedMessage()).isEqualTo("Unable to format msg: %s", pattern);
-        assertThat(statusData.getThrowable())
-                .isInstanceOf(IllegalArgumentException.class)
-                .hasMessage(
-                        "found %d argument placeholders, but provided %d for pattern `%s`",
-                        placeholderCount, argCount, pattern);
+    @Test
+    void formatWithInsufficientArgs() {
+        final String pattern = "Test message {}-{} {}";
+        final Object[] args = {"a", "b"};
+        final String message = ParameterizedMessage.format(pattern, args);
+        assertThat(message).isEqualTo("Test message a-b {}");
     }
 }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
index 58436f8be3..d2bae68748 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
@@ -240,14 +240,6 @@ final class ParameterFormatter {
             return;
         }
 
-        // Fail if there are insufficient arguments
-        if (analysis.placeholderCount > args.length) {
-            final String message = String.format(
-                    "found %d argument placeholders, but provided %d for pattern `%s`",
-                    analysis.placeholderCount, args.length, pattern);
-            throw new IllegalArgumentException(message);
-        }
-
         // Fast-path for patterns containing no escapes
         if (analysis.escapedCharFound) {
             formatMessageContainingEscapes(buffer, pattern, args, argCount, analysis);
diff --git a/src/changelog/.2.x.x/fix_2380_insufficient_args_in_ParameterFormatter.xml b/src/changelog/.2.x.x/fix_2380_insufficient_args_in_ParameterFormatter.xml
new file mode 100644
index 0000000000..558d6a1f28
--- /dev/null
+++ b/src/changelog/.2.x.x/fix_2380_insufficient_args_in_ParameterFormatter.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<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.3.xsd"
+       type="fixed">
+  <issue id="2380" link="https://github.com/apache/logging-log4j2/pull/2380"/>
+  <description format="asciidoc">
+    Fix that parameterized message formatting throws an exception when there are insufficient number of parameters.
+    It previously simply didn't replace the '{}' sequence. The behavior changed in 2.21.0 and should be restored for backward compatibility.
+  </description>
+</entry>


(logging-log4j2) 02/06: Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit f6df651de571bb1fcb91c5cf5b875faa22897249
Author: PanLongfei <24...@qq.com>
AuthorDate: Fri Mar 22 09:35:35 2024 +0800

    Fix issue 2380: Log messages with insufficient parameters should not throw exception.Add test cases.
---
 .../log4j/message/ParameterFormatterTest.java      |  37 +++++--
 .../log4j/message/ParameterizedMessageTest.java    | 108 +++++++++++++++++++--
 .../logging/log4j/message/ParameterFormatter.java  |  15 ++-
 .../log4j/message/ParameterizedMessage.java        |  18 +---
 4 files changed, 145 insertions(+), 33 deletions(-)

diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
index 6fe4f2d519..164aedf2ec 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java
@@ -22,8 +22,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis;
-import org.apache.logging.log4j.test.junit.UsingStatusLoggerMock;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
@@ -32,9 +36,15 @@ import org.junit.jupiter.params.provider.MethodSource;
 /**
  * Tests {@link ParameterFormatter}.
  */
-@UsingStatusLoggerMock
+@UsingStatusListener
 class ParameterFormatterTest {
 
+    final ListStatusListener statusListener;
+
+    ParameterFormatterTest(ListStatusListener statusListener) {
+        this.statusListener = statusListener;
+    }
+
     @ParameterizedTest
     @CsvSource({
         "0,,false,",
@@ -67,12 +77,23 @@ class ParameterFormatterTest {
         }
     }
 
-    @Test
-    void formatWithInsufficientArgs() {
-        final String pattern = "Test message {}-{} {}";
-        final Object[] args = {"a", "b"};
-        final String message = ParameterFormatter.format(pattern, args, args.length);
-        assertThat(message).isEqualTo("Test message a-b {}");
+    @ParameterizedTest
+    @CsvSource({"2,pan {} {},a,pan a {}", "3,pan {}{}{},a-b,pan ab{}", "1,pan {},a-b-c,pan a"})
+    void format_should_fail_on_insufficient_args(
+            final int placeholderCount, final String pattern, final String argsStr, final String expectedMessage) {
+        final String[] args = argsStr.split("-");
+        final int argCount = args.length;
+
+        String actualMessage = ParameterFormatter.format(pattern, args, argCount);
+        assertThat(actualMessage).isEqualTo(expectedMessage);
+        final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
+        assertThat(statusDataList).hasSize(1);
+        final StatusData statusData = statusDataList.get(0);
+        assertThat(statusData.getLevel()).isEqualTo(Level.WARN);
+        assertThat(statusData.getMessage().getFormattedMessage())
+                .isEqualTo(
+                        "found %d argument placeholders, but provided %d for pattern `%s`",
+                        placeholderCount, argCount, pattern);
     }
 
     @ParameterizedTest
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
index 149709a6a4..3f6ba36da0 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
@@ -19,7 +19,12 @@ package org.apache.logging.log4j.message;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.math.BigDecimal;
+import java.util.List;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.status.StatusData;
 import org.apache.logging.log4j.test.ListStatusListener;
 import org.apache.logging.log4j.test.junit.Mutable;
 import org.apache.logging.log4j.test.junit.SerialUtil;
@@ -181,20 +186,107 @@ class ParameterizedMessageTest {
     }
 
     @Test
-    void formatToWithInsufficientArgs() {
-        final String pattern = "Test message {}-{} {}";
-        final Object[] args = {"a", "b"};
+    void formatToWithMoreArgsButNoWarn() {
+        final String pattern = "no warn {} {}";
+        final String expectedMessage = "no warn a b";
+        final Object[] args = {"a", "b", new RuntimeException()};
         final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
         final StringBuilder buffer = new StringBuilder();
         message.formatTo(buffer);
-        assertThat(buffer.toString()).isEqualTo("Test message a-b {}");
+        assertThat(buffer.toString()).isEqualTo(expectedMessage);
+        assertThat(message.getThrowable()).isInstanceOf(RuntimeException.class);
+        final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
+        assertThat(statusDataList).hasSize(0);
     }
 
     @Test
-    void formatWithInsufficientArgs() {
-        final String pattern = "Test message {}-{} {}";
-        final Object[] args = {"a", "b"};
+    void formatWithMoreArgsButNoWarn() {
+        final String pattern = "no warn {} {}";
+        final String expectedMessage = "no warn a b";
+        final Object[] args = {"a", "b", new RuntimeException()};
         final String message = ParameterizedMessage.format(pattern, args);
-        assertThat(message).isEqualTo("Test message a-b {}");
+        assertThat(message).isEqualTo(expectedMessage);
+        final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
+        assertThat(statusDataList).hasSize(0);
+    }
+
+    /**
+     * In this test cases, constructed the following scenarios: <br>
+     * <p>
+     * 1. The placeholders are greater than the count of arguments. <br>
+     * 2. The placeholders are less than the count of arguments. <br>
+     * 3. The arguments contains an exception, and the placeholder is greater than normal arguments. <br>
+     * 4. The arguments contains an exception, and the placeholder is less than the arguments.<br>
+     * All of these should logged in status logger with WARN level.
+     * </p>
+     *
+     * @return streams
+     */
+    static Stream<Object[]> testCasesForInsufficientFormatArgs() {
+        return Stream.of(
+                new Object[] {"more {} {}", 2, new Object[] {"a"}, "more a {}"},
+                new Object[] {"more {} {} {}", 3, new Object[] {"a"}, "more a {} {}"},
+                new Object[] {"less {}", 1, new Object[] {"a", "b"}, "less a"},
+                new Object[] {"less {} {}", 2, new Object[] {"a", "b", "c"}, "less a b"},
+                new Object[] {
+                    "more throwable {} {}",
+                    2,
+                    new Object[] {"a", new RuntimeException()},
+                    "more throwable a java.lang.RuntimeException"
+                },
+                new Object[] {
+                    "more throwable {} {} {}",
+                    3,
+                    new Object[] {"a", new RuntimeException()},
+                    "more throwable a java.lang.RuntimeException {}"
+                },
+                new Object[] {
+                    "less throwable {}", 1, new Object[] {"a", "b", new RuntimeException()}, "less throwable a"
+                });
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCasesForInsufficientFormatArgs")
+    void formatTo_should_warn_on_insufficient_args(
+            final String pattern, final int placeholderCount, Object[] args, final String expected) {
+        final int argCount = args == null ? 0 : args.length;
+        verifyFormattingFailureOnInsufficientArgs(pattern, placeholderCount, argCount, expected, () -> {
+            final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
+            final StringBuilder buffer = new StringBuilder();
+            message.formatTo(buffer);
+            return buffer.toString();
+        });
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCasesForInsufficientFormatArgs")
+    void format_should_warn_on_insufficient_args(
+            final String pattern, final int placeholderCount, Object[] args, final String expected) {
+        final int argCount = args == null ? 0 : args.length;
+        verifyFormattingFailureOnInsufficientArgs(
+                pattern, placeholderCount, argCount, expected, () -> ParameterizedMessage.format(pattern, args));
+    }
+
+    private void verifyFormattingFailureOnInsufficientArgs(
+            final String pattern,
+            final int placeholderCount,
+            final int argCount,
+            final String expected,
+            final Supplier<String> formattedMessageSupplier) {
+
+        // Verify the formatted message
+        final String formattedMessage = formattedMessageSupplier.get();
+        assertThat(formattedMessage).isEqualTo(expected);
+
+        // Verify the status logger warn
+        final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
+        assertThat(statusDataList).hasSize(1);
+        final StatusData statusData = statusDataList.get(0);
+        assertThat(statusData.getLevel()).isEqualTo(Level.WARN);
+        assertThat(statusData.getMessage().getFormattedMessage())
+                .isEqualTo(
+                        "found %d argument placeholders, but provided %d for pattern `%s`",
+                        placeholderCount, argCount, pattern);
+        assertThat(statusData.getThrowable()).isNull();
     }
 }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
index d2bae68748..89d657dc02 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
@@ -26,6 +26,8 @@ import java.util.Date;
 import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.Set;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.StringBuilders;
 
 /**
@@ -65,6 +67,8 @@ final class ParameterFormatter {
     private static final DateTimeFormatter DATE_FORMATTER =
             DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ").withZone(ZoneId.systemDefault());
 
+    private static final Logger STATUS_LOGGER = StatusLogger.getLogger();
+
     private ParameterFormatter() {}
 
     /**
@@ -235,11 +239,20 @@ final class ParameterFormatter {
             final MessagePatternAnalysis analysis) {
 
         // Short-circuit if there is nothing interesting
-        if (pattern == null || args == null || analysis.placeholderCount == 0) {
+        if (pattern == null || args == null || args.length == 0 || analysis.placeholderCount == 0) {
             buffer.append(pattern);
             return;
         }
 
+        // check if there are insufficient arguments that do not include Throwable arg
+        final int noThrowableArgCount = argCount - ((args[argCount - 1] instanceof Throwable) ? 1 : 0);
+        if (analysis.placeholderCount != noThrowableArgCount) {
+            final String message = String.format(
+                    "found %d argument placeholders, but provided %d for pattern `%s`",
+                    analysis.placeholderCount, args.length, pattern);
+            STATUS_LOGGER.warn(message);
+        }
+
         // Fast-path for patterns containing no escapes
         if (analysis.escapedCharFound) {
             formatMessageContainingEscapes(buffer, pattern, args, argCount, analysis);
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
index a5b27e985e..4da3642cca 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
@@ -26,9 +26,7 @@ import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Objects;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis;
-import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.Constants;
 import org.apache.logging.log4j.util.StringBuilderFormattable;
 import org.apache.logging.log4j.util.internal.SerializationUtil;
@@ -81,8 +79,6 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable {
 
     private static final long serialVersionUID = -665975803997290697L;
 
-    private static final Logger STATUS_LOGGER = StatusLogger.getLogger();
-
     private static final ThreadLocal<FormatBufferHolder> FORMAT_BUFFER_HOLDER_REF =
             Constants.ENABLE_THREADLOCALS ? ThreadLocal.withInitial(FormatBufferHolder::new) : null;
 
@@ -278,12 +274,7 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable {
             buffer.append(formattedMessage);
         } else {
             final int argCount = args != null ? args.length : 0;
-            try {
-                ParameterFormatter.formatMessage(buffer, pattern, args, argCount, patternAnalysis);
-            } catch (final Exception error) {
-                STATUS_LOGGER.error("Unable to format msg: {}", pattern, error);
-                buffer.append(pattern);
-            }
+            ParameterFormatter.formatMessage(buffer, pattern, args, argCount, patternAnalysis);
         }
     }
 
@@ -294,12 +285,7 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable {
      */
     public static String format(final String pattern, final Object[] args) {
         final int argCount = args != null ? args.length : 0;
-        try {
-            return ParameterFormatter.format(pattern, args, argCount);
-        } catch (final Exception error) {
-            STATUS_LOGGER.error("Unable to format msg: {}", pattern, error);
-            return pattern;
-        }
+        return ParameterFormatter.format(pattern, args, argCount);
     }
 
     @Override


(logging-log4j2) 05/06: Fix issue 2380: Log messages with sufficient parameters include exception should not give waring log

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8209dfc1d645ef81e07253c03281fdcfc933d50c
Author: PanLongfei <24...@qq.com>
AuthorDate: Sat Mar 23 20:21:33 2024 +0800

    Fix issue 2380: Log messages with sufficient parameters include exception should not give waring log
---
 .../log4j/message/ParameterizedMessageTest.java    | 57 +++++++++++++---------
 .../logging/log4j/message/ParameterFormatter.java  | 19 +++++---
 2 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
index 3f6ba36da0..4bd5df91be 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
@@ -185,27 +185,44 @@ class ParameterizedMessageTest {
         assertThat(actual.getFormattedMessage()).isEqualTo(expected.getFormattedMessage());
     }
 
-    @Test
-    void formatToWithMoreArgsButNoWarn() {
-        final String pattern = "no warn {} {}";
-        final String expectedMessage = "no warn a b";
-        final Object[] args = {"a", "b", new RuntimeException()};
+    /**
+     * In this test cases, constructed the following scenarios: <br>
+     * <p>
+     * 1. The arguments contains an exception, and the count of placeholder is equal to arguments include exception. <br>
+     * 2. The arguments contains an exception, and the count of placeholder is equal to arguments except exception.<br>
+     * All of these should not logged in status logger.
+     * </p>
+     *
+     * @return Streams
+     */
+    static Stream<Object[]> testCasesWithExceptionArgsButNoWarn() {
+        return Stream.of(
+                new Object[] {
+                    "with exception {} {}",
+                    new Object[] {"a", new RuntimeException()},
+                    "with exception a java.lang.RuntimeException"
+                },
+                new Object[] {
+                    "with exception {} {}", new Object[] {"a", "b", new RuntimeException()}, "with exception a b"
+                });
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCasesWithExceptionArgsButNoWarn")
+    void formatToWithExceptionButNoWarn(final String pattern, final Object[] args, final String expected) {
         final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
         final StringBuilder buffer = new StringBuilder();
         message.formatTo(buffer);
-        assertThat(buffer.toString()).isEqualTo(expectedMessage);
-        assertThat(message.getThrowable()).isInstanceOf(RuntimeException.class);
+        assertThat(buffer.toString()).isEqualTo(expected);
         final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
         assertThat(statusDataList).hasSize(0);
     }
 
-    @Test
-    void formatWithMoreArgsButNoWarn() {
-        final String pattern = "no warn {} {}";
-        final String expectedMessage = "no warn a b";
-        final Object[] args = {"a", "b", new RuntimeException()};
+    @ParameterizedTest
+    @MethodSource("testCasesWithExceptionArgsButNoWarn")
+    void formatWithExceptionButNoWarn(final String pattern, final Object[] args, final String expected) {
         final String message = ParameterizedMessage.format(pattern, args);
-        assertThat(message).isEqualTo(expectedMessage);
+        assertThat(message).isEqualTo(expected);
         final List<StatusData> statusDataList = statusListener.getStatusData().collect(Collectors.toList());
         assertThat(statusDataList).hasSize(0);
     }
@@ -228,12 +245,6 @@ class ParameterizedMessageTest {
                 new Object[] {"more {} {} {}", 3, new Object[] {"a"}, "more a {} {}"},
                 new Object[] {"less {}", 1, new Object[] {"a", "b"}, "less a"},
                 new Object[] {"less {} {}", 2, new Object[] {"a", "b", "c"}, "less a b"},
-                new Object[] {
-                    "more throwable {} {}",
-                    2,
-                    new Object[] {"a", new RuntimeException()},
-                    "more throwable a java.lang.RuntimeException"
-                },
                 new Object[] {
                     "more throwable {} {} {}",
                     3,
@@ -247,8 +258,8 @@ class ParameterizedMessageTest {
 
     @ParameterizedTest
     @MethodSource("testCasesForInsufficientFormatArgs")
-    void formatTo_should_warn_on_insufficient_args(
-            final String pattern, final int placeholderCount, Object[] args, final String expected) {
+    void formatToShouldWarnOnInsufficientArgs(
+            final String pattern, final int placeholderCount, final Object[] args, final String expected) {
         final int argCount = args == null ? 0 : args.length;
         verifyFormattingFailureOnInsufficientArgs(pattern, placeholderCount, argCount, expected, () -> {
             final ParameterizedMessage message = new ParameterizedMessage(pattern, args);
@@ -260,8 +271,8 @@ class ParameterizedMessageTest {
 
     @ParameterizedTest
     @MethodSource("testCasesForInsufficientFormatArgs")
-    void format_should_warn_on_insufficient_args(
-            final String pattern, final int placeholderCount, Object[] args, final String expected) {
+    void formatShouldWarnOnInsufficientArgs(
+            final String pattern, final int placeholderCount, final Object[] args, final String expected) {
         final int argCount = args == null ? 0 : args.length;
         verifyFormattingFailureOnInsufficientArgs(
                 pattern, placeholderCount, argCount, expected, () -> ParameterizedMessage.format(pattern, args));
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
index 9964d118b7..cdc0e1de69 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
@@ -244,14 +244,17 @@ final class ParameterFormatter {
             return;
         }
 
-        // check if there are insufficient arguments that do not include Throwable arg
-        final int realArgCount = args.length;
-        final int noThrowableArgCount = realArgCount - ((args[realArgCount - 1] instanceof Throwable) ? 1 : 0);
-        if (analysis.placeholderCount != noThrowableArgCount) {
-            final String message = String.format(
-                    "found %d argument placeholders, but provided %d for pattern `%s`",
-                    analysis.placeholderCount, realArgCount, pattern);
-            STATUS_LOGGER.warn(message);
+        // #2380: check if the count of placeholder is not equal to the count of arguments
+        if (analysis.placeholderCount != argCount) {
+            final int realArgCount = args.length;
+            final int noThrowableArgCount = realArgCount - ((args[realArgCount - 1] instanceof Throwable) ? 1 : 0);
+            if (analysis.placeholderCount != noThrowableArgCount) {
+                STATUS_LOGGER.warn(
+                        "found {} argument placeholders, but provided {} for pattern `{}`",
+                        analysis.placeholderCount,
+                        Math.min(realArgCount, argCount),
+                        pattern);
+            }
         }
 
         // Fast-path for patterns containing no escapes


(logging-log4j2) 04/06: Fix issue 2380: use args.length to calculate noThrowableArgCount

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 529a672997497380c5a69ac51dcb39bbe50d9168
Author: PanLongfei <24...@qq.com>
AuthorDate: Fri Mar 22 11:03:52 2024 +0800

    Fix issue 2380: use args.length to calculate noThrowableArgCount
---
 .../java/org/apache/logging/log4j/message/ParameterFormatter.java  | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
index 89d657dc02..9964d118b7 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java
@@ -239,17 +239,18 @@ final class ParameterFormatter {
             final MessagePatternAnalysis analysis) {
 
         // Short-circuit if there is nothing interesting
-        if (pattern == null || args == null || args.length == 0 || analysis.placeholderCount == 0) {
+        if (pattern == null || args == null || analysis.placeholderCount == 0) {
             buffer.append(pattern);
             return;
         }
 
         // check if there are insufficient arguments that do not include Throwable arg
-        final int noThrowableArgCount = argCount - ((args[argCount - 1] instanceof Throwable) ? 1 : 0);
+        final int realArgCount = args.length;
+        final int noThrowableArgCount = realArgCount - ((args[realArgCount - 1] instanceof Throwable) ? 1 : 0);
         if (analysis.placeholderCount != noThrowableArgCount) {
             final String message = String.format(
                     "found %d argument placeholders, but provided %d for pattern `%s`",
-                    analysis.placeholderCount, args.length, pattern);
+                    analysis.placeholderCount, realArgCount, pattern);
             STATUS_LOGGER.warn(message);
         }