Explorar el Código

Catch an exception due to incorrect pattern in Strings.format (#87132)

Strings.format method, which is used heavily in logging with
Supplier should handle exceptions when a format is incorrect.
This will prevent a hard to catch mistakes to blow up in server.
Those mistakes are especially hard to detect in logging when a
code to create a message might be only executed when logger is debug
or trace. Which is not always the case in CI.

relates #87077 (comment)

relates #86549
Przemyslaw Gomulka hace 3 años
padre
commit
416a1b352c

+ 5 - 0
docs/changelog/87132.yaml

@@ -0,0 +1,5 @@
+pr: 87132
+summary: Catch an exception when formatting a string fails
+area: Infra/Logging
+type: enhancement
+issues: []

+ 9 - 2
libs/core/src/main/java/org/elasticsearch/core/Strings.java

@@ -18,11 +18,18 @@ public class Strings {
     /**
      * Returns a formatted string using the specified format string and
      * arguments.
-     *
+     * <p>
      * This method calls {@link String#format(Locale, String, Object...)}
      * with Locale.ROOT
+     * If format is incorrect the function will return format without populating
+     * its variable placeholders.
      */
     public static String format(String format, Object... args) {
-        return String.format(Locale.ROOT, format, args);
+        try {
+            return String.format(Locale.ROOT, format, args);
+        } catch (Exception e) {
+            assert false : "Exception thrown when formatting [" + format + "]. " + e.getClass().getCanonicalName() + ". " + e.getMessage();
+            return format;
+        }
     }
 }

+ 25 - 0
libs/core/src/test/java/org/elasticsearch/core/StringsTests.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.core;
+
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class StringsTests extends ESTestCase {
+
+    public void testIncorrectPattern() {
+        AssertionError assertionError = expectThrows(AssertionError.class, () -> Strings.format("%s %s", 1));
+        assertThat(
+            assertionError.getMessage(),
+            equalTo("Exception thrown when formatting [%s %s]. java.util.MissingFormatArgumentException. Format specifier '%s'")
+        );
+    }
+
+}