OIiveirra commented on code in PR #68811: URL: https://github.com/apache/doris/pull/68811#discussion_r4278567490
########## fe/be-java-extensions/plugin-toolkit/src/main/java/org/apache/doris/jni/toolkit/jdbc/JdbcExceptionUtils.java: ########## @@ -0,0 +1,222 @@ +// 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.jni.toolkit.jdbc; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.sql.SQLException; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.Deque; +import java.util.IdentityHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** JDBC diagnostics shared by FE metadata clients and BE readers/writers. */ +public final class JdbcExceptionUtils { + private static final Pattern REMOTE_DIAGNOSTIC = Pattern.compile( + "(\\[remote_sqlstate=(?:[A-Za-z0-9]{5}|unknown), remote_vendor_error_code=-?[0-9]+, message=)" + + "((?:\\[redacted JDBC URL\\]|\\\\.|[^\\\\\\]])*+)(\\])", Pattern.DOTALL); + private static final Pattern JDBC_URL = Pattern.compile("(?i)jdbc:[^\\s\"'<>]+"); + private static final Pattern CREDENTIAL = Pattern.compile( + "(?i)([\"']?(?:password|passwd|pwd|user|username|token|secret|access[_-]?key)[\"']?)(\\s*[=:]\\s*)" + + "(\\[redacted JDBC URL\\]|\"(?:\\\\.|[^\"\\\\])*+\"|'(?:\\\\.|[^'\\\\])*+'|[^\\s;&,]+)"); + + private JdbcExceptionUtils() { + } + + /** + * Collects both JDBC chains, with identity-based cycle detection. Identical diagnostics are + * emitted once, but errors with the same message and different remote codes stay distinct. + * Non-SQL exceptions retain their messages without inventing remote error codes. + */ + public static String format(String context, Throwable error, String... sensitiveValues) { + return format(context, error, true, sensitiveValues); + } + + /** Appends only SQL diagnostics, preserving non-SQL exception messages and formatting. */ + public static String appendSqlDiagnostics(String context, Throwable error, String... sensitiveValues) { + return format(context, error, false, sensitiveValues); + } + + private static String format(String context, Throwable error, boolean includeMessages, String[] sensitiveValues) { + // RuntimeException constructors allow an absent cause; retain that existing JDBC API behavior. + if (error == null) { + return redact(context, sensitiveValues); + } + Set<Throwable> visited = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>()); + Set<String> messages = new LinkedHashSet<>(); + Set<String> sqlMessages = new LinkedHashSet<>(); + Set<String> diagnostics = new LinkedHashSet<>(); + Deque<Throwable> pending = new ArrayDeque<>(); + pending.add(error); + while (!pending.isEmpty()) { + Throwable current = pending.removeFirst(); + if (!visited.add(current)) { + continue; + } + String message = redact(current.getMessage(), sensitiveValues); + if (current instanceof SQLException) { + SQLException sqlError = (SQLException) current; + String sqlState = sqlError.getSQLState(); + sqlState = sqlState == null || sqlState.trim().isEmpty() ? "unknown" : sqlState; + diagnostics.add("[remote_sqlstate=" + sqlState + + ", remote_vendor_error_code=" + sqlError.getErrorCode() + + ", message=" + escapeDiagnosticMessage(message) + "]"); + sqlMessages.add(message); + if (sqlError.getNextException() != null) { + pending.addLast(sqlError.getNextException()); + } + } else if (includeMessages && !message.isEmpty()) { + messages.add(message); + } + if (current.getCause() != null) { + pending.addLast(current.getCause()); + } + } + messages.removeAll(sqlMessages); + String safeContext = redact(context, sensitiveValues); + diagnostics.removeIf(diagnostic -> safeContext.contains(diagnostic) + || messages.stream().anyMatch(message -> message.contains(diagnostic))); Review Comment: Fixed in 65a0491b459. The formatter now removes nested wrapper messages already carried by an outer wrapper, and deduplicates complete diagnostic frames shared by different wrapper messages. Frame identity includes SQLSTATE, vendor code and the escaped message, so distinct remote errors remain distinct. Added four-layer connect/initialize/validate/job formatting coverage and overlapping-wrapper coverage in JdbcExceptionUtilsTest. All 19 formatter tests passed, including the existing cycles, delimiter escaping and distinct-code cases. The focused validation across 12 classes passed 163 tests after correcting the new success-log test fixture; standalone FE Checkstyle passed. Raw Throwable causes and connection/execution control flow are unchanged. ########## fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/JdbcJniScanner.java: ########## @@ -387,7 +391,7 @@ private void initializeClassLoaderAndDataSource() { } hikariDataSource = ds; JdbcDataSource.getDataSource().putSource(cacheKey, hikariDataSource); - LOG.info("JdbcJniScanner: Created connection pool for " + jdbcUrl); + LOG.info("JdbcJniScanner: Created connection pool for " + JdbcExceptionUtils.redact(jdbcUrl)); Review Comment: Fixed in 65a0491b459. Scanner pool creation, writer pool creation and connection-test success now redact the configured entire JDBC URL together with the configured password before logging safeJdbcUrl. This avoids relying on regex URL parsing for SQL Server brace escaping. JdbcConnectionLogDiagnosticsTest executes all three actual logging sites using a URL whose braced password contains a quote and semicolon. It verifies the complete URL and password, including the previously exposed words} suffix, are absent. The test uses a local test driver and connection/pool substitutes, without external services or new dependencies. It and the adjacent JDBC diagnostics tests passed (8 tests); standalone Checkstyle passed. ########## fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/DiagnosticException.java: ########## @@ -0,0 +1,30 @@ +// 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.spi; + +/** + * Opt-in diagnostic policy for exceptions whose original causes must be retained but must not + * be rendered directly. The connector owns sanitization; the engine uses these outputs without + * inspecting connector properties. This interface is shared parent-first across plugin loaders. + */ +public interface DiagnosticException { Review Comment: Fixed in 65a0491b459 after merging current master in f3a383dc8b2. Master had already advanced the connector family to 14.0 for ConnectorScanRange split identity, so this PR now advances connector to 15.0 and JNI to 6.0 for the new DiagnosticException interfaces. Both pinned surface-test versions move with the POM properties; the merged baselines retain the diagnostic interfaces and master's scan-range addition. ConnectorPluginSurfaceTest, JniPluginSurfaceTest, SpiVersionTest, DirectoryPluginRuntimeManagerApiVersionTest and PluginRuntimeTest all passed (46 tests across those suites), covering recorded surfaces, filtered versions and the existing version-admission gates. Standalone FE Checkstyle passed. This rejects mismatched majors before plugin class use rather than relying on missing-class failures. -- 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]
