From 9dccbe60bee68762e98f0cd5030869930e1bdda5 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 10 Mar 2026 14:41:13 +0700 Subject: [PATCH 1/3] fix: correct SSL mode string comparison and Redis double-navigation race SSL mode enum raw values are capitalized ("Disabled", "Required", etc.) but all plugin drivers compared against lowercase strings, causing TLS to be always enabled regardless of user settings. Also fixes Redis sidebar click showing data briefly then going empty due to a race between tab selection sync and async table loading. --- Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift | 6 +++--- Plugins/MongoDBDriverPlugin/MongoDBConnection.swift | 4 ++-- Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift | 9 ++++++++- Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift | 2 +- .../PostgreSQLDriverPlugin/LibPQPluginConnection.swift | 2 +- Plugins/RedisDriverPlugin/RedisPluginConnection.swift | 4 ++-- .../Extensions/MainContentCoordinator+Navigation.swift | 8 +++++++- TablePro/Views/Main/MainContentView.swift | 5 +++++ 8 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift index 5caab6a9..99d61c9c 100644 --- a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift +++ b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift @@ -78,8 +78,8 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable { func connect() async throws { let useTLS = config.additionalFields["sslMode"] != nil - && config.additionalFields["sslMode"] != "disable" - let skipVerification = config.additionalFields["sslMode"] == "required" + && config.additionalFields["sslMode"] != "Disabled" + let skipVerification = config.additionalFields["sslMode"] == "Required" let urlConfig = URLSessionConfiguration.default urlConfig.timeoutIntervalForRequest = 30 @@ -533,7 +533,7 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable { private func buildRequest(query: String, database: String, queryId: String? = nil) throws -> URLRequest { let useTLS = config.additionalFields["sslMode"] != nil - && config.additionalFields["sslMode"] != "disable" + && config.additionalFields["sslMode"] != "Disabled" var components = URLComponents() components.scheme = useTLS ? "https" : "http" diff --git a/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift b/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift index d93c2f5f..8b6429d6 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift @@ -169,10 +169,10 @@ final class MongoDBConnection: @unchecked Sendable { "authSource=admin" ] - let sslEnabled = sslMode != "disabled" && !sslMode.isEmpty + let sslEnabled = sslMode != "Disabled" && !sslMode.isEmpty if sslEnabled { params.append("tls=true") - let verifiesCert = sslMode == "verify_ca" || sslMode == "verify_identity" + let verifiesCert = sslMode == "Verify CA" || sslMode == "Verify Identity" if !verifiesCert { params.append("tlsAllowInvalidCertificates=true") } diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index 02ea8e5c..67ef8f43 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -28,6 +28,8 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { self.currentDb = config.database } + private static let systemDatabases: Set = ["admin", "local", "config"] + // MARK: - Connection Management func connect() async throws { @@ -37,7 +39,7 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { user: config.username, password: config.password, database: currentDb, - sslMode: config.additionalFields["sslMode"] ?? "disabled", + sslMode: config.additionalFields["sslMode"] ?? "Disabled", sslCACertPath: config.additionalFields["sslCACertPath"] ?? "", sslClientCertPath: config.additionalFields["sslClientCertPath"] ?? "", readPreference: config.additionalFields["mongoReadPreference"], @@ -46,6 +48,11 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { try await conn.connect() mongoConnection = conn + + if currentDb.isEmpty { + let dbs = try await conn.listDatabases() + currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "test" + } } func disconnect() { diff --git a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift index fb88fc16..0647a232 100644 --- a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift +++ b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift @@ -68,7 +68,7 @@ struct MySQLSSLConfig { let clientKeyPath: String init(from fields: [String: String]) { - self.mode = Mode(rawValue: fields["sslMode"] ?? "disabled") ?? .disabled + self.mode = Mode(rawValue: fields["sslMode"] ?? "Disabled") ?? .disabled self.caCertificatePath = fields["sslCaCertPath"] ?? "" self.clientCertificatePath = fields["sslClientCertPath"] ?? "" self.clientKeyPath = fields["sslClientKeyPath"] ?? "" diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index 8ad0b68b..552f9b42 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -24,7 +24,7 @@ struct PQSSLConfig { init() {} init(additionalFields: [String: String]) { - self.mode = additionalFields["sslMode"] ?? "disable" + self.mode = additionalFields["sslMode"] ?? "Disabled" self.caCertificatePath = additionalFields["sslCaCertPath"] ?? "" self.clientCertificatePath = additionalFields["sslClientCertPath"] ?? "" self.clientKeyPath = additionalFields["sslClientKeyPath"] ?? "" diff --git a/Plugins/RedisDriverPlugin/RedisPluginConnection.swift b/Plugins/RedisDriverPlugin/RedisPluginConnection.swift index df42963e..399cd5b6 100644 --- a/Plugins/RedisDriverPlugin/RedisPluginConnection.swift +++ b/Plugins/RedisDriverPlugin/RedisPluginConnection.swift @@ -26,8 +26,8 @@ struct RedisSSLConfig { init() {} init(additionalFields: [String: String]) { - let sslMode = additionalFields["sslMode"] ?? "disable" - self.isEnabled = sslMode != "disable" + let sslMode = additionalFields["sslMode"] ?? "Disabled" + self.isEnabled = sslMode != "Disabled" self.caCertificatePath = additionalFields["sslCaCertPath"] ?? "" self.clientCertificatePath = additionalFields["sslClientCertPath"] ?? "" self.clientKeyPath = additionalFields["sslClientKeyPath"] ?? "" diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift index 30db4805..dcc42fc3 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift @@ -89,7 +89,13 @@ extension MainContentCoordinator { AppState.shared.isCurrentTabEditable = !isView && tableName.isEmpty == false toolbarState.isTableTab = true } - runQuery() + // Redis needs selectRedisDatabaseAndQuery to ensure the correct + // database is SELECTed and session state is updated before querying. + if connection.type == .redis, let dbIndex = Int(currentDatabase) { + selectRedisDatabaseAndQuery(dbIndex) + } else { + runQuery() + } return } diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index b8a433d9..7beb0b5c 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -725,6 +725,11 @@ struct MainContentView: View { /// Navigation safety is guaranteed by `SidebarNavigationResult.resolve` returning `.skip` /// when the selected table matches the current tab. private func syncSidebarToCurrentTab() { + // Don't clear sidebar selection when the table list hasn't loaded yet. + // Clearing it prematurely triggers SidebarSyncAction to re-select on tables + // load, which causes a double-navigation race for Redis (and potentially others). + guard !tables.isEmpty else { return } + let target: Set if let currentTableName = tabManager.selectedTab?.tableName, let match = tables.first(where: { $0.name == currentTableName }) { From c8e2ba4f699390f9b2046b7905a59b3e720b6e6e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 10 Mar 2026 14:49:53 +0700 Subject: [PATCH 2/3] fix: address code review feedback for SSL mode and Redis navigation - Update MySQLSSLConfig.Mode enum raw values to match SSLMode canonical strings ("Disabled", "Required", "Verify CA", "Verify Identity") - Fix PQSSLConfig default mode from "disable" to "Disabled" - Narrow syncSidebarToCurrentTab guard to only skip clearing (not all sync) when tables are empty, so genuinely empty databases still work - Change MongoDB empty-db fallback from hardcoded "test" to empty string - Add CHANGELOG.md entries for all three fixes - Add 13 SSL mode string consistency tests covering all plugin parsers --- CHANGELOG.md | 6 + .../MongoDBPluginDriver.swift | 2 +- .../MariaDBPluginConnection.swift | 10 +- .../LibPQPluginConnection.swift | 2 +- TablePro/Views/Main/MainContentView.swift | 9 +- .../Core/Plugins/SSLModeStringTests.swift | 162 ++++++++++++++++++ 6 files changed, 179 insertions(+), 12 deletions(-) create mode 100644 TableProTests/Core/Plugins/SSLModeStringTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e566c07..e6975574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- SSL/TLS always being enabled for MongoDB, Redis, and ClickHouse connections due to case mismatch in SSL mode string comparison (#249) +- Redis sidebar click showing data briefly then going empty due to double-navigation race condition (#251) +- MongoDB showing "Invalid database name: ''" when connecting without a database name + ### Added - Safe mode levels: per-connection setting with 6 levels (Silent, Alert, Alert Full, Safe Mode, Safe Mode Full, Read-Only) replacing the boolean read-only toggle, with confirmation dialogs and Touch ID/password authentication for stricter levels diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index 67ef8f43..180ab97b 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -51,7 +51,7 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { if currentDb.isEmpty { let dbs = try await conn.listDatabases() - currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "test" + currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "" } } diff --git a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift index 0647a232..7b150d89 100644 --- a/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift +++ b/Plugins/MySQLDriverPlugin/MariaDBPluginConnection.swift @@ -55,11 +55,11 @@ struct MariaDBPluginQueryResult { struct MySQLSSLConfig { enum Mode: String { - case disabled - case preferred - case required - case verifyCa = "verify_ca" - case verifyIdentity = "verify_identity" + case disabled = "Disabled" + case preferred = "Preferred" + case required = "Required" + case verifyCa = "Verify CA" + case verifyIdentity = "Verify Identity" } let mode: Mode diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index 552f9b42..7efc79a0 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -16,7 +16,7 @@ private let logger = Logger(subsystem: "com.TablePro.PostgreSQLDriver", category // MARK: - SSL Configuration struct PQSSLConfig { - var mode: String = "disable" + var mode: String = "Disabled" var caCertificatePath: String = "" var clientCertificatePath: String = "" var clientKeyPath: String = "" diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 7beb0b5c..eec55e02 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -725,11 +725,6 @@ struct MainContentView: View { /// Navigation safety is guaranteed by `SidebarNavigationResult.resolve` returning `.skip` /// when the selected table matches the current tab. private func syncSidebarToCurrentTab() { - // Don't clear sidebar selection when the table list hasn't loaded yet. - // Clearing it prematurely triggers SidebarSyncAction to re-select on tables - // load, which causes a double-navigation race for Redis (and potentially others). - guard !tables.isEmpty else { return } - let target: Set if let currentTableName = tabManager.selectedTab?.tableName, let match = tables.first(where: { $0.name == currentTableName }) { @@ -738,6 +733,10 @@ struct MainContentView: View { target = [] } if sidebarState.selectedTables != target { + // Don't clear sidebar selection while the table list is still loading. + // Clearing it prematurely triggers SidebarSyncAction to re-select on + // tables load, causing a double-navigation race condition. + if target.isEmpty && tables.isEmpty { return } sidebarState.selectedTables = target } } diff --git a/TableProTests/Core/Plugins/SSLModeStringTests.swift b/TableProTests/Core/Plugins/SSLModeStringTests.swift new file mode 100644 index 00000000..e7ba94ae --- /dev/null +++ b/TableProTests/Core/Plugins/SSLModeStringTests.swift @@ -0,0 +1,162 @@ +// +// SSLModeStringTests.swift +// TableProTests +// +// Tests that plugin SSL config structs correctly parse SSLMode raw values. +// Plugin types are bundle targets and cannot be imported directly, so we +// duplicate the config parsing logic here as private test helpers. +// + +import Foundation +import Testing +@testable import TablePro + +// MARK: - Test Helpers (mirror plugin SSL config structs) + +/// Mirror of MySQLSSLConfig.Mode from MariaDBPluginConnection.swift +private enum TestMySQLSSLMode: String { + case disabled = "Disabled" + case preferred = "Preferred" + case required = "Required" + case verifyCa = "Verify CA" + case verifyIdentity = "Verify Identity" +} + +/// Mirror of RedisSSLConfig init from RedisPluginConnection.swift +private struct TestRedisSSLConfig { + var isEnabled: Bool + + init(additionalFields: [String: String]) { + let sslMode = additionalFields["sslMode"] ?? "Disabled" + self.isEnabled = sslMode != "Disabled" + } +} + +/// Mirror of PQSSLConfig from LibPQPluginConnection.swift +private struct TestPQSSLConfig { + var mode: String = "Disabled" + + init() {} + + init(additionalFields: [String: String]) { + self.mode = additionalFields["sslMode"] ?? "Disabled" + } + + var libpqSslMode: String { + switch mode { + case "Disabled": return "disable" + case "Preferred": return "prefer" + case "Required": return "require" + case "Verify CA": return "verify-ca" + case "Verify Identity": return "verify-full" + default: return "disable" + } + } +} + +// MARK: - SSLMode Raw Values Match Plugin Expectations + +@Suite("SSL Mode String Consistency") +struct SSLModeStringTests { + @Test("SSLMode.disabled.rawValue matches plugin disabled check") + func disabledRawValue() { + #expect(SSLMode.disabled.rawValue == "Disabled") + } + + @Test("SSLMode.required.rawValue matches plugin required check") + func requiredRawValue() { + #expect(SSLMode.required.rawValue == "Required") + } + + @Test("SSLMode.verifyCa.rawValue matches plugin verify CA check") + func verifyCaRawValue() { + #expect(SSLMode.verifyCa.rawValue == "Verify CA") + } + + @Test("SSLMode.verifyIdentity.rawValue matches plugin verify identity check") + func verifyIdentityRawValue() { + #expect(SSLMode.verifyIdentity.rawValue == "Verify Identity") + } + + @Test("All SSLMode cases round-trip through MySQL Mode enum") + func mysqlModeRoundTrip() { + for sslMode in SSLMode.allCases { + let parsed = TestMySQLSSLMode(rawValue: sslMode.rawValue) + #expect(parsed != nil, "MySQLSSLMode failed to parse '\(sslMode.rawValue)'") + } + } + + @Test("MySQL Mode parses each SSLMode raw value to the correct case") + func mysqlModeParsesCorrectCase() { + #expect(TestMySQLSSLMode(rawValue: "Disabled") == .disabled) + #expect(TestMySQLSSLMode(rawValue: "Preferred") == .preferred) + #expect(TestMySQLSSLMode(rawValue: "Required") == .required) + #expect(TestMySQLSSLMode(rawValue: "Verify CA") == .verifyCa) + #expect(TestMySQLSSLMode(rawValue: "Verify Identity") == .verifyIdentity) + } + + @Test("Redis SSL disabled when sslMode is Disabled") + func redisSSLDisabled() { + let config = TestRedisSSLConfig(additionalFields: ["sslMode": "Disabled"]) + #expect(!config.isEnabled) + } + + @Test("Redis SSL enabled when sslMode is Required") + func redisSSLEnabled() { + let config = TestRedisSSLConfig(additionalFields: ["sslMode": "Required"]) + #expect(config.isEnabled) + } + + @Test("Redis SSL defaults to disabled when sslMode key is absent") + func redisSSLDefaultDisabled() { + let config = TestRedisSSLConfig(additionalFields: [:]) + #expect(!config.isEnabled) + } + + @Test("PostgreSQL maps all SSLMode raw values to correct libpq modes") + func pqSSLModeMapping() { + #expect(TestPQSSLConfig(additionalFields: ["sslMode": "Disabled"]).libpqSslMode == "disable") + #expect(TestPQSSLConfig(additionalFields: ["sslMode": "Preferred"]).libpqSslMode == "prefer") + #expect(TestPQSSLConfig(additionalFields: ["sslMode": "Required"]).libpqSslMode == "require") + #expect(TestPQSSLConfig(additionalFields: ["sslMode": "Verify CA"]).libpqSslMode == "verify-ca") + #expect(TestPQSSLConfig(additionalFields: ["sslMode": "Verify Identity"]).libpqSslMode == "verify-full") + } + + @Test("PostgreSQL default init uses Disabled") + func pqDefaultInit() { + let config = TestPQSSLConfig() + #expect(config.mode == "Disabled") + #expect(config.libpqSslMode == "disable") + } + + @Test("MongoDB SSL mode string comparisons use correct case") + func mongoDBSSLModeStrings() { + // These mirror the comparisons in MongoDBConnection.buildUri() + let disabled = SSLMode.disabled.rawValue + let verifyCa = SSLMode.verifyCa.rawValue + let verifyIdentity = SSLMode.verifyIdentity.rawValue + + #expect(disabled == "Disabled") + let sslEnabled = disabled != "Disabled" && !disabled.isEmpty + #expect(!sslEnabled) + + let required = SSLMode.required.rawValue + let sslEnabledRequired = required != "Disabled" && !required.isEmpty + #expect(sslEnabledRequired) + + let verifiesCert = verifyCa == "Verify CA" || verifyIdentity == "Verify Identity" + #expect(verifiesCert) + } + + @Test("ClickHouse SSL mode string comparisons use correct case") + func clickHouseSSLModeStrings() { + // These mirror the comparisons in ClickHousePlugin.connect() / buildRequest() + let disabled = SSLMode.disabled.rawValue + let useTLS = disabled != "Disabled" + #expect(!useTLS) + + let required = SSLMode.required.rawValue + let skipVerification = required == "Required" + #expect(skipVerification) + } +} From 0aa1bf650799ed9799f7f489d04f791ec360c9ce Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 10 Mar 2026 14:56:37 +0700 Subject: [PATCH 3/3] fix: address CodeRabbit review for MongoDB plugin safety --- Plugins/MongoDBDriverPlugin/MongoDBConnection.swift | 4 ++-- .../MongoDBDriverPlugin/MongoDBPluginDriver.swift | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift b/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift index 8b6429d6..9280ec4c 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBConnection.swift @@ -104,7 +104,7 @@ final class MongoDBConnection: @unchecked Sendable { user: String, password: String?, database: String, - sslMode: String = "disabled", + sslMode: String = "Disabled", sslCACertPath: String = "", sslClientCertPath: String = "", readPreference: String? = nil, @@ -169,7 +169,7 @@ final class MongoDBConnection: @unchecked Sendable { "authSource=admin" ] - let sslEnabled = sslMode != "Disabled" && !sslMode.isEmpty + let sslEnabled = ["Preferred", "Required", "Verify CA", "Verify Identity"].contains(sslMode) if sslEnabled { params.append("tls=true") let verifiesCert = sslMode == "Verify CA" || sslMode == "Verify Identity" diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index 180ab97b..9686b4cc 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -47,12 +47,18 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { ) try await conn.connect() - mongoConnection = conn if currentDb.isEmpty { - let dbs = try await conn.listDatabases() - currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "" + do { + let dbs = try await conn.listDatabases() + currentDb = dbs.first { !Self.systemDatabases.contains($0) } ?? dbs.first ?? "" + } catch { + conn.disconnect() + throw error + } } + + mongoConnection = conn } func disconnect() {