From 8b81a63cfc5c991fd9764fe1ea144c6272723295 Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sat, 19 Sep 2026 13:18:19 -0500 Subject: [PATCH 1/4] Allow access to mirthdb from within integration tests Signed-off-by: Mitch Gaffigan --- Dockerfile | 1 + .../alpine-temurin21-mysql.compose.yml | 9 +++++++++ .../alpine-temurin21-oracle.compose.yml | 11 ++++++++++- .../alpine-temurin21-postgres.compose.yml | 9 +++++++++ .../alpine-temurin21-sqlserver.compose.yml | 9 +++++++++ .../ubuntu-temurin21-postgres.compose.yml | 9 +++++++++ ci/run-harness.sh | 5 +++++ smoketest/build.gradle | 15 +++++++-------- .../smoketest/HarnessConfig.java | 5 +++++ 9 files changed, 64 insertions(+), 9 deletions(-) diff --git a/Dockerfile b/Dockerfile index cdafb89d2..90709a63b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -57,6 +57,7 @@ FROM eclipse-temurin:21.0.9_10-jre-noble AS smoketest-harness COPY --from=builder /app/server/setup/server-lib /opt/engine/server-lib COPY --from=builder /app/server/setup/extensions /opt/engine/extensions +COPY --from=builder /app/server/setup/conf /opt/engine/conf COPY --from=builder /app/smoketest/build/install/smoketest-harness /harness ENTRYPOINT ["/bin/bash", "/harness/run-harness.sh"] diff --git a/ci/configurations/alpine-temurin21-mysql.compose.yml b/ci/configurations/alpine-temurin21-mysql.compose.yml index feec33920..d8416a08c 100644 --- a/ci/configurations/alpine-temurin21-mysql.compose.yml +++ b/ci/configurations/alpine-temurin21-mysql.compose.yml @@ -38,3 +38,12 @@ services: timeout: 5s retries: 30 start_period: 40s + + # Lets the Database Connector metadata test reach this configuration's database. The URL is + # resolved by the oie service, not the harness, so it is the same one the engine uses. + harness: + environment: + OIE_DB_DRIVER: com.mysql.cj.jdbc.Driver + OIE_DB_URL: jdbc:mysql://db:3306/mirthdb + OIE_DB_USERNAME: mirthdb + OIE_DB_PASSWORD: mirthdb diff --git a/ci/configurations/alpine-temurin21-oracle.compose.yml b/ci/configurations/alpine-temurin21-oracle.compose.yml index 3766e7bc9..7ddd6eb34 100644 --- a/ci/configurations/alpine-temurin21-oracle.compose.yml +++ b/ci/configurations/alpine-temurin21-oracle.compose.yml @@ -37,4 +37,13 @@ services: interval: 3s timeout: 5s retries: 30 - start_period: 40s \ No newline at end of file + start_period: 40s + + # Lets the Database Connector metadata test reach this configuration's database. The URL is + # resolved by the oie service, not the harness, so it is the same one the engine uses. + harness: + environment: + OIE_DB_DRIVER: oracle.jdbc.driver.OracleDriver + OIE_DB_URL: jdbc:oracle:thin:@//db:1521/FREEPDB1 + OIE_DB_USERNAME: mirthdb + OIE_DB_PASSWORD: mirthdb diff --git a/ci/configurations/alpine-temurin21-postgres.compose.yml b/ci/configurations/alpine-temurin21-postgres.compose.yml index 4f728f7e4..f34561c8a 100644 --- a/ci/configurations/alpine-temurin21-postgres.compose.yml +++ b/ci/configurations/alpine-temurin21-postgres.compose.yml @@ -37,3 +37,12 @@ services: timeout: 5s retries: 30 start_period: 40s + + # Lets the Database Connector metadata test reach this configuration's database. The URL is + # resolved by the oie service, not the harness, so it is the same one the engine uses. + harness: + environment: + OIE_DB_DRIVER: org.postgresql.Driver + OIE_DB_URL: jdbc:postgresql://db:5432/mirthdb + OIE_DB_USERNAME: mirthdb + OIE_DB_PASSWORD: mirthdb diff --git a/ci/configurations/alpine-temurin21-sqlserver.compose.yml b/ci/configurations/alpine-temurin21-sqlserver.compose.yml index fbc8bd7ae..ca85e7886 100644 --- a/ci/configurations/alpine-temurin21-sqlserver.compose.yml +++ b/ci/configurations/alpine-temurin21-sqlserver.compose.yml @@ -52,3 +52,12 @@ services: timeout: 5s retries: 30 start_period: 40s + + # Lets the Database Connector metadata test reach this configuration's database. The URL is + # resolved by the oie service, not the harness, so it is the same one the engine uses. + harness: + environment: + OIE_DB_DRIVER: net.sourceforge.jtds.jdbc.Driver + OIE_DB_URL: jdbc:jtds:sqlserver://db:1433/mirthdb + OIE_DB_USERNAME: sa + OIE_DB_PASSWORD: OieSqlServerPassw0rd! diff --git a/ci/configurations/ubuntu-temurin21-postgres.compose.yml b/ci/configurations/ubuntu-temurin21-postgres.compose.yml index 4f728f7e4..f34561c8a 100644 --- a/ci/configurations/ubuntu-temurin21-postgres.compose.yml +++ b/ci/configurations/ubuntu-temurin21-postgres.compose.yml @@ -37,3 +37,12 @@ services: timeout: 5s retries: 30 start_period: 40s + + # Lets the Database Connector metadata test reach this configuration's database. The URL is + # resolved by the oie service, not the harness, so it is the same one the engine uses. + harness: + environment: + OIE_DB_DRIVER: org.postgresql.Driver + OIE_DB_URL: jdbc:postgresql://db:5432/mirthdb + OIE_DB_USERNAME: mirthdb + OIE_DB_PASSWORD: mirthdb diff --git a/ci/run-harness.sh b/ci/run-harness.sh index 71ea56c6d..a1582525b 100755 --- a/ci/run-harness.sh +++ b/ci/run-harness.sh @@ -16,9 +16,14 @@ mkdir -p "$results" # Lock engine to junit-jupiter to prevent false-pass results. status=0 java \ + "@$ENGINE_HOME/conf/default_modules.vmoptions" \ -Doie.baseUrl="$OIE_BASE_URL" \ -Doie.configuration="$OIE_CONFIGURATION" \ -Doie.password="$OIE_PASSWORD" \ + ${OIE_DB_DRIVER:+-Doie.db.driver="$OIE_DB_DRIVER"} \ + ${OIE_DB_URL:+-Doie.db.url="$OIE_DB_URL"} \ + ${OIE_DB_USERNAME:+-Doie.db.username="$OIE_DB_USERNAME"} \ + ${OIE_DB_PASSWORD:+-Doie.db.password="$OIE_DB_PASSWORD"} \ ${OIE_HARNESS_OPTS:-} \ -cp "$classpath" \ org.junit.platform.console.ConsoleLauncher execute \ diff --git a/smoketest/build.gradle b/smoketest/build.gradle index 4d121012c..c71faf3d6 100644 --- a/smoketest/build.gradle +++ b/smoketest/build.gradle @@ -7,16 +7,15 @@ apply plugin: 'distribution' apply from: 'generate-smoke-tests.gradle' -def clientCoreJar = project(':server').tasks.named('clientCoreJar') -def donkeyModelJar = project(':donkey').tasks.named('donkeyModelJar') +// The harness runs with the server's whole staged distribution +def stagedServer = files([ + project(':server').fileTree('setup/server-lib') { include '**/*.jar' }, + project(':server').fileTree('setup/extensions') { include '**/*.jar' }]) { + builtBy ':server:createSetup' +} dependencies { - // Client, Channel, DashboardStatus, MessageFilter, ObjectXMLSerializer - testCompileOnly files(clientCoreJar) - // RawMessage, Message, ConnectorMessage, MessageContent, Status, DeployedState - testCompileOnly files(donkeyModelJar) - // Provided at runtime by /opt/engine/server-lib (server-main uses it too). - testCompileOnly libs.snakeyaml + testCompileOnly stagedServer testImplementation libs.junit.platform.console.standalone } diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/HarnessConfig.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/HarnessConfig.java index 59fef4fe2..246f8b4fb 100644 --- a/smoketest/src/test/java/org/openintegrationengine/smoketest/HarnessConfig.java +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/HarnessConfig.java @@ -32,6 +32,11 @@ final class HarnessConfig { */ static final String CONFIGURATION = System.getProperty("oie.configuration", ""); + static final String DB_DRIVER = System.getProperty("oie.db.driver"); + static final String DB_URL = System.getProperty("oie.db.url"); + static final String DB_USERNAME = System.getProperty("oie.db.username", ""); + static final String DB_PASSWORD = System.getProperty("oie.db.password", ""); + /** Ceiling on waiting for a channel to start or a message to reach its asserted state. */ static final Duration TIMEOUT = Duration.ofSeconds(Long.parseLong(System.getProperty("oie.timeoutSeconds", "90"))); From 6163f7e1564d686792773f477472a4c0814d6390 Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sat, 19 Sep 2026 13:56:03 -0500 Subject: [PATCH 2/4] Add integration tests for DatabaseConnectorServletInterface Signed-off-by: Mitch Gaffigan --- .../smoketest/DatabaseMetadataTest.java | 111 ++++++++++++++++++ .../smoketest/OieServer.java | 8 ++ 2 files changed, 119 insertions(+) create mode 100644 smoketest/src/test/java/org/openintegrationengine/smoketest/DatabaseMetadataTest.java diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/DatabaseMetadataTest.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/DatabaseMetadataTest.java new file mode 100644 index 000000000..03ab8943d --- /dev/null +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/DatabaseMetadataTest.java @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Open Integration Engine + +package org.openintegrationengine.smoketest; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import com.mirth.connect.connectors.jdbc.Column; +import com.mirth.connect.connectors.jdbc.DatabaseConnectorServletInterface; +import com.mirth.connect.connectors.jdbc.Table; + +/** Exercises the Database Connector's "get tables" API */ +@DisplayName("Database Connector table metadata") +class DatabaseMetadataTest { + + /** Created by every engine's schema script with the same three columns. */ + private static final String TABLE = "CONFIGURATION"; + + private static final List EXPECTED_COLUMNS = List.of("CATEGORY", "NAME", "VALUE"); + + @BeforeAll + static void requireDatabaseCoordinates() { + assumeTrue(HarnessConfig.DB_URL != null && HarnessConfig.DB_DRIVER != null, + "configuration declares no separately reachable database"); + } + + @Test + @DisplayName("returns the columns of a known table") + void returnsColumnsOfKnownTable() throws Exception { + Table table = getTable(TABLE); + + assertEquals(TABLE, table.getName().toUpperCase(), "table name"); + assertEquals(EXPECTED_COLUMNS, columnNames(table), "columns of " + table.getName()); + } + + /** + * Column metadata is only useful if it carries the type and size the connector shows in the + * dialog, which is where the old per-driver query mattered. + */ + @Test + @DisplayName("reports a type and a sensible precision for each column") + void reportsTypeAndPrecision() throws Exception { + Table table = getTable(TABLE); + + for (Column column : table.getColumns()) { + assertNotNull(column.getType(), "type of " + column.getName()); + assertEquals(false, column.getType().isBlank(), "type of " + column.getName()); + } + + Column category = table.getColumns().get(0); + assertEquals("CATEGORY", category.getName().toUpperCase()); + assertEquals(255, category.getPrecision(), "CATEGORY is declared VARCHAR(255) everywhere"); + } + + /** + * A "*" in the pattern has to be translated to the SQL wildcard "%"; untranslated it matches + * nothing. The result is only checked for containment because the pattern also reaches whatever + * system tables the engine exposes - SQL Server answers with sys.configurations too. + */ + @Test + @DisplayName("filters by wildcard table name pattern") + void filtersByWildcardPattern() throws Exception { + SortedSet tables = getTables(Set.of("CONFIGURATIO*", "configuratio*")); + + List names = new ArrayList(); + for (Table table : tables) { + names.add(table.getName().toUpperCase()); + } + assertTrue(names.contains(TABLE), "expected the wildcard to match " + TABLE + ", got " + names); + } + + private static Table getTable(String name) throws Exception { + // Engines differ on identifier folding: Postgres lower-cases unquoted names while Oracle and + // Derby upper-case them, and the pattern is matched against whatever is stored. + SortedSet
tables = getTables(Set.of(name.toUpperCase(), name.toLowerCase())); + + assertEquals(1, tables.size(), "expected exactly one " + name + " table, got " + tables); + return tables.first(); + } + + private static SortedSet
getTables(Set patterns) throws Exception { + SortedSet
tables = SharedServer.get() + .servlet(DatabaseConnectorServletInterface.class) + .getTables("smoketest", "smoketest", HarnessConfig.DB_DRIVER, HarnessConfig.DB_URL, + HarnessConfig.DB_USERNAME, HarnessConfig.DB_PASSWORD, patterns, null, Collections.emptySet()); + + return tables == null ? new TreeSet
() : tables; + } + + private static List columnNames(Table table) { + List names = new ArrayList(); + for (Column column : table.getColumns()) { + names.add(column.getName().toUpperCase()); + } + return names; + } +} diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java index 51ded95d8..9df99d794 100644 --- a/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/OieServer.java @@ -80,6 +80,14 @@ private static synchronized void initSerializer(String serverVersion) throws Exc } } + /** + * Returns a typed client for one of the server's servlet interfaces, for the APIs the + * administrator uses that have no {@link Client} convenience method. + */ + T servlet(Class servletInterface) { + return client.getServlet(servletInterface); + } + /** * Deploys an exported channel and waits for it to reach {@link DeployedState#STARTED}. * From 202aaf3c911e51c9085a48302f961c0d5fc3093b Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sat, 19 Sep 2026 14:18:30 -0500 Subject: [PATCH 3/4] Fix SQLi in DatabaseConnectorServlet (CVE-2026-82583) Prior code was attempting to retrieve 0-1 rows in a per-driver manner, which is hard. Switched to using the standard `WHERE 1 = 0` approach. Left the now-pointless parameter in place. Added tests. Signed-off-by: Mitch Gaffigan --- .../jdbc/DatabaseConnectorServlet.java | 75 ++++++++----------- 1 file changed, 31 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java index 2493be8d7..597e08c48 100644 --- a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java +++ b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java @@ -22,7 +22,6 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import java.util.regex.Matcher; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; @@ -52,6 +51,9 @@ public DatabaseConnectorServlet(@Context HttpServletRequest request, @Context Se @Override public SortedSet
getTables(String channelId, String channelName, String driver, String url, String username, String password, Set tableNamePatterns, String selectLimit, Set resourceIds) { + // selectLimit is deprecated and ignored for security reasons. Kept for backcompat. + selectLimit = null; + CustomDriver customDriver = null; Connection connection = null; try { @@ -150,55 +152,40 @@ public SortedSet
getTables(String channelId, String channelName, String d // then we'll define to the generic method of getting column information, but // this could be extremely slow List columnList = new ArrayList(); - if (StringUtils.isEmpty(selectLimit)) { - logger.debug("No select limit is defined, using generic method"); - rs = dbMetaData.getColumns(null, null, tableName, null); - - // retrieve all relevant column information - for (int i = 0; rs.next(); i++) { - Column column = new Column(rs.getString("COLUMN_NAME"), rs.getString("TYPE_NAME"), rs.getInt("COLUMN_SIZE")); + final String schemaTableName = StringUtils.isNotEmpty(schema) ? "\"" + schema + "\".\"" + tableName + "\"" : "\"" + tableName + "\""; + final String queryString = "SELECT * FROM " + schemaTableName + " WHERE 1 = 0"; + Statement statement = connection.createStatement(); + try { + rs = statement.executeQuery(queryString); + ResultSetMetaData rsmd = rs.getMetaData(); + + // retrieve all relevant column information + for (int i = 1; i < rsmd.getColumnCount() + 1; i++) { + Column column = new Column(rsmd.getColumnName(i), rsmd.getColumnTypeName(i), rsmd.getPrecision(i)); columnList.add(column); } - } else { - logger.debug("Select limit is defined, using specific select query : '" + selectLimit + "'"); - - // replace the '?' with the appropriate schema.table name, and use ResultSetMetaData to - // retrieve column information - final String schemaTableName = StringUtils.isNotEmpty(schema) ? "\"" + schema + "\".\"" + tableName + "\"" : "\"" + tableName + "\""; - final String queryString = selectLimit.trim().replaceAll("\\?", Matcher.quoteReplacement(schemaTableName)); - Statement statement = connection.createStatement(); - try { - rs = statement.executeQuery(queryString); - ResultSetMetaData rsmd = rs.getMetaData(); - - // retrieve all relevant column information - for (int i = 1; i < rsmd.getColumnCount() + 1; i++) { - Column column = new Column(rsmd.getColumnName(i), rsmd.getColumnTypeName(i), rsmd.getPrecision(i)); - columnList.add(column); - } - } catch (SQLException sqle) { - logger.info("Failed to execute '" + queryString + "', fall back to generic approach to retrieve column information"); - fallback = true; - } finally { - if (statement != null) { - statement.close(); - } + } catch (SQLException sqle) { + logger.info("Failed to execute '" + queryString + "', fall back to generic approach to retrieve column information"); + fallback = true; + } finally { + if (statement != null) { + statement.close(); } + } - // failed to use selectLimit method, so we need to fall back to generic - // if this generic approach fails, then there's nothing we can do - if (fallback) { - // Re-initialize in case some columns were added before failing - columnList = new ArrayList(); + // failed to use selectLimit method, so we need to fall back to generic + // if this generic approach fails, then there's nothing we can do + if (fallback) { + // Re-initialize in case some columns were added before failing + columnList = new ArrayList(); - logger.debug("Using fallback method for retrieving columns"); - backupRs = dbMetaData.getColumns(null, null, tableName.replace("/", "//"), null); + logger.debug("Using fallback method for retrieving columns"); + backupRs = dbMetaData.getColumns(null, null, tableName.replace("/", "//"), null); - // retrieve all relevant column information - while (backupRs.next()) { - Column column = new Column(backupRs.getString("COLUMN_NAME"), backupRs.getString("TYPE_NAME"), backupRs.getInt("COLUMN_SIZE")); - columnList.add(column); - } + // retrieve all relevant column information + while (backupRs.next()) { + Column column = new Column(backupRs.getString("COLUMN_NAME"), backupRs.getString("TYPE_NAME"), backupRs.getInt("COLUMN_SIZE")); + columnList.add(column); } } From d6d86eb376436d28bc2cb3f4a53e61f37b185abb Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sat, 19 Sep 2026 15:25:30 -0500 Subject: [PATCH 4/4] Extract TableMetadataReader for testing Signed-off-by: Mitch Gaffigan --- .../jdbc/DatabaseConnectorServlet.java | 155 +----------- .../connectors/jdbc/TableMetadataReader.java | 219 ++++++++++++++++ .../jdbc/TableMetadataReaderTest.java | 238 ++++++++++++++++++ 3 files changed, 458 insertions(+), 154 deletions(-) create mode 100644 server/src/main/java/com/mirth/connect/connectors/jdbc/TableMetadataReader.java create mode 100644 server/src/test/java/com/mirth/connect/connectors/jdbc/TableMetadataReaderTest.java diff --git a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java index 597e08c48..b2eb3d218 100644 --- a/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java +++ b/server/src/main/java/com/mirth/connect/connectors/jdbc/DatabaseConnectorServlet.java @@ -10,24 +10,15 @@ package com.mirth.connect.connectors.jdbc; import java.sql.Connection; -import java.sql.DatabaseMetaData; import java.sql.DriverManager; -import java.sql.ResultSet; -import java.sql.ResultSetMetaData; import java.sql.SQLException; -import java.sql.Statement; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.SortedSet; -import java.util.TreeSet; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; import javax.ws.rs.core.SecurityContext; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -40,7 +31,6 @@ public class DatabaseConnectorServlet extends MirthServlet implements DatabaseConnectorServletInterface { - private static final String[] TABLE_TYPES = { "TABLE", "VIEW" }; private static final Logger logger = LogManager.getLogger(DatabaseConnectorServlet.class); private static final TemplateValueReplacer replacer = new TemplateValueReplacer(); private static final ContextFactoryController contextFactoryController = ControllerFactory.getFactory().createContextFactoryController(); @@ -61,8 +51,6 @@ public SortedSet
getTables(String channelId, String channelName, String d username = replacer.replaceValues(username, channelId, channelName); password = replacer.replaceValues(password, channelId, channelName); - String schema = null; - try { MirthContextFactory contextFactory = contextFactoryController.getContextFactory(resourceIds); @@ -95,115 +83,8 @@ public SortedSet
getTables(String channelId, String channelName, String d } DriverManager.setLoginTimeout(oldLoginTimeout); - DatabaseMetaData dbMetaData = connection.getMetaData(); - - // the sorted set to hold the table information - SortedSet
tableInfoList = new TreeSet
(); - - // Use a schema if the user name matches one of the schemas. - // Fix for Oracle: MIRTH-1045 - ResultSet schemasResult = null; - try { - schemasResult = dbMetaData.getSchemas(); - while (schemasResult.next()) { - String schemaResult = schemasResult.getString(1); - if (username.equalsIgnoreCase(schemaResult)) { - schema = schemaResult; - } - } - } finally { - if (schemasResult != null) { - schemasResult.close(); - } - } - - // based on the table name pattern, attempt to retrieve the table information - tableNamePatterns = translateTableNamePatterns(tableNamePatterns); - List tableNameList = new ArrayList(); - - // go through each possible table name patterns and query for the tables - for (String tableNamePattern : tableNamePatterns) { - ResultSet rs = null; - try { - rs = dbMetaData.getTables(null, schema, tableNamePattern, TABLE_TYPES); - - // based on the result set, loop through to store the table name so it can be used to - // retrieve the table's column information - while (rs.next()) { - tableNameList.add(rs.getString("TABLE_NAME")); - } - } finally { - if (rs != null) { - rs.close(); - } - } - } - - // for each table, grab their column information - for (String tableName : tableNameList) { - ResultSet rs = null; - ResultSet backupRs = null; - boolean fallback = false; - try { - // apparently it's much more efficient to use ResultSetMetaData to retrieve - // column information. So each driver is defined with their own unique SELECT - // statement to query the table columns and use ResultSetMetaData to retrieve - // the column information. If driver is not defined with the select statement - // then we'll define to the generic method of getting column information, but - // this could be extremely slow - List columnList = new ArrayList(); - final String schemaTableName = StringUtils.isNotEmpty(schema) ? "\"" + schema + "\".\"" + tableName + "\"" : "\"" + tableName + "\""; - final String queryString = "SELECT * FROM " + schemaTableName + " WHERE 1 = 0"; - Statement statement = connection.createStatement(); - try { - rs = statement.executeQuery(queryString); - ResultSetMetaData rsmd = rs.getMetaData(); - - // retrieve all relevant column information - for (int i = 1; i < rsmd.getColumnCount() + 1; i++) { - Column column = new Column(rsmd.getColumnName(i), rsmd.getColumnTypeName(i), rsmd.getPrecision(i)); - columnList.add(column); - } - } catch (SQLException sqle) { - logger.info("Failed to execute '" + queryString + "', fall back to generic approach to retrieve column information"); - fallback = true; - } finally { - if (statement != null) { - statement.close(); - } - } - - // failed to use selectLimit method, so we need to fall back to generic - // if this generic approach fails, then there's nothing we can do - if (fallback) { - // Re-initialize in case some columns were added before failing - columnList = new ArrayList(); - logger.debug("Using fallback method for retrieving columns"); - backupRs = dbMetaData.getColumns(null, null, tableName.replace("/", "//"), null); - - // retrieve all relevant column information - while (backupRs.next()) { - Column column = new Column(backupRs.getString("COLUMN_NAME"), backupRs.getString("TYPE_NAME"), backupRs.getInt("COLUMN_SIZE")); - columnList.add(column); - } - } - - // create table object and add to the list of table definitions - Table table = new Table(tableName, columnList); - tableInfoList.add(table); - } finally { - if (rs != null) { - rs.close(); - } - - if (backupRs != null) { - backupRs.close(); - } - } - } - - return tableInfoList; + return TableMetadataReader.getTables(connection, tableNamePatterns, username); } catch (Exception e) { throw new MirthApiException(new Exception("Could not retrieve database tables and columns.", e)); } finally { @@ -216,38 +97,4 @@ public SortedSet
getTables(String channelId, String channelName, String d } } - /** - * Translate the given pattern expression so that it can be used properly for searching tables - * in the database. Multiple table name patterns are delimited by comma (,) - *

- * This interpret and translate to the following: - *

- *

    - *
  • "*" = wild card for more than one character, will be converted to be used as '%'
    • - *
  • "_" = one character wild card
    • - *
  • "" = empty string will retrieve all tables - *
- *

- * Eg. rad*,table*test => Find all tables starts with 'rad' AND tables prefix with 'table' - * and postfix with 'test' - * - * @param tableNamePatternExpression - * pattern expression to translate, cannot be NULL. - * @return If table name pattern is an empty string, it'll never return NULL. - */ - private Set translateTableNamePatterns(Set tableNamePatterns) { - if (tableNamePatterns == null) { - throw new IllegalArgumentException("Parameter 'tableNamePatterns' cannot be NULL'"); - } - - Set patterns = new HashSet(); - if (tableNamePatterns.isEmpty()) { - patterns.add("%"); - } else { - for (String pattern : tableNamePatterns) { - patterns.add(pattern.trim().replaceAll("\\*", "%")); - } - } - return patterns; - } } \ No newline at end of file diff --git a/server/src/main/java/com/mirth/connect/connectors/jdbc/TableMetadataReader.java b/server/src/main/java/com/mirth/connect/connectors/jdbc/TableMetadataReader.java new file mode 100644 index 000000000..2e5a8caee --- /dev/null +++ b/server/src/main/java/com/mirth/connect/connectors/jdbc/TableMetadataReader.java @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Mirth Corporation +// SPDX-FileCopyrightText: Mitch Gaffigan + +package com.mirth.connect.connectors.jdbc; + +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; + +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * Reads table and column metadata from an open JDBC connection, for the Database Reader/Writer + * "select tables" dialog. + * + *

+ * Lifted out of {@link DatabaseConnectorServlet} so that it can be exercised directly against a + * real database without a server, a connection pool or a servlet context. + *

+ */ +final class TableMetadataReader { + + private static final String[] TABLE_TYPES = { "TABLE", "VIEW" }; + private static final Logger logger = LogManager.getLogger(TableMetadataReader.class); + + private TableMetadataReader() {} + + /** + * Retrieves the tables and views matching the given name patterns, along with their columns. + * + * @param connection + * an open connection to the database to inspect + * @param tableNamePatterns + * patterns to filter table names by, in the syntax described by + * {@link #translateTableNamePatterns(Set)}; empty retrieves every table + * @param username + * the user the connection was opened as, used to select a schema when the database + * has one named after the user (MIRTH-1045, which affects Oracle); cannot be NULL + */ + static SortedSet
getTables(Connection connection, Set tableNamePatterns, String username) throws SQLException { + DatabaseMetaData dbMetaData = connection.getMetaData(); + String schema = getSchema(dbMetaData, username); + + // the sorted set to hold the table information + SortedSet
tableInfoList = new TreeSet
(); + + // for each table, grab their column information + for (String tableName : getTableNames(dbMetaData, schema, tableNamePatterns)) { + tableInfoList.add(new Table(tableName, getColumns(connection, dbMetaData, schema, tableName))); + } + + return tableInfoList; + } + + /** + * Use a schema if the user name matches one of the schemas. Fix for Oracle: MIRTH-1045 + */ + private static String getSchema(DatabaseMetaData dbMetaData, String username) throws SQLException { + String schema = null; + + try (ResultSet schemasResult = dbMetaData.getSchemas()) { + while (schemasResult.next()) { + String schemaResult = schemasResult.getString(1); + if (username.equalsIgnoreCase(schemaResult)) { + schema = schemaResult; + } + } + } + + return schema; + } + + /** + * Based on the table name pattern, attempt to retrieve the table information. + */ + private static List getTableNames(DatabaseMetaData dbMetaData, String schema, Set tableNamePatterns) throws SQLException { + List tableNameList = new ArrayList(); + + // go through each possible table name patterns and query for the tables + for (String tableNamePattern : translateTableNamePatterns(tableNamePatterns)) { + try (ResultSet rs = dbMetaData.getTables(null, schema, tableNamePattern, TABLE_TYPES)) { + // based on the result set, loop through to store the table name so it can be used + // to retrieve the table's column information + while (rs.next()) { + tableNameList.add(rs.getString("TABLE_NAME")); + } + } + } + + return tableNameList; + } + + /** + * Retrieves the column information for a single table. + * + *

+ * Apparently it's much more efficient to use ResultSetMetaData to retrieve column information, + * so a select against the table is used to describe its columns. If that select fails we fall + * back to the generic method of getting column information, but this could be extremely slow. + *

+ */ + private static List getColumns(Connection connection, DatabaseMetaData dbMetaData, String schema, String tableName) throws SQLException { + final String queryString = buildColumnQuery(dbMetaData.getIdentifierQuoteString(), schema, tableName); + + if (queryString != null) { + try (Statement statement = connection.createStatement(); ResultSet rs = statement.executeQuery(queryString)) { + List columnList = new ArrayList(); + ResultSetMetaData rsmd = rs.getMetaData(); + + // retrieve all relevant column information + for (int i = 1; i < rsmd.getColumnCount() + 1; i++) { + columnList.add(new Column(rsmd.getColumnName(i), rsmd.getColumnTypeName(i), rsmd.getPrecision(i))); + } + + return columnList; + } catch (SQLException sqle) { + logger.info("Failed to execute '" + queryString + "', fall back to generic approach to retrieve column information"); + } + } + + // failed to use the select method, so we need to fall back to generic + // if this generic approach fails, then there's nothing we can do + logger.debug("Using fallback method for retrieving columns"); + return getGenericColumns(dbMetaData, tableName); + } + + /** + * Builds the statement used to describe a table's columns: the table name quoted the way the + * connected database expects, qualified with the schema when one was selected, and a WHERE + * clause that no row can satisfy so that only the shape of the result set comes back. + * + * @param identifierQuote + * the quote string the driver reports, from + * {@link DatabaseMetaData#getIdentifierQuoteString()}; not every database uses the + * SQL standard double quote, and MySQL uses a backtick + * @return the statement, or NULL if the database does not support quoted identifiers, in which + * case there is no safe way to name the table and the caller has to fall back to the + * generic column metadata + */ + static String buildColumnQuery(String identifierQuote, String schema, String tableName) { + // The JDBC contract returns a single space when the database does not support quoting. + final String quote = StringUtils.trimToNull(identifierQuote); + if (quote == null) { + return null; + } + + final String quotedTableName = quote(quote, tableName); + final String schemaTableName = StringUtils.isNotEmpty(schema) ? quote(quote, schema) + "." + quotedTableName : quotedTableName; + return "SELECT * FROM " + schemaTableName + " WHERE 1 = 0"; + } + + /** + * Quotes an identifier, doubling any embedded quote character so that it is taken literally + * rather than closing the quoted identifier early. + */ + private static String quote(String quote, String identifier) { + return quote + StringUtils.replace(identifier, quote, quote + quote) + quote; + } + + private static List getGenericColumns(DatabaseMetaData dbMetaData, String tableName) throws SQLException { + List columnList = new ArrayList(); + + try (ResultSet rs = dbMetaData.getColumns(null, null, tableName.replace("/", "//"), null)) { + // retrieve all relevant column information + while (rs.next()) { + columnList.add(new Column(rs.getString("COLUMN_NAME"), rs.getString("TYPE_NAME"), rs.getInt("COLUMN_SIZE"))); + } + } + + return columnList; + } + + /** + * Translate the given pattern expression so that it can be used properly for searching tables + * in the database. Multiple table name patterns are delimited by comma (,) + *

+ * This interpret and translate to the following: + *

+ *

    + *
  • "*" = wild card for more than one character, will be converted to be used as '%'
    • + *
  • "_" = one character wild card
    • + *
  • "" = empty string will retrieve all tables + *
+ *

+ * Eg. rad*,table*test => Find all tables starts with 'rad' AND tables prefix with 'table' + * and postfix with 'test' + * + * @param tableNamePatterns + * pattern expressions to translate, cannot be NULL. + * @return If table name pattern is an empty string, it'll never return NULL. + */ + private static Set translateTableNamePatterns(Set tableNamePatterns) { + if (tableNamePatterns == null) { + throw new IllegalArgumentException("Parameter 'tableNamePatterns' cannot be NULL'"); + } + + Set patterns = new HashSet(); + if (tableNamePatterns.isEmpty()) { + patterns.add("%"); + } else { + for (String pattern : tableNamePatterns) { + patterns.add(pattern.trim().replaceAll("\\*", "%")); + } + } + return patterns; + } +} diff --git a/server/src/test/java/com/mirth/connect/connectors/jdbc/TableMetadataReaderTest.java b/server/src/test/java/com/mirth/connect/connectors/jdbc/TableMetadataReaderTest.java new file mode 100644 index 000000000..04550043c --- /dev/null +++ b/server/src/test/java/com/mirth/connect/connectors/jdbc/TableMetadataReaderTest.java @@ -0,0 +1,238 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: Mitch Gaffigan + +package com.mirth.connect.connectors.jdbc; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.SortedSet; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Exercises {@link TableMetadataReader} against a real embedded Derby database rather than mocks, so + * that the statements it builds are actually parsed and executed by a database engine. Derby is + * already a server dependency and is used by other tests here. + */ +public class TableMetadataReaderTest { + + private static final String URL = "jdbc:derby:memory:tablemetadata"; + + /** Derby folds unquoted identifiers to upper case, and the default schema is APP. */ + private static final String SCHEMA = "APP"; + + /** What Derby, and the SQL standard, quote identifiers with. */ + private static final String DOUBLE_QUOTE = "\""; + + /** What MySQL quotes identifiers with unless ANSI_QUOTES is set. */ + private static final String BACKTICK = "`"; + + /** No schema is selected unless it matches the user the connection was opened as. */ + private static final String NO_USER = ""; + + private static Connection connection; + + @BeforeClass + public static void setUpClass() throws Exception { + Class.forName("org.apache.derby.jdbc.EmbeddedDriver"); + connection = DriverManager.getConnection(URL + ";create=true"); + + execute("CREATE TABLE PLAIN_TABLE (ID INTEGER NOT NULL, NAME VARCHAR(20), AMOUNT DECIMAL(9,2))"); + execute("CREATE TABLE PLAIN_OTHER (ID INTEGER)"); + // Needs delimiting: unquoted, the space is a syntax error and the case would be folded away. + execute("CREATE TABLE \"My Table\" (ID INTEGER, \"Mixed Case\" VARCHAR(5))"); + // The identifier delimiter itself, which has to be doubled inside the quotes. + execute("CREATE TABLE \"Quote\"\"Table\" (ID INTEGER)"); + } + + @AfterClass + public static void tearDownClass() throws Exception { + if (connection != null) { + connection.close(); + } + try { + DriverManager.getConnection(URL + ";drop=true"); + } catch (SQLException e) { + // Derby always reports dropping an in-memory database as an exception (SQLState 08006). + } + } + + private static void execute(String sql) throws SQLException { + try (Statement statement = connection.createStatement()) { + statement.execute(sql); + } + } + + private static SortedSet

getTables(Connection connection, String pattern, String username) throws SQLException { + return TableMetadataReader.getTables(connection, new HashSet(Collections.singletonList(pattern)), username); + } + + private static Table single(SortedSet
tables) { + assertEquals("expected exactly one table, got " + tables, 1, tables.size()); + return tables.first(); + } + + private static List columnNames(Table table) { + List names = new ArrayList(); + for (Column column : table.getColumns()) { + names.add(column.getName()); + } + return names; + } + + @Test + public void buildsTheColumnQuery() { + assertEquals("SELECT * FROM \"PLAIN_TABLE\" WHERE 1 = 0", TableMetadataReader.buildColumnQuery(DOUBLE_QUOTE, null, "PLAIN_TABLE")); + assertEquals("SELECT * FROM \"APP\".\"PLAIN_TABLE\" WHERE 1 = 0", TableMetadataReader.buildColumnQuery(DOUBLE_QUOTE, SCHEMA, "PLAIN_TABLE")); + assertEquals("SELECT * FROM \"My Table\" WHERE 1 = 0", TableMetadataReader.buildColumnQuery(DOUBLE_QUOTE, "", "My Table")); + } + + /** MySQL quotes with a backtick; a double quoted name there is a string literal, not a table. */ + @Test + public void quotesWithWhateverTheDriverReports() { + assertEquals("SELECT * FROM `PLAIN_TABLE` WHERE 1 = 0", TableMetadataReader.buildColumnQuery(BACKTICK, null, "PLAIN_TABLE")); + assertEquals("SELECT * FROM `mydb`.`PLAIN_TABLE` WHERE 1 = 0", TableMetadataReader.buildColumnQuery(BACKTICK, "mydb", "PLAIN_TABLE")); + assertEquals("SELECT * FROM `Quote``Table` WHERE 1 = 0", TableMetadataReader.buildColumnQuery(BACKTICK, null, "Quote`Table")); + } + + /** + * The JDBC contract returns a single space when the database does not support quoting, and + * there is then no safe way to name the table. + */ + @Test + public void buildsNoQueryWhenTheDatabaseCannotQuoteIdentifiers() { + assertNull(TableMetadataReader.buildColumnQuery(" ", null, "PLAIN_TABLE")); + assertNull(TableMetadataReader.buildColumnQuery("", null, "PLAIN_TABLE")); + assertNull(TableMetadataReader.buildColumnQuery(null, null, "PLAIN_TABLE")); + } + + @Test + public void readsColumnNamesTypesAndPrecisions() throws Exception { + Table table = single(getTables(connection, "PLAIN_TABLE", NO_USER)); + + assertEquals("PLAIN_TABLE", table.getName()); + assertEquals(3, table.getColumns().size()); + + Column id = table.getColumns().get(0); + assertEquals("ID", id.getName()); + assertEquals("INTEGER", id.getType()); + assertEquals(10, id.getPrecision()); + + Column name = table.getColumns().get(1); + assertEquals("NAME", name.getName()); + assertEquals("VARCHAR", name.getType()); + assertEquals(20, name.getPrecision()); + + Column amount = table.getColumns().get(2); + assertEquals("AMOUNT", amount.getName()); + assertEquals("DECIMAL", amount.getType()); + assertEquals(9, amount.getPrecision()); + } + + /** + * The generated statement has to quote the table name: unquoted, "My Table" would not parse. + */ + @Test + public void readsTableNamesThatRequireDelimiting() throws Exception { + Table table = single(getTables(connection, "My Table", NO_USER)); + + assertEquals("My Table", table.getName()); + assertEquals("[ID, Mixed Case]", columnNames(table).toString()); + } + + /** The schema is used when it matches the connecting user, which qualifies the statement. */ + @Test + public void qualifiesWithTheSchemaMatchingTheUsername() throws Exception { + Table table = single(getTables(connection, "PLAIN_TABLE", SCHEMA)); + + assertEquals("PLAIN_TABLE", table.getName()); + assertEquals("[ID, NAME, AMOUNT]", columnNames(table).toString()); + } + + @Test + public void translatesStarWildcardsToPercent() throws Exception { + SortedSet
tables = getTables(connection, "PLAIN_*", NO_USER); + + List names = new ArrayList(); + for (Table table : tables) { + names.add(table.getName()); + } + assertEquals("[PLAIN_OTHER, PLAIN_TABLE]", names.toString()); + } + + @Test + public void emptyPatternSetRetrievesEveryTable() throws Exception { + SortedSet
tables = TableMetadataReader.getTables(connection, new HashSet(), NO_USER); + + Set names = new HashSet(); + for (Table table : tables) { + names.add(table.getName()); + } + assertTrue("expected the user tables to be present, got " + names, names.containsAll(new HashSet(Arrays.asList("PLAIN_TABLE", "PLAIN_OTHER", "My Table", "Quote\"Table")))); + } + + /** + * Whatever the statement cannot describe still has to come back, via the generic (slower) + * DatabaseMetaData path. + */ + @Test + public void fallsBackToDatabaseMetaDataWhenTheStatementFails() throws Exception { + Table table = single(getTables(connectionFailingToCreateStatement(), "PLAIN_TABLE", NO_USER)); + + assertEquals("PLAIN_TABLE", table.getName()); + assertEquals("[ID, NAME, AMOUNT]", columnNames(table).toString()); + } + + /** + * A table name containing the identifier delimiter must have it doubled rather than closing the + * quoted identifier early, which would leave a statement that does not parse. + */ + @Test + public void escapesTheIdentifierQuoteCharacter() throws Exception { + assertEquals("SELECT * FROM \"Quote\"\"Table\" WHERE 1 = 0", TableMetadataReader.buildColumnQuery(DOUBLE_QUOTE, null, "Quote\"Table")); + + Table table = single(getTables(connection, "Quote\"Table", NO_USER)); + + assertEquals("Quote\"Table", table.getName()); + assertEquals("[ID]", columnNames(table).toString()); + } + + /** Wraps the live connection so that creating any statement fails. */ + private static Connection connectionFailingToCreateStatement() { + return (Connection) Proxy.newProxyInstance(TableMetadataReaderTest.class.getClassLoader(), new Class[] { + Connection.class }, new InvocationHandler() { + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if ("createStatement".equals(method.getName())) { + throw new SQLException("simulated driver failure"); + } + + assertNotNull(connection); + try { + return method.invoke(connection, args); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + } + }); + } +}