using non null format string

apache · Sep 29, 2023 · 45739b4 · 45739b4
1 parent abec4a7
commit 45739b4
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 26 deletions.
diff --git a/...ti-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java b/...ti-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
@@ -217,7 +217,7 @@ public Response doPost(final SqlQuery sqlQuery, @Context final HttpServletReques
       return buildNonOkResponse(
           DruidException.forPersona(DruidException.Persona.DEVELOPER)
                         .ofCategory(DruidException.Category.UNCATEGORIZED)
-                        .build(e.getMessage())
+                        .build("%s", e.getMessage())
       );
     }
     finally {

diff --git a/...e/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java b/...e/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java
@@ -170,7 +170,7 @@ public Response doPost(
           sqlQueryId,
           DruidException.forPersona(DruidException.Persona.DEVELOPER)
                         .ofCategory(DruidException.Category.UNCATEGORIZED)
-                        .build(e.getMessage())
+                        .build("%s", e.getMessage())
       );
     }
     finally {

diff --git a/...multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java b/...multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
@@ -234,7 +234,7 @@ public static Optional<SqlStatementResult> getExceptionPayload(
           null,
           DruidException.forPersona(DruidException.Persona.DEVELOPER)
                         .ofCategory(DruidException.Category.UNCATEGORIZED)
-                        .build(taskResponse.getStatus().getErrorMsg()).toErrorResponse()
+                        .build("%s", taskResponse.getStatus().getErrorMsg()).toErrorResponse()
       ));
     }
 

diff --git a/processing/src/main/java/org/apache/druid/error/ErrorResponse.java b/processing/src/main/java/org/apache/druid/error/ErrorResponse.java
@@ -204,7 +204,7 @@ protected DruidException makeException(DruidException.DruidExceptionBuilder bob)
             {
               return bob.forPersona(cause.getTargetPersona())
                         .ofCategory(cause.getCategory())
-                        .build(cause, cause.getMessage())
+                        .build(cause, "%s", cause.getMessage())
                         .withContext(cause.getContext());
             }
           }

diff --git a/processing/src/main/java/org/apache/druid/error/QueryExceptionCompat.java b/processing/src/main/java/org/apache/druid/error/QueryExceptionCompat.java
@@ -47,7 +47,7 @@ protected DruidException makeException(DruidException.DruidExceptionBuilder bob)
   {
     return bob.forPersona(DruidException.Persona.OPERATOR)
               .ofCategory(convertFailType(exception.getFailType()))
-              .build(exception, exception.getMessage())
+              .build(exception, "%s", exception.getMessage())
               .withContext("host", exception.getHost())
               .withContext("errorClass", exception.getErrorClass())
               .withContext("legacyErrorCode", exception.getErrorCode());

diff --git a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java
@@ -373,24 +373,16 @@ public static String nonStrictFormat(String message, Object... formatArgs)
     if (formatArgs == null || formatArgs.length == 0) {
       return message;
     }
-    if (message == null) {
-      return compactFormatArgs("", formatArgs);
-    }
     try {
       return String.format(Locale.ENGLISH, message, formatArgs);
     }
     catch (IllegalFormatException e) {
-      return compactFormatArgs(message, formatArgs);
-    }
-  }
-
-  private static String compactFormatArgs(String message, Object... formatArgs)
-  {
-    StringBuilder bob = new StringBuilder(message);
-    for (Object formatArg : formatArgs) {
-      bob.append("; ").append(formatArg);
+      StringBuilder bob = new StringBuilder(message);
+      for (Object formatArg : formatArgs) {
+        bob.append("; ").append(formatArg);
+      }
+      return bob.toString();
     }
-    return bob.toString();
   }
 
   /**

diff --git a/processing/src/test/java/org/apache/druid/error/ErrorResponseTest.java b/processing/src/test/java/org/apache/druid/error/ErrorResponseTest.java
@@ -131,7 +131,7 @@ public void testQueryExceptionCompatWithNullMessage()
             "TIMEOUT",
 
             "errorMessage",
-            null
+            "null"
         )
     );
   }

diff --git a/processing/src/test/java/org/apache/druid/java/util/common/StringUtilsTest.java b/processing/src/test/java/org/apache/druid/java/util/common/StringUtilsTest.java
@@ -386,11 +386,19 @@ public void testJavaStringCompare()
       }
     }
   }
-  @Test
+
+  @Test(expected = NullPointerException.class)
   public void testNonStrictFormatWithNullMessage()
   {
-    Assert.assertEquals("; 1; 2", StringUtils.nonStrictFormat(null, 1, 2));
-    Assert.assertEquals(null, StringUtils.nonStrictFormat(null));
-    Assert.assertEquals("", StringUtils.nonStrictFormat("", 1));
+    StringUtils.nonStrictFormat(null, 1, 2);
+  }
+
+  @Test
+  public void testNonStrictFormatWithStringContainingPercent()
+  {
+    Assert.assertEquals(
+        "some string containing % %s %d %f",
+        StringUtils.nonStrictFormat("%s", "some string containing % %s %d %f")
+    );
   }
 }
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
@@ -345,7 +345,7 @@ public static DruidException translateException(Exception e)
           return DruidException.forPersona(DruidException.Persona.USER)
                                .ofCategory(DruidException.Category.INVALID_INPUT)
                                .withErrorCode("invalidInput")
-                               .build(inner, inner.getMessage()).withContext("sourceType", "sql");
+                               .build(inner, "%s", inner.getMessage()).withContext("sourceType", "sql");
         } else {
           final String theUnexpectedToken = getUnexpectedTokenString(parseException);
 
@@ -390,14 +390,14 @@ public static DruidException translateException(Exception e)
     catch (RelOptPlanner.CannotPlanException inner) {
       return DruidException.forPersona(DruidException.Persona.USER)
                            .ofCategory(DruidException.Category.INVALID_INPUT)
-                           .build(inner, inner.getMessage());
+                           .build(inner, "%s", inner.getMessage());
     }
     catch (Exception inner) {
       // Anything else. Should not get here. Anything else should already have
       // been translated to a DruidException unless it is an unexpected exception.
       return DruidException.forPersona(DruidException.Persona.ADMIN)
                            .ofCategory(DruidException.Category.UNCATEGORIZED)
-                           .build(inner, inner.getMessage());
+                           .build(inner, "%s", inner.getMessage());
     }
   }