This is an automated email from the ASF dual-hosted git repository.
gian pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 07c28f17ca6 Fix missing format strings in calls to
DruidException.build (#15056)
07c28f17ca6 is described below
commit 07c28f17ca6020c1cc0fa954d19b9b0cc8d32c1b
Author: Pranav <[email protected]>
AuthorDate: Fri Sep 29 17:00:36 2023 -0700
Fix missing format strings in calls to DruidException.build (#15056)
* Fix the NPE bug in nonStrictFormat
* using non null format string
* using Assert.assertThrows
---
.../msq/sql/resources/SqlStatementResource.java | 2 +-
.../druid/msq/sql/resources/SqlTaskResource.java | 2 +-
.../druid/msq/util/SqlStatementResourceHelper.java | 2 +-
.../java/org/apache/druid/error/ErrorResponse.java | 2 +-
.../apache/druid/error/QueryExceptionCompat.java | 2 +-
.../org/apache/druid/error/ErrorResponseTest.java | 28 ++++++++++++++++++++++
.../druid/java/util/common/StringUtilsTest.java | 15 ++++++++++++
.../druid/sql/calcite/planner/DruidPlanner.java | 6 ++---
8 files changed, 51 insertions(+), 8 deletions(-)
diff --git
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
index 97be6dbc3bb..dd4e0840300 100644
---
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
+++
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java
@@ -217,7 +217,7 @@ public class SqlStatementResource
return buildNonOkResponse(
DruidException.forPersona(DruidException.Persona.DEVELOPER)
.ofCategory(DruidException.Category.UNCATEGORIZED)
- .build(e.getMessage())
+ .build("%s", e.getMessage())
);
}
finally {
diff --git
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java
index 187601cea0e..e9b4c61cef2 100644
---
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java
+++
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlTaskResource.java
@@ -170,7 +170,7 @@ public class SqlTaskResource
sqlQueryId,
DruidException.forPersona(DruidException.Persona.DEVELOPER)
.ofCategory(DruidException.Category.UNCATEGORIZED)
- .build(e.getMessage())
+ .build("%s", e.getMessage())
);
}
finally {
diff --git
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
index 08bc3dc54d9..86aed98f063 100644
---
a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
+++
b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/SqlStatementResourceHelper.java
@@ -234,7 +234,7 @@ public class SqlStatementResourceHelper
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
index 7b571cca271..0c6edf25c10 100644
--- 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 @@ public class ErrorResponse
{
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
index 163c7b04cd0..5782ca55ee0 100644
--- 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 @@ public class QueryExceptionCompat extends
DruidException.Failure
{
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/test/java/org/apache/druid/error/ErrorResponseTest.java
b/processing/src/test/java/org/apache/druid/error/ErrorResponseTest.java
index 2ddd39aa7da..16a1ccecec0 100644
--- a/processing/src/test/java/org/apache/druid/error/ErrorResponseTest.java
+++ b/processing/src/test/java/org/apache/druid/error/ErrorResponseTest.java
@@ -107,4 +107,32 @@ public class ErrorResponseTest
.expectMessageIs("Query did not complete within configured timeout
period. You can increase query timeout or tune the performance of query.")
);
}
+ @Test
+ public void testQueryExceptionCompatWithNullMessage()
+ {
+ ErrorResponse response = new ErrorResponse(DruidException.fromFailure(new
QueryExceptionCompat(new QueryTimeoutException(
+ null,
+ "hostname"
+ ))));
+ final Map<String, Object> asMap = response.getAsMap();
+ MatcherAssert.assertThat(
+ asMap,
+ DruidMatchers.mapMatcher(
+ "error",
+ "Query timeout",
+
+ "errorCode",
+ "legacyQueryException",
+
+ "persona",
+ "OPERATOR",
+
+ "category",
+ "TIMEOUT",
+
+ "errorMessage",
+ "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
index 3f2d5713c2f..5927207ba08 100644
---
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,4 +386,19 @@ public class StringUtilsTest
}
}
}
+
+ @Test()
+ public void testNonStrictFormatWithNullMessage()
+ {
+ Assert.assertThrows(NullPointerException.class, () ->
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
index b77a5fdd498..e47411361b2 100644
--- 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 class DruidPlanner implements Closeable
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 class DruidPlanner implements Closeable
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());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
