github-actions[bot] commented on code in PR #68811:
URL: https://github.com/apache/doris/pull/68811#discussion_r4236479385
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -368,7 +376,30 @@ public final synchronized void makeSureInitialized() {
* {@code alter catalog ... set properties} triggers {@link
#resetToUninitialized(boolean)}.
*/
protected void recordDeferredInitError(Throwable t) {
- this.errorMsg = ExceptionUtils.getRootCauseMessage(t);
+ this.errorMsg = initErrorMessage(t);
+ }
+
+ static String initErrorMessage(Throwable error) {
+ DiagnosticException diagnostic = findDiagnosticException(error);
+ return diagnostic == null ? ExceptionUtils.getRootCauseMessage(error)
: diagnostic.getDiagnosticMessage();
Review Comment:
[P1] Use the sanitized diagnostic for database-selection errors too. With
`lower_case_database_names=2`, a MySQL `COM_INIT_DB` or default-database
request loads JDBC database names through `ExternalCatalog.getDbNullable()`
before the later existence-check catch. On a driver failure,
`PluginDrivenExternalCatalog.listDatabaseNames()` records this safe catalog
message but rethrows the exception with its raw `SQLException` cause;
`ConnectContextUtil.initCatalogAndDb()` then uses `Util.getRootCauseMessage()`
as the client error. If the driver echoes the configured catalog password, a
user with database `SHOW` privilege receives it. Route this response through
the diagnostic-aware message extraction before sending it to the client.
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/client/JdbcDiagnosticException.java:
##########
@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.connector.jdbc.client;
+
+import org.apache.doris.connector.spi.DiagnosticException;
+import org.apache.doris.connector.spi.DorisConnectorException;
+import org.apache.doris.jni.toolkit.jdbc.JdbcExceptionUtils;
+
+/** Connector-owned, sanitized JDBC diagnostics with the original driver cause
retained. */
+public final class JdbcDiagnosticException extends DorisConnectorException
implements DiagnosticException {
+ private final String[] sensitiveValues;
+
+ public JdbcDiagnosticException(String context, Throwable cause, String...
sensitiveValues) {
+ super(JdbcExceptionUtils.format(context, cause, sensitiveValues),
cause);
Review Comment:
[P2] Sanitize FE logging of this retained cause. `SHOW DATABASES FROM
<jdbc_catalog>` loads names lazily, so a JDBC failure is rethrown through
`PluginDrivenExternalCatalog.listDatabaseNames()` and logged as a Throwable by
`StmtExecutor` (lines 957-963). That prints the original `SQLException` cause,
including any configured password or JDBC URL echoed by the driver, although
the catalog status and this exception's message are sanitized.
`ExternalCatalog.getDbNamesOrEmpty()` and other metadata catch sites also log
the raw Throwable; make those log sinks use the diagnostic stack formatter for
the full cause chain.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClientException.java:
##########
@@ -50,17 +72,6 @@ private static Object[] escapePercentInArgs(Object... args) {
}
public static String getAllExceptionMessages(Throwable throwable) {
- StringBuilder sb = new StringBuilder();
- while (throwable != null) {
- String message = throwable.getMessage();
- if (message != null && !message.isEmpty()) {
- if (sb.length() > 0) {
- sb.append(" | Caused by: ");
- }
- sb.append(message);
- }
- throwable = throwable.getCause();
- }
- return sb.toString();
+ return JdbcExceptionUtils.format("", throwable);
Review Comment:
[P2] Preserve the configured redaction values when aggregating JDBC
failures. OceanBase streaming validation calls this helper on a
`JdbcClientException` from `getConnection()`: the outer message is sanitized
with its password and URL, but this call reformats the retained `SQLException`
without either value. A driver message such as `raw=<source password>` is
copied into the resulting `JobException` and logged by `StreamingInsertJob`.
Reuse the diagnostic exception's safe message or carry its redaction policy
through this aggregation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
