From 9c2fb574bd1b6500d296a12c166f334f1ed98412 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 22 Sep 2026 14:49:06 +0700 Subject: [PATCH 1/2] fix(export): give a dump the TLS options its own client tool takes --- CHANGELOG.md | 5 + .../Core/Database/CLIToolVersionProbe.swift | 74 ++++++++ .../Core/Database/MySQLClientArguments.swift | 133 +++++++++++++ .../Database/MySQLDumpToolIdentifier.swift | 66 +++++++ .../Core/Database/NativeDumpDescriptor.swift | 38 +++- .../Core/Database/NativeDumpRegistry.swift | 108 ++++++----- .../Database/NativeDumpResolvedTool.swift | 44 +++++ .../Core/Database/NativeDumpService.swift | 36 +++- .../Database/PostgreSQLDumpToolLocator.swift | 28 +-- .../Database/CLIToolVersionProbeTests.swift | 52 +++++ .../Database/MySQLClientArgumentsTests.swift | 171 +++++++++++++++++ .../MySQLDumpToolIdentifierTests.swift | 84 +++++++++ .../Database/NativeDumpRegistryTests.swift | 178 ++++++++++++++++-- .../Database/NativeDumpScopeTests.swift | 43 +++-- .../Database/NativeDumpServiceTests.swift | 48 +++-- .../Database/ServerSideExportTests.swift | 4 +- docs/features/backup-restore.mdx | 7 +- scripts/check-mysql-dump-tool-flags.sh | 176 +++++++++++++++++ 18 files changed, 1164 insertions(+), 131 deletions(-) create mode 100644 TablePro/Core/Database/CLIToolVersionProbe.swift create mode 100644 TablePro/Core/Database/MySQLClientArguments.swift create mode 100644 TablePro/Core/Database/MySQLDumpToolIdentifier.swift create mode 100644 TablePro/Core/Database/NativeDumpResolvedTool.swift create mode 100644 TableProTests/Database/CLIToolVersionProbeTests.swift create mode 100644 TableProTests/Database/MySQLClientArgumentsTests.swift create mode 100644 TableProTests/Database/MySQLDumpToolIdentifierTests.swift create mode 100755 scripts/check-mysql-dump-tool-flags.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c417c534..edb68eca45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -291,6 +291,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Explain on Redshift failing on PostgreSQL's `FORMAT JSON` and `ANALYZE` options. - Decimal points and minus signs accepted in DuckDB Port and BigQuery Max Bytes Billed. - Wrong Redis database in the toolbar of a second window opened on the same connection. +- Backup Dump failing with `unknown variable 'ssl-mode=PREFERRED'` when the `mysqldump` on `PATH` is MariaDB's. (#3046) +- Verify CA and Verify Identity connections unable to back up or restore on MySQL, MariaDB and PostgreSQL. +- Partial MySQL dump of a MariaDB or MySQL 5.7 server, from the column statistics `mysqldump` 8 reads. +- A database whose name starts with a dash backed up as a different database, reported as a success. +- Cancel ignored while TablePro was locating the backup tool, and the dump running anyway. ### Security diff --git a/TablePro/Core/Database/CLIToolVersionProbe.swift b/TablePro/Core/Database/CLIToolVersionProbe.swift new file mode 100644 index 0000000000..2faec18143 --- /dev/null +++ b/TablePro/Core/Database/CLIToolVersionProbe.swift @@ -0,0 +1,74 @@ +// +// CLIToolVersionProbe.swift +// TablePro +// + +import Foundation +import os + +/// Asks a command line tool what it is, by running it with `--version`. +/// +/// Synchronous on purpose: `NativeDumpDescriptor.CommandLineTool`'s resolution hooks are +/// synchronous closures that `NativeDumpService` already runs inside a detached task, so an async +/// probe would have to change every one of them. +enum CLIToolVersionProbe { + private static let logger = Logger(subsystem: "com.TablePro", category: "CLIToolVersionProbe") + + static let defaultTimeout: TimeInterval = 3 + + /// A version banner is one line. Reading past this is a tool doing something other than + /// answering the question, and the answer is taken from what arrived rather than waited for. + static let outputCap = 64 * 1_024 + + /// Standard output of ` --version`, or nil when the tool cannot run, does not answer in + /// time, or exits non-zero. + static func versionOutput(of path: String, timeout: TimeInterval = defaultTimeout) -> String? { + let process = Process() + process.executableURL = URL(fileURLWithPath: path) + process.arguments = ["--version"] + process.environment = CLIToolEnvironment.augmented() + let pipe = Pipe() + process.standardOutput = pipe + process.standardError = FileHandle.nullDevice + let finished = DispatchSemaphore(value: 0) + process.terminationHandler = { _ in finished.signal() } + do { + try process.run() + } catch { + return nil + } + + if finished.wait(timeout: .now() + timeout) == .timedOut { + process.terminate() + logger.warning( + "\(path, privacy: .private(mask: .hash)) did not answer --version within \(timeout, privacy: .public)s" + ) + return nil + } + guard process.terminationStatus == 0 else { return nil } + return String(data: readAvailable(from: pipe.fileHandleForReading), encoding: .utf8) + } + + /// What the pipe holds now, rather than what it holds at EOF. + /// + /// The tool has exited, so its own output is already here. `readDataToEndOfFile` would wait for + /// every writer to close instead, and a wrapper that prints its version, starts a helper that + /// inherits standard output and exits leaves that EOF to the helper: the deadline above covers + /// only the process, so the read has none and the dump never starts. + private static func readAvailable(from handle: FileHandle) -> Data { + let descriptor = handle.fileDescriptor + let flags = fcntl(descriptor, F_GETFL) + guard flags != -1, fcntl(descriptor, F_SETFL, flags | O_NONBLOCK) != -1 else { return Data() } + + var output = Data() + var buffer = [UInt8](repeating: 0, count: 4_096) + while output.count < outputCap { + let received = buffer.withUnsafeMutableBytes { raw in + read(descriptor, raw.baseAddress, raw.count) + } + guard received > 0 else { break } + output.append(contentsOf: buffer[0 ..< received]) + } + return output + } +} diff --git a/TablePro/Core/Database/MySQLClientArguments.swift b/TablePro/Core/Database/MySQLClientArguments.swift new file mode 100644 index 0000000000..18229ff42d --- /dev/null +++ b/TablePro/Core/Database/MySQLClientArguments.swift @@ -0,0 +1,133 @@ +// +// MySQLClientArguments.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +/// The arguments a mysql-family client takes, which depend on which family it belongs to. +/// +/// MySQL and MariaDB no longer share an SSL surface. Measured, MariaDB 12.3.3 answers +/// `--ssl-mode=PREFERRED` with `unknown variable 'ssl-mode=PREFERRED'` and exit 7, and MySQL +/// 8.4.11 answers `--ssl` with `unknown option '--ssl'` and exit 2. Every mapping here was measured +/// against a MariaDB server with TLS off, a MariaDB server with a self-signed certificate and a +/// MySQL server with its own, using both client families. +enum MySQLClientArguments { + /// | mode | MySQL | MariaDB | + /// | --- | --- | --- | + /// | disabled | `--ssl-mode=DISABLED` | `--skip-ssl` | + /// | preferred | `--ssl-mode=PREFERRED` | `--ssl --skip-ssl-verify-server-cert` | + /// | required | `--ssl-mode=REQUIRED` | `--ssl --ssl-verify-server-cert` | + /// | verifyCa | `--ssl-mode=VERIFY_CA` | `--ssl --ssl-verify-server-cert` | + /// | verifyIdentity | `--ssl-mode=VERIFY_IDENTITY` | `--ssl --ssl-verify-server-cert` | + /// + /// `required` is the one that takes an argument. MariaDB has no flag for "encrypt and do not + /// verify": measured, `--ssl` on its own falls back to plaintext against a server without TLS + /// and reports success, which is the silent cleartext dump the mode exists to prevent. Only + /// `--ssl-verify-server-cert` refuses that server. It is not the stricter choice it reads as: + /// with no `--ssl-ca` given, MariaDB 12.3.3 accepted a server whose certificate said + /// `CN=totally.other.invalid`, so the flag requires TLS rather than an identity, and the + /// identity check is what `--ssl-ca` adds. MariaDB has no hostname-only tier, so `verifyCa` and + /// `verifyIdentity` reach it the same way. + static func tls(_ ssl: SSLConfiguration, flavor: NativeDumpToolFlavor, toolPath: String) throws -> [String] { + guard ssl.isEnabled else { + return disabledFlags(flavor: flavor) + } + return try modeFlags(ssl.mode, flavor: flavor, toolPath: toolPath) + certificateFlags(ssl) + } + + /// Everything the connection holds beyond the mode. The certificate implies `--ssl` on MariaDB, + /// so none of it is sent for a connection whose SSL is off, whatever the form left behind. + private static func certificateFlags(_ ssl: SSLConfiguration) -> [String] { + var flags: [String] = [] + if ssl.verifiesCertificate, !ssl.caCertificatePath.isEmpty { + flags.append("--ssl-ca=\(ssl.caCertificatePath)") + } + if !ssl.clientCertificatePath.isEmpty { + flags.append("--ssl-cert=\(ssl.clientCertificatePath)") + } + if !ssl.clientKeyPath.isEmpty { + flags.append("--ssl-key=\(ssl.clientKeyPath)") + } + return flags + } + + private static func disabledFlags(flavor: NativeDumpToolFlavor) -> [String] { + switch flavor { + case .mysql: return ["--ssl-mode=DISABLED"] + case .mariadb: return ["--skip-ssl"] + case .unidentified: return [] + } + } + + private static func modeFlags( + _ mode: SSLMode, + flavor: NativeDumpToolFlavor, + toolPath: String + ) throws -> [String] { + switch flavor { + case .mysql: + return [mysqlSSLMode(mode)] + case .mariadb: + return mode == .preferred + ? ["--ssl", "--skip-ssl-verify-server-cert"] + : ["--ssl", "--ssl-verify-server-cert"] + case .unidentified: + guard mode == .preferred else { throw unidentifiedToolError(toolPath) } + return [] + } + } + + /// A tool that answered nothing is not guessed at for a mode that promises encryption: picking + /// the wrong family's spelling either fails the dump or, for the flag MariaDB accepts and + /// ignores, sends it in cleartext. + private static func unidentifiedToolError(_ toolPath: String) -> NativeDumpError { + .incompatibleTool( + message: String( + format: String( + localized: """ + TablePro could not tell whether %@ is MySQL's client or MariaDB's. They take \ + different SSL options, and it will not guess for a connection that asks for \ + an encrypted one. Reinstall the client tools and try again. + """), + toolPath + ) + ) + } + + static func mysqlSSLMode(_ mode: SSLMode) -> String { + switch mode { + case .disabled: return "--ssl-mode=DISABLED" + case .preferred: return "--ssl-mode=PREFERRED" + case .required: return "--ssl-mode=REQUIRED" + case .verifyCa: return "--ssl-mode=VERIFY_CA" + case .verifyIdentity: return "--ssl-mode=VERIFY_IDENTITY" + } + } + + /// `mysqldump` 8.0 and newer read `information_schema.COLUMN_STATISTICS`, which no MariaDB + /// server and no MySQL server before 8.0 has. Measured against MariaDB 12.3.3: the dump stops + /// on `Unknown table 'column_statistics' in information_schema (1109)` and exits 2 having + /// written part of the file. With this flag it exits 0. + /// + /// Backup only. MariaDB's own dump tool does not know the flag, and neither restore client + /// takes it. + static func dumpCompatibility(tool: NativeDumpResolvedTool, serverVersion: String?) -> [String] { + guard tool.flavor == .mysql else { return [] } + guard let major = MySQLDumpToolIdentifier.majorVersion(fromVersionText: tool.versionText), major >= 8 else { + return [] + } + guard !serverHasColumnStatistics(serverVersion) else { return [] } + return ["--skip-column-statistics"] + } + + /// Answered from the server's own banner, and `true` when it cannot be read, so an unknown + /// server keeps the argument list it has always had. + static func serverHasColumnStatistics(_ serverVersion: String?) -> Bool { + guard let serverVersion, !serverVersion.isEmpty else { return true } + guard !serverVersion.lowercased().contains("mariadb") else { return false } + guard let major = Int(serverVersion.prefix { $0.isNumber }) else { return true } + return major >= 8 + } +} diff --git a/TablePro/Core/Database/MySQLDumpToolIdentifier.swift b/TablePro/Core/Database/MySQLDumpToolIdentifier.swift new file mode 100644 index 0000000000..8a53755d4e --- /dev/null +++ b/TablePro/Core/Database/MySQLDumpToolIdentifier.swift @@ -0,0 +1,66 @@ +// +// MySQLDumpToolIdentifier.swift +// TablePro +// + +import Foundation +import os + +/// Tells MySQL's dump and restore tools apart from MariaDB's. +/// +/// The two client families no longer share an option surface, and the binary's name does not say +/// which one is on disk: Homebrew's `mariadb` formula installs MariaDB's dump tool as +/// `/opt/homebrew/bin/mysqldump`, which is the first name the descriptor tries. Measured, the two +/// announce themselves differently and the token survives a rename, because it comes from the +/// build rather than from `argv[0]`: +/// +/// /opt/homebrew/bin/mysqldump from 12.3.3-MariaDB, client 10.20 for osx10.21 (arm64) +/// mysqldump Ver 8.4.11 for macos26.6 on arm64 (Homebrew) +enum MySQLDumpToolIdentifier { + private static let logger = Logger(subsystem: "com.TablePro", category: "MySQLDumpToolIdentifier") + + private static let mariaDBToken = "mariadb" + + /// The names MariaDB gave its clients in 11.0. A binary called one of these is MariaDB's + /// whatever `--version` says, which is the answer when the probe cannot run at all. + private static let mariaDBBinaryNames: Set = ["mariadb-dump", "mariadb"] + + static func identify( + name: String, + path: String, + probe: (String) -> String? = { CLIToolVersionProbe.versionOutput(of: $0) } + ) -> NativeDumpResolvedTool { + let versionText = probe(path)?.trimmingCharacters(in: .whitespacesAndNewlines) + let flavor = self.flavor(name: name, versionText: versionText) + if flavor == .unidentified { + logger.warning( + "\(name, privacy: .public) at \(path, privacy: .private(mask: .hash)) reports no readable version" + ) + } + return NativeDumpResolvedTool(name: name, path: path, flavor: flavor, versionText: versionText) + } + + static func flavor(name: String, versionText: String?) -> NativeDumpToolFlavor { + if let versionText, !versionText.isEmpty { + return versionText.lowercased().contains(mariaDBToken) ? .mariadb : .mysql + } + return mariaDBBinaryNames.contains(name.lowercased()) ? .mariadb : .unidentified + } + + /// The MySQL release the tool belongs to, read from its `--version` line. + /// + /// `Ver` is not always that number. MySQL 8 prints `mysqldump Ver 8.4.11 for macos26.6 on + /// arm64 (Homebrew)`, where it is, but 5.7 prints `mysqldump Ver 10.13 Distrib 5.7.44, for + /// ...`, where `Ver` is the tool's own version and `Distrib` is the release. Reading `Ver` + /// alone makes a 5.7 client look newer than an 8.4 one, which is the wrong way round for every + /// question worth asking of it. + static func majorVersion(fromVersionText versionText: String?) -> Int? { + guard let versionText else { return nil } + for pattern in [#"\bDistrib\s+"#, #"\bVer\s+"#] { + guard let marker = versionText.range(of: pattern, options: .regularExpression) else { continue } + let digits = versionText[marker.upperBound...].prefix { $0.isNumber } + if let major = Int(digits) { return major } + } + return nil + } +} diff --git a/TablePro/Core/Database/NativeDumpDescriptor.swift b/TablePro/Core/Database/NativeDumpDescriptor.swift index 88c354e7b1..1484c3447c 100644 --- a/TablePro/Core/Database/NativeDumpDescriptor.swift +++ b/TablePro/Core/Database/NativeDumpDescriptor.swift @@ -73,6 +73,10 @@ struct NativeDumpDescriptor: Sendable { /// result sheet then reports as a successful backup. let localFilePath: String? + /// What the live session's driver reports the server to be, so a tool can be given the + /// flags that server needs. `mysqldump` 8.0 reads a table no MariaDB server has. + let serverVersion: String? + init( connection: DatabaseConnection, database: String, @@ -80,7 +84,8 @@ struct NativeDumpDescriptor: Sendable { password: String?, scope: NativeDumpScope = .wholeDatabase, currentCatalog: String? = nil, - localFilePath: String? = nil + localFilePath: String? = nil, + serverVersion: String? = nil ) { self.connection = connection self.database = database @@ -89,6 +94,7 @@ struct NativeDumpDescriptor: Sendable { self.scope = scope self.currentCatalog = currentCatalog self.localFilePath = localFilePath + self.serverVersion = serverVersion } var host: String { @@ -130,8 +136,14 @@ struct NativeDumpDescriptor: Sendable { /// some servers. Nil leaves the plain PATH lookup in place. let toolForServer: (@Sendable (_ binary: String, _ serverVersion: String?) -> NativeDumpToolSelection)? - let backupArguments: @Sendable (Request) -> [String] - let restoreArguments: @Sendable (Request) -> [String] + /// Says what the resolved binary actually is, for an engine whose tools forked their + /// options. It runs a process, so it runs once per resolution rather than once per + /// argument list. Nil leaves the tool unidentified, which is what every engine but MySQL + /// and MariaDB wants. + let identifyExecutable: (@Sendable (_ name: String, _ path: String) -> NativeDumpResolvedTool)? + + let backupArguments: @Sendable (Request, NativeDumpResolvedTool) throws -> [String] + let restoreArguments: @Sendable (Request, NativeDumpResolvedTool) throws -> [String] let environment: @Sendable (Request) -> [String: String] init( @@ -145,8 +157,9 @@ struct NativeDumpDescriptor: Sendable { restoreExitPolicy: NativeDumpExitPolicy = .zeroExitOnly, requiresUntranslatedMessages: Bool = false, toolForServer: (@Sendable (_ binary: String, _ serverVersion: String?) -> NativeDumpToolSelection)? = nil, - backupArguments: @escaping @Sendable (Request) -> [String], - restoreArguments: @escaping @Sendable (Request) -> [String], + identifyExecutable: (@Sendable (_ name: String, _ path: String) -> NativeDumpResolvedTool)? = nil, + backupArguments: @escaping @Sendable (Request, NativeDumpResolvedTool) throws -> [String], + restoreArguments: @escaping @Sendable (Request, NativeDumpResolvedTool) throws -> [String], environment: @escaping @Sendable (Request) -> [String: String] = { _ in [:] } ) { self.backupBinaries = backupBinaries @@ -159,6 +172,7 @@ struct NativeDumpDescriptor: Sendable { self.restoreExitPolicy = restoreExitPolicy self.requiresUntranslatedMessages = requiresUntranslatedMessages self.toolForServer = toolForServer + self.identifyExecutable = identifyExecutable self.backupArguments = backupArguments self.restoreArguments = restoreArguments self.environment = environment @@ -168,8 +182,18 @@ struct NativeDumpDescriptor: Sendable { kind == .backup ? backupBinaries : restoreBinaries } - func arguments(for kind: NativeDumpKind, request: Request) -> [String] { - kind == .backup ? backupArguments(request) : restoreArguments(request) + func arguments( + for kind: NativeDumpKind, + request: Request, + resolved: NativeDumpResolvedTool + ) throws -> [String] { + try kind == .backup ? backupArguments(request, resolved) : restoreArguments(request, resolved) + } + + /// What the app knows about the binary it resolved. An engine that declares no + /// `identifyExecutable` gets the plain answer, which is what its arguments already assume. + func identify(name: String, path: String) -> NativeDumpResolvedTool { + identifyExecutable?(name, path) ?? NativeDumpResolvedTool(name: name, path: path) } func delivery(for kind: NativeDumpKind) -> OutputDelivery { diff --git a/TablePro/Core/Database/NativeDumpRegistry.swift b/TablePro/Core/Database/NativeDumpRegistry.swift index fe48df12b2..2be8264d80 100644 --- a/TablePro/Core/Database/NativeDumpRegistry.swift +++ b/TablePro/Core/Database/NativeDumpRegistry.swift @@ -92,13 +92,13 @@ enum NativeDumpRegistry { restoreExitPolicy: .toleratesUnrecognizedSessionSettings, requiresUntranslatedMessages: true, toolForServer: toolForServer, - backupArguments: { request in + backupArguments: { request, _ in connectionFlags(request) + ["-Fc", "-d", request.database] + postgresTableFlags(request) + ["-f", request.fileURL.path] }, - restoreArguments: { request in + restoreArguments: { request, _ in connectionFlags(request) + ["--no-owner", "--no-acl", "-d", request.database, request.fileURL.path] }, environment: { request in @@ -106,10 +106,7 @@ enum NativeDumpRegistry { if let password = request.password, !password.isEmpty { environment["PGPASSWORD"] = password } - if request.connection.sslConfig.isEnabled, - let mode = postgresSSLMode(request.connection.sslConfig.mode) { - environment["PGSSLMODE"] = mode - } + environment.merge(postgresSSLEnvironment(request.connection.sslConfig)) { _, new in new } return environment } ) @@ -155,6 +152,28 @@ enum NativeDumpRegistry { } } + /// The whole of the connection's SSL configuration, not just its mode. + /// + /// libpq falls back to `~/.postgresql/root.crt` when no root certificate is named, and measured + /// with pg_dump 17.11 a `verify-ca` connection with no such file fails before connecting with + /// `root certificate file "..." does not exist`. So a Verify CA connection that opens in the + /// app could never be dumped. The rule matches what `LibPQConnectionString` already sends on + /// the live connection, down to the CA being tied to the modes that verify. + static func postgresSSLEnvironment(_ ssl: SSLConfiguration) -> [String: String] { + guard ssl.isEnabled, let mode = postgresSSLMode(ssl.mode) else { return [:] } + var environment = ["PGSSLMODE": mode] + if ssl.verifiesCertificate, !ssl.caCertificatePath.isEmpty { + environment["PGSSLROOTCERT"] = ssl.caCertificatePath + } + if !ssl.clientCertificatePath.isEmpty { + environment["PGSSLCERT"] = ssl.clientCertificatePath + } + if !ssl.clientKeyPath.isEmpty { + environment["PGSSLKEY"] = ssl.clientKeyPath + } + return environment + } + // MARK: - MySQL and MariaDB /// MariaDB 11.0 renamed every client, keeping the `mysql`-prefixed names as symlinks that some @@ -168,18 +187,25 @@ enum NativeDumpRegistry { installHint: String(localized: "Install it with `brew install mysql-client` and link it."), backupDelivery: .standardOutput, restoreDelivery: .standardOutput, - backupArguments: { request in - mysqlConnectionFlags(request) + [ + identifyExecutable: { name, path in + MySQLDumpToolIdentifier.identify(name: name, path: path) + }, + backupArguments: { request, resolved in + try mysqlConnectionFlags(request, resolved) + [ "--single-transaction", "--routines", "--triggers", "--events", - "--default-character-set=utf8mb4", - request.database - ] + mysqlTableArguments(request) + "--default-character-set=utf8mb4" + ] + + MySQLClientArguments.dumpCompatibility( + tool: resolved, serverVersion: request.serverVersion + ) + + mysqlObjectArguments(request) }, - restoreArguments: { request in - mysqlConnectionFlags(request) + ["--default-character-set=utf8mb4", request.database] + restoreArguments: { request, resolved in + try mysqlConnectionFlags(request, resolved) + + ["--default-character-set=utf8mb4", "--", request.database] }, environment: { request in guard let password = request.password, !password.isEmpty else { return [:] } @@ -201,40 +227,34 @@ enum NativeDumpRegistry { ) } - /// `--` before the table list, because `my_getopt` does not stop parsing options at the first - /// positional argument. Measured with mysqldump 12.3.2: a table named `--no-data` passed as a - /// bare argument was read as the option and the dump came back with zero rows, exit 0, which - /// the result sheet reports as a successful backup. The same trick reaches `--ssl-mode=DISABLED` - /// and sends the whole dump in cleartext. With `--` in front, the name is a table again. - private static func mysqlTableArguments(_ request: NativeDumpDescriptor.Request) -> [String] { - let names = request.scope.objects.map(\.name) - guard !names.isEmpty else { return [] } - return ["--"] + names + /// `--` before the database, because `my_getopt` does not stop parsing options at the first + /// positional argument and every name after it is the user's. Measured with mysqldump 8.4.11 + /// and 12.3.2, a database named `--no-data` with the terminator behind it dumped a *different* + /// database with no rows and exited 0, which the result sheet reports as a successful backup; + /// a table named `--ssl-mode=DISABLED` in the same slot sends the whole dump in cleartext. With + /// `--` in front of the database, both are names again. + private static func mysqlObjectArguments(_ request: NativeDumpDescriptor.Request) -> [String] { + ["--", request.database] + request.scope.objects.map(\.name) } - private static func mysqlConnectionFlags(_ request: NativeDumpDescriptor.Request) -> [String] { + private static func mysqlConnectionFlags( + _ request: NativeDumpDescriptor.Request, + _ resolved: NativeDumpResolvedTool + ) throws -> [String] { var flags = ["--protocol=TCP", "-h", request.host, "-P", String(request.connection.port)] if !request.connection.username.isEmpty { flags.append(contentsOf: ["-u", request.connection.username]) } - if request.connection.sslConfig.isEnabled { - flags.append(mysqlSSLMode(request.connection.sslConfig.mode)) - } else { - flags.append("--ssl-mode=DISABLED") - } + flags.append( + contentsOf: try MySQLClientArguments.tls( + request.connection.sslConfig, + flavor: resolved.flavor, + toolPath: resolved.path + ) + ) return flags } - static func mysqlSSLMode(_ mode: SSLMode) -> String { - switch mode { - case .disabled: return "--ssl-mode=DISABLED" - case .preferred: return "--ssl-mode=PREFERRED" - case .required: return "--ssl-mode=REQUIRED" - case .verifyCa: return "--ssl-mode=VERIFY_CA" - case .verifyIdentity: return "--ssl-mode=VERIFY_IDENTITY" - } - } - // MARK: - MongoDB /// `mongodump` reads no password from the environment, and a password in `argv` is readable by @@ -250,13 +270,13 @@ enum NativeDumpRegistry { backupDelivery: .toolWritesFile, restoreDelivery: .toolWritesFile, needsCredentialsFile: true, - backupArguments: { request in + backupArguments: { request, _ in mongoConnectionFlags(request) + mongoNamespaceFlags(request) + [ "--gzip", "--archive=\(request.fileURL.path)" ] }, - restoreArguments: { request in + restoreArguments: { request, _ in mongoConnectionFlags(request) + [ "--nsInclude=\(request.database).*", "--gzip", @@ -313,13 +333,13 @@ enum NativeDumpRegistry { backupDelivery: .toolWritesFile, restoreDelivery: .toolWritesFile, exposesPasswordInArguments: true, - backupArguments: { request in + backupArguments: { request, _ in ["/Action:Export", "/TargetFile:\(request.fileURL.path)", "/SourceConnectionString:\(sqlServerConnectionString(request))"] + request.scope.objects.map { "/p:TableData=\(sqlServerTableData($0))" } }, - restoreArguments: { request in + restoreArguments: { request, _ in ["/Action:Import", "/SourceFile:\(request.fileURL.path)", "/TargetConnectionString:\(sqlServerConnectionString(request))"] @@ -384,7 +404,7 @@ enum NativeDumpRegistry { installHint: String(localized: "Install it with `brew install sqlite` and link it."), backupDelivery: .standardOutput, restoreDelivery: .standardOutput, - backupArguments: { request in + backupArguments: { request, _ in [ sqlitePath(request), NativeDumpArgumentQuoting.sqliteDumpCommand( @@ -392,7 +412,7 @@ enum NativeDumpRegistry { ) ] }, - restoreArguments: { request in [sqlitePath(request)] } + restoreArguments: { request, _ in [sqlitePath(request)] } ) ), archiveFormat: NativeDumpDescriptor.ArchiveFormat( diff --git a/TablePro/Core/Database/NativeDumpResolvedTool.swift b/TablePro/Core/Database/NativeDumpResolvedTool.swift new file mode 100644 index 0000000000..bd81e82d99 --- /dev/null +++ b/TablePro/Core/Database/NativeDumpResolvedTool.swift @@ -0,0 +1,44 @@ +// +// NativeDumpResolvedTool.swift +// TablePro +// + +import Foundation + +/// Which family a resolved binary belongs to, when its arguments depend on the answer. +/// +/// `unidentified` is not a failure on its own: most engines never ask, and a tool that did not +/// answer `--version` is still run. It is only the engines whose option surface forked that have to +/// act on it. +enum NativeDumpToolFlavor: String, Equatable, Sendable { + case unidentified + case mysql + case mariadb +} + +/// The binary a dump or restore is about to run, and what the app knows about it. +/// +/// It exists because an argument list is not a function of the connection alone. MariaDB renamed +/// every client in 11.0 and forked their options, and Homebrew's `mariadb` formula installs +/// MariaDB's dump tool under the name `mysqldump`, so neither the engine nor the binary's name says +/// which options the thing on disk accepts. Whoever builds the arguments gets the answer handed to +/// it rather than guessing. +struct NativeDumpResolvedTool: Equatable, Sendable { + let name: String + let path: String + let flavor: NativeDumpToolFlavor + /// What the tool printed for `--version`, kept so a descriptor can read a version out of it + /// without probing again. + let versionText: String? + + init(name: String, path: String, flavor: NativeDumpToolFlavor = .unidentified, versionText: String? = nil) { + self.name = name + self.path = path + self.flavor = flavor + self.versionText = versionText + } + + var executableURL: URL { + URL(fileURLWithPath: path) + } +} diff --git a/TablePro/Core/Database/NativeDumpService.swift b/TablePro/Core/Database/NativeDumpService.swift index b4b5d95ac9..4ceaccc975 100644 --- a/TablePro/Core/Database/NativeDumpService.swift +++ b/TablePro/Core/Database/NativeDumpService.swift @@ -184,6 +184,7 @@ final class NativeDumpService: ObservableObject { private var byteSizeTask: Task? private var stateObservers: [UUID: AsyncStream.Continuation] = [:] private var toolName = "dump" + private var cancelRequested = false func stateUpdates() -> AsyncStream { let (stream, continuation) = AsyncStream.makeStream() @@ -272,6 +273,7 @@ final class NativeDumpService: ObservableObject { ? nil : await Self.resolvedCatalog(connectionId: connection.id, database: database) + let serverVersion = session?.driver?.serverVersion let request = NativeDumpDescriptor.Request( connection: effective, database: database, @@ -279,21 +281,22 @@ final class NativeDumpService: ObservableObject { password: password, scope: scope, currentCatalog: catalog, - localFilePath: localFilePath + localFilePath: localFilePath, + serverVersion: serverVersion ) switch descriptor.mechanism { case .commandLineTool(let tool): - let (binaryName, resolvedPath) = try await Self.resolveExecutable( + let resolved = try await Self.resolveExecutable( tool: tool, kind: kind, - serverVersion: session?.driver?.serverVersion + serverVersion: serverVersion ) - toolName = binaryName + toolName = resolved.name let command = try Self.buildCommand( kind: kind, tool: tool, - executable: URL(fileURLWithPath: resolvedPath), + resolved: resolved, request: request ) try run(job: .process(command), database: database, fileURL: fileURL, totalBytesEstimate: totalBytesEstimate) @@ -333,6 +336,10 @@ final class NativeDumpService: ObservableObject { ) throws { if case .running = state { throw NativeDumpError.alreadyRunning } if case .cancelling = state { throw NativeDumpError.alreadyRunning } + guard !cancelRequested else { + setState(.cancelled) + return + } let runner = runnerFactory(job) try runner.start() @@ -350,7 +357,14 @@ final class NativeDumpService: ObservableObject { } } + /// A cancel that arrives before the tool is running is latched rather than dropped. + /// + /// `start` finds the binary and asks it what it is before there is anything to cancel, and both + /// steps spawn a process, so the window is real: a Cancel taken during it used to be discarded + /// and the tool launched afterwards, which on a restore means writing to the target database + /// the user just said to leave alone. func cancel() { + cancelRequested = true guard case .running = state else { return } setState(.cancelling) runner?.cancel() @@ -358,11 +372,13 @@ final class NativeDumpService: ObservableObject { // MARK: - Resolution + /// Both halves run off the main actor together: the lookup spawns `which`, and identifying what + /// it found spawns the tool itself. private static func resolveExecutable( tool: NativeDumpDescriptor.CommandLineTool, kind: NativeDumpKind, serverVersion: String? - ) async throws -> (name: String, path: String) { + ) async throws -> NativeDumpResolvedTool { let candidates = tool.binaries(for: kind) let selector = tool.toolForServer let resolved = await Task.detached { @@ -370,7 +386,7 @@ final class NativeDumpService: ObservableObject { }.value switch resolved { case .found(let name, let path): - return (name, path) + return await Task.detached { tool.identify(name: name, path: path) }.value case .incompatible(let message): throw NativeDumpError.incompatibleTool(message: message) case .missing: @@ -472,10 +488,10 @@ final class NativeDumpService: ObservableObject { nonisolated static func buildCommand( kind: NativeDumpKind, tool: NativeDumpDescriptor.CommandLineTool, - executable: URL, + resolved: NativeDumpResolvedTool, request: NativeDumpDescriptor.Request ) throws -> NativeDumpCommand { - var arguments = tool.arguments(for: kind, request: request) + var arguments = try tool.arguments(for: kind, request: request, resolved: resolved) var environment = minimalEnvironment() environment.merge(tool.environment(request)) { _, new in new } if tool.requiresUntranslatedMessages { @@ -493,7 +509,7 @@ final class NativeDumpService: ObservableObject { let delivery = tool.delivery(for: kind) return NativeDumpCommand( - executable: executable, + executable: resolved.executableURL, arguments: arguments, environment: environment, stderrByteCap: 64_000, diff --git a/TablePro/Core/Database/PostgreSQLDumpToolLocator.swift b/TablePro/Core/Database/PostgreSQLDumpToolLocator.swift index 1c98accf1c..61871f0b1e 100644 --- a/TablePro/Core/Database/PostgreSQLDumpToolLocator.swift +++ b/TablePro/Core/Database/PostgreSQLDumpToolLocator.swift @@ -105,7 +105,7 @@ enum PostgreSQLDumpToolLocator { } static func probeVersion(of path: String, timeout: TimeInterval = versionProbeTimeout) -> ProbedVersion { - guard let output = versionOutput(of: path, timeout: timeout), + guard let output = CLIToolVersionProbe.versionOutput(of: path, timeout: timeout), let version = PostgreSQLServerVersion(output) else { return .unknown } @@ -135,30 +135,4 @@ enum PostgreSQLDumpToolLocator { private static func resolvedPath(_ path: String) -> String { URL(fileURLWithPath: path).resolvingSymlinksInPath().path } - - private static func versionOutput(of path: String, timeout: TimeInterval) -> String? { - let process = Process() - process.executableURL = URL(fileURLWithPath: path) - process.arguments = ["--version"] - process.environment = CLIToolEnvironment.augmented() - let pipe = Pipe() - process.standardOutput = pipe - process.standardError = FileHandle.nullDevice - let finished = DispatchSemaphore(value: 0) - process.terminationHandler = { _ in finished.signal() } - do { - try process.run() - } catch { - return nil - } - - if finished.wait(timeout: .now() + timeout) == .timedOut { - process.terminate() - logger.warning("\(path, privacy: .private(mask: .hash)) did not answer --version within \(timeout, privacy: .public)s") - return nil - } - guard process.terminationStatus == 0 else { return nil } - let data = pipe.fileHandleForReading.readDataToEndOfFile() - return String(data: data, encoding: .utf8) - } } diff --git a/TableProTests/Database/CLIToolVersionProbeTests.swift b/TableProTests/Database/CLIToolVersionProbeTests.swift new file mode 100644 index 0000000000..807284ea56 --- /dev/null +++ b/TableProTests/Database/CLIToolVersionProbeTests.swift @@ -0,0 +1,52 @@ +// +// CLIToolVersionProbeTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("CLI tool version probe") +struct CLIToolVersionProbeTests { + private func script(_ body: String) throws -> String { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("tablepro-probe-\(UUID().uuidString).sh") + try ("#!/bin/sh\n" + body + "\n").write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: url.path) + return url.path + } + + @Test("A tool's answer comes back as it printed it") + func readsTheBanner() throws { + let path = try script("printf '%s' 'mysqldump Ver 8.4.11 for macos26.6 on arm64 (Homebrew)'") + defer { try? FileManager.default.removeItem(atPath: path) } + #expect(CLIToolVersionProbe.versionOutput(of: path)?.contains("Ver 8.4.11") == true) + } + + @Test("A tool that cannot run answers nothing") + func missingBinary() { + #expect(CLIToolVersionProbe.versionOutput(of: "/nonexistent/mysqldump") == nil) + } + + @Test("A tool that fails answers nothing") + func nonZeroExit() throws { + let path = try script("printf '%s' 'half an answer'\nexit 1") + defer { try? FileManager.default.removeItem(atPath: path) } + #expect(CLIToolVersionProbe.versionOutput(of: path) == nil) + } + + /// The deadline covers the process, and a wrapper that prints its version, starts a helper + /// holding standard output and exits leaves EOF to the helper. Reading to EOF would wait for + /// that helper, which is a dump that never starts. + @Test("A child left holding standard output does not hold up the answer", .timeLimit(.minutes(1))) + func survivingChildDoesNotBlock() throws { + let path = try script("(sleep 30) &\nprintf '%s' 'mysqldump Ver 8.4.11'") + defer { try? FileManager.default.removeItem(atPath: path) } + let started = Date() + let answer = CLIToolVersionProbe.versionOutput(of: path) + #expect(answer?.contains("Ver 8.4.11") == true) + #expect(Date().timeIntervalSince(started) < 10) + } +} diff --git a/TableProTests/Database/MySQLClientArgumentsTests.swift b/TableProTests/Database/MySQLClientArgumentsTests.swift new file mode 100644 index 0000000000..0a9f5cc1ab --- /dev/null +++ b/TableProTests/Database/MySQLClientArgumentsTests.swift @@ -0,0 +1,171 @@ +// +// MySQLClientArgumentsTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@testable import TablePro + +/// Every expectation here was measured against MariaDB 12.3.3 and MySQL 8.4.11 client tools, run +/// against a MariaDB server with TLS off, a MariaDB server with a self-signed certificate and a +/// MySQL server with its own. +@Suite("MySQL client arguments") +struct MySQLClientArgumentsTests { + private func ssl( + _ mode: SSLMode, + ca: String = "", + certificate: String = "", + key: String = "" + ) -> SSLConfiguration { + SSLConfiguration( + mode: mode, + caCertificatePath: ca, + clientCertificatePath: certificate, + clientKeyPath: key + ) + } + + @Test("MySQL's tools take --ssl-mode for every mode") + func mysqlSpelling() throws { + let expected: [SSLMode: String] = [ + .disabled: "--ssl-mode=DISABLED", + .preferred: "--ssl-mode=PREFERRED", + .required: "--ssl-mode=REQUIRED", + .verifyCa: "--ssl-mode=VERIFY_CA", + .verifyIdentity: "--ssl-mode=VERIFY_IDENTITY" + ] + for (mode, flag) in expected { + let flags = try MySQLClientArguments.tls(ssl(mode), flavor: .mysql, toolPath: "/usr/bin/mysqldump") + #expect(flags == [flag], "\(mode.rawValue) should map to \(flag)") + } + } + + /// MariaDB has no `--ssl-mode` at all: measured, it answers `unknown variable 'ssl-mode=...'` + /// and exits 7 whatever the value. + @Test("MariaDB's tools never see --ssl-mode") + func mariaDBNeverSeesSSLMode() throws { + for mode in SSLMode.allCases { + let flags = try MySQLClientArguments.tls(ssl(mode), flavor: .mariadb, toolPath: "/usr/bin/mysqldump") + #expect(!flags.contains { $0.hasPrefix("--ssl-mode") }, "\(mode.rawValue) leaked --ssl-mode") + } + } + + /// `--ssl` on its own falls back to plaintext against a server without TLS and exits 0, so it + /// cannot carry Required. Only `--ssl-verify-server-cert` refuses that server. + @Test("MariaDB's spelling distinguishes preferred from required") + func mariaDBSpelling() throws { + #expect( + try MySQLClientArguments.tls(ssl(.disabled), flavor: .mariadb, toolPath: "/t") == ["--skip-ssl"] + ) + #expect( + try MySQLClientArguments.tls(ssl(.preferred), flavor: .mariadb, toolPath: "/t") + == ["--ssl", "--skip-ssl-verify-server-cert"] + ) + for mode in [SSLMode.required, .verifyCa, .verifyIdentity] { + #expect( + try MySQLClientArguments.tls(ssl(mode), flavor: .mariadb, toolPath: "/t") + == ["--ssl", "--ssl-verify-server-cert"] + ) + } + } + + @Test("The CA goes with the modes that verify one, and the client pair always follows") + func certificatePaths() throws { + let verifying = ssl(.verifyCa, ca: "/certs/ca.pem", certificate: "/certs/c.pem", key: "/certs/c.key") + for flavor in [NativeDumpToolFlavor.mysql, .mariadb] { + let flags = try MySQLClientArguments.tls(verifying, flavor: flavor, toolPath: "/t") + #expect(flags.contains("--ssl-ca=/certs/ca.pem")) + #expect(flags.contains("--ssl-cert=/certs/c.pem")) + #expect(flags.contains("--ssl-key=/certs/c.key")) + } + + let required = ssl(.required, ca: "/certs/ca.pem", certificate: "/certs/c.pem") + let flags = try MySQLClientArguments.tls(required, flavor: .mysql, toolPath: "/t") + #expect(!flags.contains { $0.hasPrefix("--ssl-ca") }) + #expect(flags.contains("--ssl-cert=/certs/c.pem")) + } + + /// `--ssl-cert` implies `--ssl` on MariaDB, so a connection the user turned SSL off on must not + /// carry the paths its form still holds. + @Test("An SSL-off connection sends no certificate paths") + func disabledSendsNothingElse() throws { + let stale = ssl(.disabled, ca: "/certs/ca.pem", certificate: "/certs/c.pem", key: "/certs/c.key") + #expect(try MySQLClientArguments.tls(stale, flavor: .mariadb, toolPath: "/t") == ["--skip-ssl"]) + #expect(try MySQLClientArguments.tls(stale, flavor: .mysql, toolPath: "/t") == ["--ssl-mode=DISABLED"]) + } + + /// Guessing costs more than refusing here: MariaDB accepts `--loose-ssl-mode=REQUIRED`, ignores + /// it, and sends the dump in cleartext. + @Test("An unidentified tool refuses every mode that promises encryption") + func unidentifiedRefusesEncryptedModes() throws { + for mode in [SSLMode.required, .verifyCa, .verifyIdentity] { + #expect(throws: NativeDumpError.self) { + try MySQLClientArguments.tls(ssl(mode), flavor: .unidentified, toolPath: "/usr/bin/mysqldump") + } + } + #expect(try MySQLClientArguments.tls(ssl(.preferred), flavor: .unidentified, toolPath: "/t").isEmpty) + #expect(try MySQLClientArguments.tls(ssl(.disabled), flavor: .unidentified, toolPath: "/t").isEmpty) + } + + private func tool(_ flavor: NativeDumpToolFlavor, _ versionText: String?) -> NativeDumpResolvedTool { + NativeDumpResolvedTool(name: "mysqldump", path: "/usr/bin/mysqldump", flavor: flavor, versionText: versionText) + } + + private static let mysql8 = "mysqldump Ver 8.4.11 for macos26.6 on arm64 (Homebrew)" + private static let mysql57 = "mysqldump Ver 10.13 Distrib 5.7.44, for osx10.17 (x86_64)" + private static let mariaDB = "mysqldump from 12.3.3-MariaDB, client 10.20 for osx10.21 (arm64)" + + /// Measured against MariaDB 12.3.3: without the flag mysqldump 8.4.11 stops on + /// `Unknown table 'column_statistics' in information_schema (1109)` and exits 2 having written + /// part of the file. + @Test("Column statistics are skipped only where the server has none and the tool asks for them") + func columnStatistics() { + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mysql, Self.mysql8), serverVersion: "12.3.3-MariaDB" + ) == ["--skip-column-statistics"] + ) + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mysql, Self.mysql8), serverVersion: "5.7.44-log" + ) == ["--skip-column-statistics"] + ) + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mysql, Self.mysql8), serverVersion: "8.0.36" + ).isEmpty + ) + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mysql, Self.mysql57), serverVersion: "12.3.3-MariaDB" + ).isEmpty, + "a 5.7 tool does not know the flag and never reads the table" + ) + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mariadb, Self.mariaDB), serverVersion: "12.3.3-MariaDB" + ).isEmpty, + "MariaDB's own tool answers unknown option" + ) + #expect( + MySQLClientArguments.dumpCompatibility( + tool: tool(.mysql, Self.mysql8), serverVersion: nil + ).isEmpty, + "an unreadable server banner keeps the argument list it has always had" + ) + } + + @Test("The column statistics table is read off the server's own banner") + func serverColumnStatistics() { + #expect(!MySQLClientArguments.serverHasColumnStatistics("12.3.3-MariaDB")) + #expect(!MySQLClientArguments.serverHasColumnStatistics("10.11.2-MariaDB-log")) + #expect(!MySQLClientArguments.serverHasColumnStatistics("5.7.44-log")) + #expect(MySQLClientArguments.serverHasColumnStatistics("8.0.36")) + #expect(MySQLClientArguments.serverHasColumnStatistics("9.1.0")) + #expect(MySQLClientArguments.serverHasColumnStatistics(nil)) + #expect(MySQLClientArguments.serverHasColumnStatistics("")) + } +} diff --git a/TableProTests/Database/MySQLDumpToolIdentifierTests.swift b/TableProTests/Database/MySQLDumpToolIdentifierTests.swift new file mode 100644 index 0000000000..05e95abf5a --- /dev/null +++ b/TableProTests/Database/MySQLDumpToolIdentifierTests.swift @@ -0,0 +1,84 @@ +// +// MySQLDumpToolIdentifierTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +/// The version strings here are verbatim output from the binaries on a Mac with both families +/// installed, including the renamed copy that proves the token comes from the build rather than +/// from `argv[0]`. +@Suite("MySQL dump tool identifier") +struct MySQLDumpToolIdentifierTests { + private static let mariaDB = "/opt/homebrew/bin/mysqldump from 12.3.3-MariaDB, client 10.20 for osx10.21 (arm64)" + private static let renamedMariaDB = "./totally-not-mariadb from 12.3.3-MariaDB, client 10.20 for osx10.21 (arm64)" + private static let mysql84 = "mysqldump Ver 8.4.11 for macos26.6 on arm64 (Homebrew)" + private static let mysql57 = "mysqldump Ver 10.13 Distrib 5.7.44, for osx10.17 (x86_64)" + + /// Homebrew installs MariaDB's dump tool as `mysqldump`, so the name proves nothing and the + /// banner is the answer (#3046). + @Test("A MariaDB tool is recognized whatever it is called") + func mariaDBFromBanner() { + #expect(MySQLDumpToolIdentifier.flavor(name: "mysqldump", versionText: Self.mariaDB) == .mariadb) + let renamed = MySQLDumpToolIdentifier.flavor(name: "totally-not-mariadb", versionText: Self.renamedMariaDB) + #expect(renamed == .mariadb) + #expect(MySQLDumpToolIdentifier.flavor(name: "mysql", versionText: Self.mariaDB) == .mariadb) + } + + @Test("A banner without the MariaDB token is MySQL's") + func mysqlFromBanner() { + #expect(MySQLDumpToolIdentifier.flavor(name: "mysqldump", versionText: Self.mysql84) == .mysql) + #expect(MySQLDumpToolIdentifier.flavor(name: "mariadb-dump", versionText: Self.mysql84) == .mysql) + #expect(MySQLDumpToolIdentifier.flavor(name: "mysqldump", versionText: Self.mysql57) == .mysql) + } + + /// MariaDB renamed its clients in 11.0, so those names still answer when the tool itself will + /// not. A binary called `mysqldump` that says nothing stays unidentified rather than being + /// assumed to be either one. + @Test("A tool that answers nothing falls back to its name, and only that far") + func unreadableVersion() { + #expect(MySQLDumpToolIdentifier.flavor(name: "mariadb-dump", versionText: nil) == .mariadb) + #expect(MySQLDumpToolIdentifier.flavor(name: "mariadb", versionText: "") == .mariadb) + #expect(MySQLDumpToolIdentifier.flavor(name: "mysqldump", versionText: nil) == .unidentified) + #expect(MySQLDumpToolIdentifier.flavor(name: "mysql", versionText: "") == .unidentified) + } + + /// `Ver` carries the tool's own version on 5.7 and the release on 8, so reading it alone makes + /// a 5.7 client look like a 10. + @Test("The MySQL release is read from Distrib when the banner carries one") + func majorVersion() { + #expect(MySQLDumpToolIdentifier.majorVersion(fromVersionText: Self.mysql84) == 8) + #expect(MySQLDumpToolIdentifier.majorVersion(fromVersionText: Self.mysql57) == 5) + let mysql80 = "mysqldump Ver 8.0.36 for macos14 on arm64" + #expect(MySQLDumpToolIdentifier.majorVersion(fromVersionText: mysql80) == 8) + #expect(MySQLDumpToolIdentifier.majorVersion(fromVersionText: Self.mariaDB) == nil) + #expect(MySQLDumpToolIdentifier.majorVersion(fromVersionText: nil) == nil) + } + + @Test("Identifying a tool keeps what it said, so nothing probes it twice") + func identifyCarriesTheBanner() { + let resolved = MySQLDumpToolIdentifier.identify( + name: "mysqldump", + path: "/opt/homebrew/bin/mysqldump", + probe: { _ in Self.mariaDB + "\n" } + ) + #expect(resolved.flavor == .mariadb) + #expect(resolved.versionText == Self.mariaDB) + #expect(resolved.path == "/opt/homebrew/bin/mysqldump") + #expect(resolved.executableURL.path == "/opt/homebrew/bin/mysqldump") + } + + @Test("A tool that cannot be run is reported unidentified rather than assumed") + func identifyWithoutAProbe() { + let resolved = MySQLDumpToolIdentifier.identify( + name: "mysqldump", + path: "/usr/bin/mysqldump", + probe: { _ in nil } + ) + #expect(resolved.flavor == .unidentified) + #expect(resolved.versionText == nil) + } +} diff --git a/TableProTests/Database/NativeDumpRegistryTests.swift b/TableProTests/Database/NativeDumpRegistryTests.swift index 9790dbb39e..84ade80c58 100644 --- a/TableProTests/Database/NativeDumpRegistryTests.swift +++ b/TableProTests/Database/NativeDumpRegistryTests.swift @@ -11,7 +11,6 @@ import Testing @Suite("Native dump registry") struct NativeDumpRegistryTests { - private func connection( type: DatabaseType, host: String = "db.example.com", @@ -43,21 +42,30 @@ struct NativeDumpRegistryTests { password: String? = "s3cret", fileURL: URL = URL(fileURLWithPath: "/tmp/out.bin"), scope: NativeDumpScope = .wholeDatabase, - localFilePath: String? = nil + localFilePath: String? = nil, + flavor: NativeDumpToolFlavor = .mysql, + toolVersionText: String? = nil, + serverVersion: String? = nil ) throws -> NativeDumpCommand { let tool = try #require(NativeDumpRegistry.descriptor(for: type)?.commandLineTool) let effective = overrideConnection ?? connection(type: type) return try NativeDumpService.buildCommand( kind: kind, tool: tool, - executable: URL(fileURLWithPath: "/usr/bin/tool"), + resolved: NativeDumpResolvedTool( + name: "tool", + path: "/usr/bin/tool", + flavor: flavor, + versionText: toolVersionText + ), request: NativeDumpDescriptor.Request( connection: effective, database: "sales", fileURL: fileURL, password: password, scope: scope, - localFilePath: localFilePath ?? effective.database + localFilePath: localFilePath ?? effective.database, + serverVersion: serverVersion ) ) } @@ -225,13 +233,161 @@ struct NativeDumpRegistryTests { #expect(mongo.arguments.contains("--host=127.0.0.1")) } - @Test("MySQL SSL mode maps to the client's own spelling") - func mysqlSSLModes() { - #expect(NativeDumpRegistry.mysqlSSLMode(.disabled) == "--ssl-mode=DISABLED") - #expect(NativeDumpRegistry.mysqlSSLMode(.preferred) == "--ssl-mode=PREFERRED") - #expect(NativeDumpRegistry.mysqlSSLMode(.required) == "--ssl-mode=REQUIRED") - #expect(NativeDumpRegistry.mysqlSSLMode(.verifyCa) == "--ssl-mode=VERIFY_CA") - #expect(NativeDumpRegistry.mysqlSSLMode(.verifyIdentity) == "--ssl-mode=VERIFY_IDENTITY") + /// Measured, MariaDB 12.3.3 answers any `--ssl-mode` with `unknown variable` and exit 7, and + /// MySQL 8.4.11 answers `--ssl` with `unknown option` and exit 2, so neither spelling may reach + /// the other family's tool in either direction (#3046). + @Test("Neither client family is ever handed the other one's SSL flags", arguments: [ + NativeDumpKind.backup, .restore + ]) + func sslFlagsFollowTheResolvedTool(kind: NativeDumpKind) throws { + let secured = connection(type: .mysql, sslMode: .required, sslEnabled: true) + let maria = try command(.mysql, kind: kind, connection: secured, flavor: .mariadb) + #expect(!maria.arguments.contains { $0.hasPrefix("--ssl-mode") }) + #expect(maria.arguments.contains("--ssl")) + #expect(maria.arguments.contains("--ssl-verify-server-cert")) + + let mysql = try command(.mysql, kind: kind, connection: secured, flavor: .mysql) + #expect(mysql.arguments.contains("--ssl-mode=REQUIRED")) + #expect(!mysql.arguments.contains("--ssl")) + #expect(!mysql.arguments.contains("--skip-ssl")) + } + + /// SSL off is the other half of the same defect: the old code sent `--ssl-mode=DISABLED`, which + /// MariaDB rejects exactly as it rejects the rest. + @Test("An SSL-off connection is disabled in the tool's own spelling") + func sslDisabledFollowsTheResolvedTool() throws { + let maria = try command(.mysql, flavor: .mariadb) + #expect(maria.arguments.contains("--skip-ssl")) + #expect(!maria.arguments.contains { $0.hasPrefix("--ssl-mode") }) + + let mysql = try command(.mysql, flavor: .mysql) + #expect(mysql.arguments.contains("--ssl-mode=DISABLED")) + } + + /// `--ssl-cert` implies `--ssl` on MariaDB, so a connection whose SSL is off must not carry the + /// certificate the form still holds. + @Test("Certificate paths reach the tool, and only while SSL is on") + func certificatePathsFollowTheMode() throws { + var secured = connection(type: .mysql, sslMode: .verifyCa, sslEnabled: true) + secured.sslConfig.caCertificatePath = "/certs/ca.pem" + secured.sslConfig.clientCertificatePath = "/certs/client.pem" + secured.sslConfig.clientKeyPath = "/certs/client.key" + let enabled = try command(.mysql, connection: secured, flavor: .mysql) + #expect(enabled.arguments.contains("--ssl-ca=/certs/ca.pem")) + #expect(enabled.arguments.contains("--ssl-cert=/certs/client.pem")) + #expect(enabled.arguments.contains("--ssl-key=/certs/client.key")) + + var off = secured + off.sslConfig.mode = .disabled + let disabled = try command(.mysql, connection: off, flavor: .mariadb) + #expect(!disabled.arguments.contains { $0.hasPrefix("--ssl-ca") }) + #expect(!disabled.arguments.contains { $0.hasPrefix("--ssl-cert") }) + #expect(!disabled.arguments.contains { $0.hasPrefix("--ssl-key") }) + } + + /// A tool that answered nothing is not guessed at once the connection asks for encryption: + /// MariaDB accepts `--loose-ssl-mode=REQUIRED` and ignores it, which is a cleartext dump. + @Test("An unidentified client refuses an encrypted connection rather than guessing") + func unidentifiedToolRefusesEncryptedModes() throws { + for mode in [SSLMode.required, .verifyCa, .verifyIdentity] { + let secured = connection(type: .mysql, sslMode: mode, sslEnabled: true) + #expect(throws: NativeDumpError.self) { + try command(.mysql, connection: secured, flavor: .unidentified) + } + } + let preferred = connection(type: .mysql, sslMode: .preferred, sslEnabled: true) + let built = try command(.mysql, connection: preferred, flavor: .unidentified) + #expect(!built.arguments.contains { $0.hasPrefix("--ssl") }) + } + + /// `mysqldump` 8.0 reads a table no MariaDB server and no MySQL before 8.0 has, and exits 2 + /// after writing part of the file. The flag that skips it is MySQL's own and backup only. + @Test("A MySQL 8 dump tool skips column statistics on a server that has none") + func columnStatisticsFlagFollowsTheServer() throws { + let mysql8 = "mysqldump Ver 8.4.11 for macos26.6 on arm64 (Homebrew)" + let againstMariaDB = try command( + .mysql, flavor: .mysql, toolVersionText: mysql8, serverVersion: "12.3.3-MariaDB" + ) + #expect(againstMariaDB.arguments.contains("--skip-column-statistics")) + + let againstMySQL8 = try command( + .mysql, flavor: .mysql, toolVersionText: mysql8, serverVersion: "8.4.11" + ) + #expect(!againstMySQL8.arguments.contains("--skip-column-statistics")) + + let restore = try command( + .mysql, kind: .restore, flavor: .mysql, toolVersionText: mysql8, serverVersion: "12.3.3-MariaDB" + ) + #expect(!restore.arguments.contains("--skip-column-statistics")) + + let mariaTool = try command( + .mysql, + flavor: .mariadb, + toolVersionText: "mysqldump from 12.3.3-MariaDB, client 10.20 for osx10.21 (arm64)", + serverVersion: "12.3.3-MariaDB" + ) + #expect(!mariaTool.arguments.contains("--skip-column-statistics")) + } + + /// `my_getopt` parses past a positional argument, so a database named `--no-data` was read as + /// the option: measured on MySQL 8.4.11 and MariaDB 12.3.3, that dumped a different database + /// with no rows and exited 0, which the result sheet reports as a successful backup. + @Test("The option terminator comes before the database, in both directions", arguments: [ + NativeDumpKind.backup, .restore + ]) + func optionTerminatorPrecedesTheDatabase(kind: NativeDumpKind) throws { + let built = try command(.mysql, kind: kind, flavor: .mysql) + let terminator = try #require(built.arguments.firstIndex(of: "--")) + let database = try #require(built.arguments.firstIndex(of: "sales")) + #expect(terminator < database) + #expect(built.arguments.filter { $0 == "--" }.count == 1) + } + + @Test("A narrowed dump keeps its tables behind the same terminator") + func narrowedDumpKeepsOneTerminator() throws { + let scope = NativeDumpScope.objects([ + NativeDumpObject(name: "orders"), NativeDumpObject(name: "customers") + ]) + let built = try command(.mysql, scope: scope, flavor: .mysql) + let terminator = try #require(built.arguments.firstIndex(of: "--")) + #expect(Array(built.arguments[terminator...]) == ["--", "sales", "orders", "customers"]) + } + + /// libpq falls back to `~/.postgresql/root.crt` when no root certificate is named, so a + /// Verify CA connection that opens in the app could never be dumped: measured with pg_dump + /// 17.11, it fails with `root certificate file "..." does not exist`. + @Test("PostgreSQL sends the whole SSL configuration, not just the mode") + func postgresSendsCertificatePaths() throws { + var secured = connection(type: .postgresql, sslMode: .verifyCa, sslEnabled: true) + secured.sslConfig.caCertificatePath = "/certs/ca.pem" + secured.sslConfig.clientCertificatePath = "/certs/client.pem" + secured.sslConfig.clientKeyPath = "/certs/client.key" + let built = try command(.postgresql, connection: secured) + #expect(built.environment["PGSSLMODE"] == "verify-ca") + #expect(built.environment["PGSSLROOTCERT"] == "/certs/ca.pem") + #expect(built.environment["PGSSLCERT"] == "/certs/client.pem") + #expect(built.environment["PGSSLKEY"] == "/certs/client.key") + + var off = secured + off.sslConfig.mode = .disabled + let disabled = try command(.postgresql, connection: off) + #expect(disabled.environment["PGSSLMODE"] == nil) + #expect(disabled.environment["PGSSLROOTCERT"] == nil) + #expect(disabled.environment["PGSSLCERT"] == nil) + #expect(disabled.environment["PGSSLKEY"] == nil) + } + + /// The CA belongs to the modes that verify one, which is the rule the live connection already + /// follows in `LibPQConnectionString`. + @Test("A required connection sends its client certificate but not a CA it does not check") + func postgresRequiredKeepsTheClientCertificate() throws { + var secured = connection(type: .postgresql, sslMode: .required, sslEnabled: true) + secured.sslConfig.caCertificatePath = "/certs/ca.pem" + secured.sslConfig.clientCertificatePath = "/certs/client.pem" + let built = try command(.postgresql, connection: secured) + #expect(built.environment["PGSSLMODE"] == "require") + #expect(built.environment["PGSSLROOTCERT"] == nil) + #expect(built.environment["PGSSLCERT"] == "/certs/client.pem") } /// MariaDB 11.0 renamed every client and some builds ship no `mysql`-prefixed symlink, so both diff --git a/TableProTests/Database/NativeDumpScopeTests.swift b/TableProTests/Database/NativeDumpScopeTests.swift index f09d02319a..0dd570e0b3 100644 --- a/TableProTests/Database/NativeDumpScopeTests.swift +++ b/TableProTests/Database/NativeDumpScopeTests.swift @@ -11,7 +11,6 @@ import Testing @Suite("Native dump object scope") struct NativeDumpScopeTests { - private func connection(type: DatabaseType, database: String = "sales") -> DatabaseConnection { DatabaseConnection( name: "Test", @@ -41,7 +40,11 @@ struct NativeDumpScopeTests { scope: scope, localFilePath: localFilePath ) - return tool.arguments(for: kind, request: request) + return try tool.arguments( + for: kind, + request: request, + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/tool", flavor: .mysql) + ) } // MARK: - PostgreSQL @@ -121,32 +124,39 @@ struct NativeDumpScopeTests { .mysql, scope: .objects([NativeDumpObject(name: "orders"), NativeDumpObject(name: "line_items")]) ) - let databaseIndex = try #require(narrowed.firstIndex(of: "sales")) - #expect(Array(narrowed[databaseIndex...]) == ["sales", "--", "orders", "line_items"]) + let separator = try #require(narrowed.firstIndex(of: "--")) + #expect(Array(narrowed[separator...]) == ["--", "sales", "orders", "line_items"]) #expect(!narrowed.contains("--wildcards")) } - /// `my_getopt` does not stop parsing options at the first positional argument. Measured with - /// mysqldump 12.3.2: a table named `--no-data` passed bare was read as the option and the dump - /// came back with zero rows at exit 0, which the result sheet reports as a success. `--` in - /// front makes it a table name again. - @Test("A hostile MySQL table name cannot become an option") - func mysqlSeparatesItsTableList() throws { + /// `my_getopt` does not stop parsing options at the first positional argument, so the database + /// is as exposed as the tables. Measured with mysqldump 8.4.11 and 12.3.2: a database named + /// `--no-data` passed bare was read as the option, and a narrowed dump of it wrote the *first + /// table name* as the database, with no rows and exit 0, which the result sheet reports as a + /// success. `--` in front of the database makes every name after it a name again. + @Test("A hostile MySQL name cannot become an option, database or table") + func mysqlSeparatesEveryName() throws { let narrowed = try arguments( .mysql, scope: .objects([ NativeDumpObject(name: "orders"), NativeDumpObject(name: "--no-data") - ]) + ]), + database: "--skip-lock-tables" ) let separator = try #require(narrowed.firstIndex(of: "--")) - let hostile = try #require(narrowed.firstIndex(of: "--no-data")) - #expect(separator < hostile, "every table name must sit after the end-of-options marker") + let hostileDatabase = try #require(narrowed.firstIndex(of: "--skip-lock-tables")) + let hostileTable = try #require(narrowed.firstIndex(of: "--no-data")) + #expect(separator < hostileDatabase) + #expect(separator < hostileTable) } - @Test("A whole-database MySQL dump passes no separator") - func mysqlWholeDatabaseHasNoSeparator() throws { - #expect(!(try arguments(.mysql, scope: .wholeDatabase)).contains("--")) + /// The terminator is not a table-list marker, so a whole-database dump carries it too. + @Test("A whole-database MySQL dump keeps the separator in front of the database") + func mysqlWholeDatabaseKeepsTheSeparator() throws { + let whole = try arguments(.mysql, scope: .wholeDatabase) + let separator = try #require(whole.firstIndex(of: "--")) + #expect(Array(whole[separator...]) == ["--", "sales"]) } // MARK: - MongoDB @@ -274,7 +284,6 @@ struct NativeDumpScopeTests { @Suite("DuckDB in-engine dump statements") struct DuckDBDumpStatementTests { - private func connection() -> DatabaseConnection { var connection = DatabaseConnection( name: "Local", diff --git a/TableProTests/Database/NativeDumpServiceTests.swift b/TableProTests/Database/NativeDumpServiceTests.swift index ef7b122fa4..647297ae6e 100644 --- a/TableProTests/Database/NativeDumpServiceTests.swift +++ b/TableProTests/Database/NativeDumpServiceTests.swift @@ -58,7 +58,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/sales.dump"), @@ -81,7 +81,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .restore, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_restore"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_restore"), request: request( connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/sales.dump"), @@ -103,13 +103,13 @@ struct NativeDumpServiceCommandTests { let restore = try NativeDumpService.buildCommand( kind: .restore, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_restore"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_restore"), request: request(connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/sales.dump")) ) let backup = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request(connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/sales.dump")) ) #expect(restore.exitPolicy == .toleratesUnrecognizedSessionSettings) @@ -143,7 +143,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: kind, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_restore"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_restore"), request: request(connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/sales.dump")) ) #expect(command.environment["LC_MESSAGES"] == "C") @@ -189,7 +189,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(host: ""), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -204,7 +204,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(username: ""), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -219,7 +219,7 @@ struct NativeDumpServiceCommandTests { let nilPw = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -229,7 +229,7 @@ struct NativeDumpServiceCommandTests { let emptyPw = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -254,7 +254,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(sslMode: mode), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -269,7 +269,7 @@ struct NativeDumpServiceCommandTests { let command = try NativeDumpService.buildCommand( kind: .backup, tool: postgresTool, - executable: URL(fileURLWithPath: "/usr/bin/pg_dump"), + resolved: NativeDumpResolvedTool(name: "tool", path: "/usr/bin/pg_dump"), request: request( connection: connection(sslMode: .required), fileURL: URL(fileURLWithPath: "/tmp/x.dump"), @@ -447,6 +447,32 @@ struct NativeDumpServiceStateMachineTests { }) } + /// `start` finds the binary and asks it what it is before there is anything to cancel, and both + /// steps spawn a process. A Cancel taken in that window used to be dropped, and on a restore + /// the tool then launched and wrote to the database the user had just said to leave alone. + @Test("A cancel taken before the tool runs stops it from running at all") + func cancelBeforeRunNeverStartsTheTool() async throws { + let runner = FakeDumpRunner() + let service = service(kind: .restore, runner: runner) + let updates = service.stateUpdates() + + service.cancel() + try service.run( + job: fakeJob(), + database: "sales", + fileURL: URL(fileURLWithPath: "/tmp/test-cancel-before-run.dump") + ) + + #expect(runner.startCount == 0, "the tool must not launch after a cancel") + let finalState = try await firstMatching(updates) { + switch $0 { + case .cancelled, .running, .finished, .failed: return true + default: return false + } + } + #expect(finalState == .cancelled) + } + @Test("successful run transitions idle -> running -> finished") func successfulBackup() async throws { let runner = FakeDumpRunner() diff --git a/TableProTests/Database/ServerSideExportTests.swift b/TableProTests/Database/ServerSideExportTests.swift index 3408269448..bac2b6eae9 100644 --- a/TableProTests/Database/ServerSideExportTests.swift +++ b/TableProTests/Database/ServerSideExportTests.swift @@ -11,7 +11,6 @@ import Testing @Suite("Server-side export") struct ServerSideExportTests { - private func statement( _ type: DatabaseType, destination: ServerSideExport.Destination, @@ -210,7 +209,6 @@ struct ServerSideExportTests { @Suite("SQL Server dump") struct SQLServerDumpTests { - private func command(kind: NativeDumpKind, username: String = "sa") throws -> NativeDumpCommand { var sslConfig = SSLConfiguration() sslConfig.mode = .disabled @@ -223,7 +221,7 @@ struct SQLServerDumpTests { return try NativeDumpService.buildCommand( kind: kind, tool: tool, - executable: URL(fileURLWithPath: "/usr/local/bin/sqlpackage"), + resolved: NativeDumpResolvedTool(name: "sqlpackage", path: "/usr/local/bin/sqlpackage"), request: NativeDumpDescriptor.Request( connection: connection, database: "sales", diff --git a/docs/features/backup-restore.mdx b/docs/features/backup-restore.mdx index 6d7378ee12..c1eba84bcb 100644 --- a/docs/features/backup-restore.mdx +++ b/docs/features/backup-restore.mdx @@ -104,7 +104,11 @@ DuckDB writes one of two things. A `.duckdb` file carries tables, views, indexes ## SSH tunnels and SSL -Both flows reuse the connection's active SSH tunnel, with no second port forward. SSL mode reaches PostgreSQL through `PGSSLMODE` and MySQL through `--ssl-mode`, `verify-ca` and `verify-full` included. SQLite and DuckDB open a file, so neither applies. +Both flows reuse the connection's active SSH tunnel, with no second port forward. The whole of the connection's SSL configuration reaches the tool, the CA certificate and the client certificate and key included: PostgreSQL takes them in `PGSSLMODE`, `PGSSLROOTCERT`, `PGSSLCERT` and `PGSSLKEY`, and MySQL and MariaDB in their client's own flags. SQLite and DuckDB open a file, so neither applies. + +MySQL's clients and MariaDB's no longer take the same flags. MySQL's take `--ssl-mode`, with `VERIFY_CA` and `VERIFY_IDENTITY` included. MariaDB's take `--ssl`, `--skip-ssl` and `--ssl-verify-server-cert`, and reject `--ssl-mode`. Which family a tool belongs to comes from its `--version` rather than its name: Homebrew's `mariadb` formula installs MariaDB's dump tool as `mysqldump`. A tool that answers no version is used for **Disabled** and **Preferred**, and refused for **Required**, **Verify CA** and **Verify Identity**. + +MariaDB's clients have no flag for encrypting without verifying, so **Required** reaches them as `--ssl --ssl-verify-server-cert`. That refuses a server offering no TLS. With no CA certificate set it checks the encryption rather than the server's identity. ## Server-side export @@ -138,6 +142,7 @@ A non-zero exit shows the last 64 KB of the tool's stderr in a scrollable monosp |---|---| | *"… was not found on this system"* followed by an install command | Run that command so the binaries land on `PATH`. The message names the tool and the package for your engine | | An authentication failure | The password goes through the environment or a config file rather than a prompt, so this is the account or the database. Check that the role can log in | +| *"could not tell whether … is MySQL's client or MariaDB's"* | The tool on `PATH` did not answer `--version`, and the two families take different SSL flags. Reinstall `mysql-client` or `mariadb` | | Objects that conflict with the dump | Restore into a fresh database, or drop the conflicting objects first | | *"a single transaction can only write to a single attached database"* | A DuckDB query tab has an open transaction that has already written. Commit or roll it back, then run the backup again | diff --git a/scripts/check-mysql-dump-tool-flags.sh b/scripts/check-mysql-dump-tool-flags.sh new file mode 100755 index 0000000000..2f4ac550a8 --- /dev/null +++ b/scripts/check-mysql-dump-tool-flags.sh @@ -0,0 +1,176 @@ +#!/usr/bin/env bash +# +# Check the dump subsystem's MySQL and MariaDB flag tables against the client tools on this Mac. +# +# MySQL and MariaDB forked their client option surfaces and neither accepts the other's: MariaDB +# answers --ssl-mode with "unknown variable" and exit 7, MySQL 8.4 answers --ssl with "unknown +# option" and exit 2. MySQLClientArguments.swift holds the mapping by hand and nothing at runtime +# checks it, which is how #3046 shipped. This hands every flag it names to a real tool of that +# flavor and fails on one the tool does not know. +# +# No server is needed. A tool rejects an unknown option before it opens a socket, so a closed port +# tells the two apart: a flag it understands fails with "Can't connect", one it does not fails with +# "unknown variable" or "unknown option". +# +# Usage: +# scripts/check-mysql-dump-tool-flags.sh +# +# Needs at least one mysql-family client on PATH or in a Homebrew prefix. Skips (exit 3) when +# neither flavor can be found, so it is safe to run anywhere. Exits non-zero on a disagreement. + +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SOURCE="$ROOT/TablePro/Core/Database/MySQLClientArguments.swift" +CLOSED_PORT=59999 + +[ -f "$SOURCE" ] || { + echo "not found: $SOURCE" >&2 + exit 3 +} + +# The table this script holds the tools to. Every flag here must also appear in the Swift file, and +# every SSL flag in the Swift file must appear here, so neither can drift alone. +MYSQL_FLAGS=( + "--ssl-mode=DISABLED" + "--ssl-mode=PREFERRED" + "--ssl-mode=REQUIRED" + "--ssl-mode=VERIFY_CA" + "--ssl-mode=VERIFY_IDENTITY" + "--ssl-ca=/dev/null" + "--ssl-cert=/dev/null" + "--ssl-key=/dev/null" + "--skip-column-statistics" +) +MARIADB_FLAGS=( + "--skip-ssl" + "--ssl" + "--ssl-verify-server-cert" + "--skip-ssl-verify-server-cert" + "--ssl-ca=/dev/null" + "--ssl-cert=/dev/null" + "--ssl-key=/dev/null" +) + +failures=0 + +fail() { + echo "FAIL: $1" >&2 + failures=$((failures + 1)) +} + +# The exact flags the Swift file emits, from its code rather than from its prose: comment lines are +# dropped first, so the explanatory table in the doc comment cannot vouch for a literal the code no +# longer contains. A path built by interpolation keeps its `=` and loses the expression, which is +# what the checked flags normalize to as well. +source_flags() { + grep -vE '^[[:space:]]*//' "$SOURCE" \ + | grep -o '"--[^"]*"' \ + | tr -d '"' \ + | sed -E 's/=\\\(.*/=/' \ + | sort -u +} + +checked_flags() { + printf '%s\n' "${MYSQL_FLAGS[@]}" "${MARIADB_FLAGS[@]}" | sed -E 's|=/dev/null|=|' | sort -u +} + +# Both directions, so neither a flag the code dropped nor one it gained goes unchecked. +check_source_agrees() { + local flag + while read -r flag; do + [ -n "$flag" ] || continue + fail "$flag is checked here but no longer emitted by MySQLClientArguments.swift" + done < <(comm -23 <(checked_flags) <(source_flags)) + while read -r flag; do + [ -n "$flag" ] || continue + fail "$flag is emitted by MySQLClientArguments.swift but not checked here" + done < <(comm -13 <(checked_flags) <(source_flags)) +} + +flavor_of() { + local banner + banner="$("$1" --version 2> /dev/null)" + [ -n "$banner" ] || return 1 + case "$banner" in + *MariaDB* | *mariadb*) echo "mariadb" ;; + *) echo "mysql" ;; + esac +} + +# A tool that rejects the flag says so before connecting, so the connection failure is the pass. +check_tool() { + local tool=$1 flavor=$2 flag output + local -a flags + if [ "$flavor" = "mariadb" ]; then + flags=("${MARIADB_FLAGS[@]}") + else + flags=("${MYSQL_FLAGS[@]}") + fi + echo "checking $flavor tool $tool" + for flag in "${flags[@]}"; do + output="$("$tool" --protocol=TCP -h 127.0.0.1 -P "$CLOSED_PORT" -u probe "$flag" nodb 2>&1)" + case "$output" in + *"unknown variable"* | *"unknown option"* | *"Unknown option"*) + fail "$tool ($flavor) rejects $flag: $(echo "$output" | head -1)" + ;; + *) + echo " ok $flag" + ;; + esac + done +} + +candidates() { + local name + for name in mysqldump mariadb-dump; do + command -v "$name" 2> /dev/null + done + ls -1 /opt/homebrew/opt/*/bin/mysqldump /usr/local/opt/*/bin/mysqldump /usr/local/mysql/bin/mysqldump 2> /dev/null +} + +check_source_agrees + +declare -a seen_flavors=() +while read -r tool; do + [ -x "$tool" ] || continue + flavor="$(flavor_of "$tool")" || continue + case " ${seen_flavors[*]:-} " in + *" $flavor "*) continue ;; + esac + seen_flavors+=("$flavor") + check_tool "$tool" "$flavor" +done < <(candidates | sort -u) + +if [ ${#seen_flavors[@]} -eq 0 ]; then + echo "no mysql-family client found; install mysql-client or mariadb to run this check" >&2 + exit 3 +fi + +# A MySQL 8 tool is the only one that reads information_schema.COLUMN_STATISTICS, and MariaDB's own +# tool does not know the flag that skips it, so the mapping is only correct while that stays true. +case " ${seen_flavors[*]} " in + *" mariadb "*) + maria="$(candidates | sort -u | while read -r t; do + [ -x "$t" ] && [ "$(flavor_of "$t")" = "mariadb" ] && echo "$t" && break + done)" + if [ -n "$maria" ]; then + # MariaDB's tools report an unknown option and still exit 0 for --version, so the + # message is the signal rather than the exit code. + case "$("$maria" --skip-column-statistics --version 2>&1)" in + *"unknown option"* | *"unknown variable"*) + echo " ok --skip-column-statistics is still MySQL-only" + ;; + *) + fail "MariaDB's tool now accepts --skip-column-statistics; the compatibility rule needs revisiting" + ;; + esac + fi + ;; +esac + +if [ "$failures" -gt 0 ]; then + echo "$failures disagreement(s) between the flag tables and the installed tools" >&2 + exit 1 +fi +echo "flag tables agree with the installed tools" From d8f910bb02be6958eb8d1353842001f9fe7b81d0 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 22 Sep 2026 14:49:21 +0700 Subject: [PATCH 2/2] fix(export): report each database's backup outcome as its own row --- CHANGELOG.md | 3 + .../Database/ProcessNativeDumpRunner.swift | 72 +++++++- TablePro/Views/Backup/BackupOutcomeRow.swift | 64 +++++++ TablePro/Views/Backup/BackupResultSheet.swift | 161 +++++++++++++++--- .../ProcessNativeDumpRunnerTests.swift | 71 ++++++++ .../Views/Backup/BackupOutcomeRowTests.swift | 88 ++++++++++ 6 files changed, 423 insertions(+), 36 deletions(-) create mode 100644 TablePro/Views/Backup/BackupOutcomeRow.swift create mode 100644 TableProTests/Database/ProcessNativeDumpRunnerTests.swift create mode 100644 TableProTests/Views/Backup/BackupOutcomeRowTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index edb68eca45..37325e5b10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -296,6 +296,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Partial MySQL dump of a MariaDB or MySQL 5.7 server, from the column statistics `mysqldump` 8 reads. - A database whose name starts with a dash backed up as a different database, reported as a success. - Cancel ignored while TablePro was locating the backup tool, and the dump running anyway. +- Destination folder and the first database reading as one path in the backup result sheet. (#3046) +- Only the last line of a failed backup's error shown, which on `pg_dump` is the hint rather than the cause. +- Backup failure reported as an exit code alone when the tool wrote its message and exited at once. ### Security diff --git a/TablePro/Core/Database/ProcessNativeDumpRunner.swift b/TablePro/Core/Database/ProcessNativeDumpRunner.swift index c07a740cf0..6bba657d66 100644 --- a/TablePro/Core/Database/ProcessNativeDumpRunner.swift +++ b/TablePro/Core/Database/ProcessNativeDumpRunner.swift @@ -11,6 +11,10 @@ final class ProcessNativeDumpRunner: NativeDumpRunner, @unchecked Sendable { private let process = Process() private let stderrPipe = Pipe() private let stateLock = NSLock() + /// Held across the read as well as the append, so a chunk can never be taken out of the pipe by + /// one reader and still be missing from the buffer when another snapshots it. Separate from + /// `stateLock`, which `cancel()` takes and which must never wait on a pipe. + private let stderrLock = NSLock() private var stderrBuffer = Data() private var wasCancelled = false private var terminationResult: NativeDumpRunResult? @@ -34,24 +38,25 @@ final class ProcessNativeDumpRunner: NativeDumpRunner, @unchecked Sendable { try attachRedirection(for: command) stderrPipe.fileHandleForReading.readabilityHandler = { [weak self] handle in + guard let self else { return } + self.stderrLock.lock() let chunk = handle.availableData - guard !chunk.isEmpty, let self else { return } - self.stateLock.lock() - self.stderrBuffer.append(chunk) - if self.stderrBuffer.count > stderrCap { - self.stderrBuffer = Data(self.stderrBuffer.suffix(stderrCap)) - } - self.stateLock.unlock() + self.append(chunk, cap: stderrCap) + self.stderrLock.unlock() } process.terminationHandler = { [weak self] proc in guard let self else { return } self.stderrPipe.fileHandleForReading.readabilityHandler = nil + self.drainStderr(cap: stderrCap) self.releaseRedirection() - self.stateLock.lock() + self.stderrLock.lock() let stderrText = String(data: self.stderrBuffer, encoding: .utf8)? .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + self.stderrLock.unlock() + + self.stateLock.lock() let result = NativeDumpRunResult( exitCode: proc.terminationStatus, stderr: stderrText, @@ -77,6 +82,57 @@ final class ProcessNativeDumpRunner: NativeDumpRunner, @unchecked Sendable { } } + /// Takes whatever the pipe still holds once the process has gone. + /// + /// The readability source and the process reaper run on independent queues, so bytes written + /// just before the child exits can still be in the pipe when `terminationHandler` reads the + /// buffer, and a tool that exits on its first argument writes everything it has to say in that + /// window. Measured with a harness mirroring this class against a child that writes 66 bytes + /// and exits immediately: 2 of 300 runs captured nothing at all, and with this drain 0 of 300 + /// did. What the user saw instead was "Process exited with code 7" and no message. + /// Takes whatever the pipe still holds once the process has gone. + /// + /// The readability source and the process reaper run on independent queues, so a tool that + /// exits on its first argument can have written everything it has to say and still be waiting + /// to be read. Measured with a harness mirroring this class against a child that writes 66 + /// bytes and exits at once: between 1 and 6 of every 300 runs captured nothing at all, the rate + /// rising with load, and 0 of 1,200 with the drain and the lock above. What the sheet showed + /// instead was the exit code alone. + /// + /// The child has exited, so everything it wrote is already in the pipe's buffer and a + /// non-blocking read takes all of it. `readDataToEndOfFile` would take it too and then wait for + /// every writer to close, which a grandchild that inherited this end would never do, hanging + /// the termination handler and with it the run. The read stops at the cap for the same reason: + /// a grandchild still writing would otherwise keep the loop fed for as long as it cared to, and + /// nothing downstream, including the credentials file's removal, happens until it returns. + private func drainStderr(cap: Int) { + let descriptor = stderrPipe.fileHandleForReading.fileDescriptor + let flags = fcntl(descriptor, F_GETFL) + guard flags != -1, fcntl(descriptor, F_SETFL, flags | O_NONBLOCK) != -1 else { return } + + var buffer = [UInt8](repeating: 0, count: 4_096) + var taken = 0 + stderrLock.lock() + while taken < cap { + let received = buffer.withUnsafeMutableBytes { raw in + read(descriptor, raw.baseAddress, raw.count) + } + guard received > 0 else { break } + taken += received + append(Data(buffer[0 ..< received]), cap: cap) + } + stderrLock.unlock() + } + + /// Call with `stderrLock` held. + private func append(_ chunk: Data, cap: Int) { + guard !chunk.isEmpty else { return } + stderrBuffer.append(chunk) + if stderrBuffer.count > cap { + stderrBuffer = Data(stderrBuffer.suffix(cap)) + } + } + /// A tool that writes to standard output gets the destination file as its stdout, and a restore /// that reads from standard input gets the dump file as its stdin. The one that manages its own /// file gets the null device, which is what keeps a chatty tool from filling a pipe nobody diff --git a/TablePro/Views/Backup/BackupOutcomeRow.swift b/TablePro/Views/Backup/BackupOutcomeRow.swift new file mode 100644 index 0000000000..e55100141e --- /dev/null +++ b/TablePro/Views/Backup/BackupOutcomeRow.swift @@ -0,0 +1,64 @@ +// +// BackupOutcomeRow.swift +// TablePro +// + +import Foundation + +/// One database's line in the backup result sheet. +/// +/// A row rather than a line of text, because the sheet used to join the destination folder and one +/// sentence per database into a single monospaced block: a folder ending in `New/` above a database +/// called `Music` read as the one path `/Users/Nick/Music/New/Music` (#3046). +struct BackupOutcomeRow: Identifiable, Equatable { + enum State: Equatable { + case succeeded + case failed + case cancelled + } + + let id: String + let database: String + let state: State + /// The size for a database that was written, and the file's name under it. + let fileName: String + let size: String? + /// Everything the tool said, kept whole. The last line alone is not the diagnosis: `pg_dump` + /// ends a refused connection with "Is the server running on that host…" and a permission + /// failure with "detail: Query was: LOCK TABLE …", and the cause is the line above. + let errorDetail: String? + + static func rows(for outcomes: [NativeDumpBatchOutcome]) -> [BackupOutcomeRow] { + outcomes.map { outcome in + switch outcome.result { + case .succeeded(let bytes): + return BackupOutcomeRow( + id: outcome.destination.path, + database: outcome.database, + state: .succeeded, + fileName: outcome.destination.lastPathComponent, + size: ByteCountFormatter.string(fromByteCount: bytes, countStyle: .file), + errorDetail: nil + ) + case .failed(let message): + return BackupOutcomeRow( + id: outcome.destination.path, + database: outcome.database, + state: .failed, + fileName: outcome.destination.lastPathComponent, + size: nil, + errorDetail: message.trimmingCharacters(in: .whitespacesAndNewlines) + ) + case .cancelled: + return BackupOutcomeRow( + id: outcome.destination.path, + database: outcome.database, + state: .cancelled, + fileName: outcome.destination.lastPathComponent, + size: nil, + errorDetail: nil + ) + } + } + } +} diff --git a/TablePro/Views/Backup/BackupResultSheet.swift b/TablePro/Views/Backup/BackupResultSheet.swift index 6f91e71141..8a70bfb225 100644 --- a/TablePro/Views/Backup/BackupResultSheet.swift +++ b/TablePro/Views/Backup/BackupResultSheet.swift @@ -83,7 +83,7 @@ struct BackupResultSheet: View { } scrollingDetail(message) case .batch(let outcomes, let directory): - scrollingDetail(Self.batchDetail(outcomes, directory: directory)) + batchDetailView(outcomes, directory: directory) case .restoreSuccess(_, _, let skippedSettings): summaryDetail if let note = Self.skippedSettingsNote(skippedSettings) { @@ -113,6 +113,107 @@ struct BackupResultSheet: View { } } + /// The folder is a labelled row and each database is its own row, because one text block made + /// the folder and the first database read as a single path (#3046). + private func batchDetailView(_ outcomes: [NativeDumpBatchOutcome], directory: URL) -> some View { + VStack(alignment: .leading, spacing: 10) { + LabeledContent { + Text(directory.path(percentEncoded: false)) + .lineLimit(1) + .truncationMode(.middle) + .textSelection(.enabled) + } label: { + Text("Destination") + } + .font(.callout) + + ScrollView { + VStack(alignment: .leading, spacing: 8) { + ForEach(BackupOutcomeRow.rows(for: outcomes)) { row in + outcomeRow(row) + } + } + .frame(maxWidth: .infinity, alignment: .leading) + .padding(8) + } + .frame(maxWidth: .infinity) + .frame(maxHeight: 200) + .background(Color(nsColor: .textBackgroundColor)) + .clipShape(RoundedRectangle(cornerRadius: 6)) + .overlay( + RoundedRectangle(cornerRadius: 6) + .stroke(Color(nsColor: .separatorColor), lineWidth: 1) + ) + } + } + + private func outcomeRow(_ row: BackupOutcomeRow) -> some View { + VStack(alignment: .leading, spacing: 2) { + HStack(spacing: 6) { + Image(systemName: Self.symbolName(for: row.state)) + .foregroundStyle(Self.tint(for: row.state)) + .accessibilityLabel(Self.stateAccessibilityLabel(for: row.state)) + Text(row.database) + .font(.callout) + .lineLimit(1) + .truncationMode(.middle) + .textSelection(.enabled) + Spacer(minLength: 8) + Text(Self.stateLabel(for: row)) + .font(.caption) + .foregroundStyle(.secondary) + } + if let errorDetail = row.errorDetail { + RevealedTextView(errorDetail) + .font(.system(.caption, design: .monospaced)) + .foregroundStyle(.secondary) + .textSelection(.enabled) + .fixedSize(horizontal: false, vertical: true) + } else if row.state == .succeeded { + Text(row.fileName) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + .truncationMode(.middle) + .textSelection(.enabled) + } + } + } + + private static func symbolName(for state: BackupOutcomeRow.State) -> String { + switch state { + case .succeeded: return "checkmark.circle.fill" + case .failed: return "xmark.circle.fill" + case .cancelled: return "slash.circle" + } + } + + private static func tint(for state: BackupOutcomeRow.State) -> Color { + switch state { + case .succeeded: return .green + case .failed: return .red + case .cancelled: return .secondary + } + } + + /// A successful row's trailing text is its size, so without this the state reaches VoiceOver + /// through the symbol's colour alone. + private static func stateAccessibilityLabel(for state: BackupOutcomeRow.State) -> String { + switch state { + case .succeeded: return String(localized: "Backed up") + case .failed: return String(localized: "Failed") + case .cancelled: return String(localized: "Cancelled") + } + } + + internal static func stateLabel(for row: BackupOutcomeRow) -> String { + switch row.state { + case .succeeded: return row.size ?? "" + case .failed: return String(localized: "Failed") + case .cancelled: return String(localized: "Cancelled") + } + } + private func scrollingDetail(_ text: String) -> some View { ScrollView { Text(text) @@ -209,8 +310,8 @@ struct BackupResultSheet: View { case .restore: return Self.partialStateWarning } - case .batch(let outcomes, let directory): - return Self.batchDetail(outcomes, directory: directory) + case .batch: + return nil } } @@ -227,31 +328,6 @@ struct BackupResultSheet: View { settings.formatted(.list(type: .and)) ) } - - /// One line per database, so a run where the second of three failed says which one and keeps - /// the other two visible rather than reporting a single verdict for the batch. - private static func batchDetail(_ outcomes: [NativeDumpBatchOutcome], directory: URL) -> String { - let lines = outcomes.map { outcome -> String in - switch outcome.result { - case .succeeded(let bytes): - return String( - format: String(localized: "%1$@ \u{2192} %2$@ (%3$@)"), - outcome.database, - outcome.destination.lastPathComponent, - ByteCountFormatter.string(fromByteCount: bytes, countStyle: .file) - ) - case .failed(let message): - return String( - format: String(localized: "%1$@ failed: %2$@"), - outcome.database, - message.split(separator: "\n").last.map(String.init) ?? message - ) - case .cancelled: - return String(format: String(localized: "%@ cancelled"), outcome.database) - } - } - return ([directory.path(percentEncoded: false)] + lines).joined(separator: "\n") - } } #Preview("Backup Success") { @@ -293,6 +369,35 @@ struct BackupResultSheet: View { ) } +#Preview("Backup Batch With A Failure") { + BackupResultSheet( + kind: .backup, + outcome: .batch( + outcomes: [ + NativeDumpBatchOutcome( + database: "Music", + destination: URL(fileURLWithPath: "/Users/me/Music/New/Music-2026-09-21-181500.sql"), + result: .failed( + message: "/opt/homebrew/bin/mysqldump: unknown variable 'ssl-mode=PREFERRED'") + ), + NativeDumpBatchOutcome( + database: "production", + destination: URL(fileURLWithPath: "/Users/me/Music/New/production-2026-09-21-181500.sql"), + result: .succeeded(bytes: 4_512_000) + ), + NativeDumpBatchOutcome( + database: "analytics", + destination: URL(fileURLWithPath: "/Users/me/Music/New/analytics-2026-09-21-181500.sql"), + result: .cancelled + ) + ], + directory: URL(fileURLWithPath: "/Users/me/Music/New", isDirectory: true) + ), + onClose: {}, + onShowInFinder: {} + ) +} + #Preview("Restore Failure") { BackupResultSheet( kind: .restore, diff --git a/TableProTests/Database/ProcessNativeDumpRunnerTests.swift b/TableProTests/Database/ProcessNativeDumpRunnerTests.swift new file mode 100644 index 0000000000..6afb8a601c --- /dev/null +++ b/TableProTests/Database/ProcessNativeDumpRunnerTests.swift @@ -0,0 +1,71 @@ +// +// ProcessNativeDumpRunnerTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("Process native dump runner") +struct ProcessNativeDumpRunnerTests { + private func command(_ script: String) -> NativeDumpCommand { + NativeDumpCommand( + executable: URL(fileURLWithPath: "/bin/sh"), + arguments: ["-c", script], + environment: [:], + stderrByteCap: 64_000 + ) + } + + /// A tool that rejects one of its arguments writes its whole complaint and exits at once, and + /// the pipe still held those bytes when the buffer was read. Measured with a harness mirroring + /// this class: 2 of 300 such runs captured nothing, so the sheet said "Process exited with + /// code 7" and named no cause (#3046). + @Test("A tool that exits at once still has everything it said") + func stderrSurvivesAnImmediateExit() async throws { + let message = "/opt/homebrew/bin/mysqldump: unknown variable 'ssl-mode=PREFERRED'" + let runner = ProcessNativeDumpRunner(command: command("printf '%s' \"\(message)\" >&2; exit 7")) + try runner.start() + let result = await runner.result + #expect(result.exitCode == 7) + #expect(result.stderr == message) + #expect(!result.wasCancelled) + } + + @Test("A tool that says nothing reports its exit code alone") + func silentFailure() async throws { + let runner = ProcessNativeDumpRunner(command: command("exit 3")) + try runner.start() + let result = await runner.result + #expect(result.exitCode == 3) + #expect(result.stderr.isEmpty) + } + + /// A tool that leaves a child holding its standard error keeps the pipe open and writable after + /// it has gone. Nothing downstream runs until the drain returns, the temporary credentials file + /// included, so the drain stops at the cap instead of following whatever arrives next. + @Test("Output from a surviving child cannot grow the buffer past its cap", .timeLimit(.minutes(1))) + func outputAfterExitIsBounded() async throws { + let cap = 4_096 + let script = """ + (i=0; while [ $i -lt 400 ]; do printf '%s' \ + 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' >&2; i=$((i+1)); done) & + printf '%s' 'first' >&2 + exit 4 + """ + let runner = ProcessNativeDumpRunner( + command: NativeDumpCommand( + executable: URL(fileURLWithPath: "/bin/sh"), + arguments: ["-c", script], + environment: [:], + stderrByteCap: cap + ) + ) + try runner.start() + let result = await runner.result + #expect(result.exitCode == 4) + #expect(result.stderr.utf8.count <= cap) + } +} diff --git a/TableProTests/Views/Backup/BackupOutcomeRowTests.swift b/TableProTests/Views/Backup/BackupOutcomeRowTests.swift new file mode 100644 index 0000000000..63aaff2b37 --- /dev/null +++ b/TableProTests/Views/Backup/BackupOutcomeRowTests.swift @@ -0,0 +1,88 @@ +// +// BackupOutcomeRowTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("Backup outcome rows") +struct BackupOutcomeRowTests { + private func outcome( + _ database: String, + _ result: NativeDumpBatchOutcome.Result, + directory: String = "/Users/me/Music/New" + ) -> NativeDumpBatchOutcome { + NativeDumpBatchOutcome( + database: database, + destination: URL(fileURLWithPath: "\(directory)/\(database)-2026-09-21-181500.sql"), + result: result + ) + } + + /// The reported case: a folder ending in `New/` above a database called `Music` read as the one + /// path `/Users/Nick/Music/New/Music` once both were text in the same block (#3046). + @Test("A failed database is its own row, never text joined to the folder") + func failedRowStandsAlone() throws { + let rows = BackupOutcomeRow.rows(for: [ + outcome("Music", .failed(message: "/opt/homebrew/bin/mysqldump: unknown variable 'ssl-mode=PREFERRED'")) + ]) + let row = try #require(rows.first) + #expect(rows.count == 1) + #expect(row.database == "Music") + #expect(row.state == .failed) + #expect(row.size == nil) + #expect(row.errorDetail == "/opt/homebrew/bin/mysqldump: unknown variable 'ssl-mode=PREFERRED'") + #expect(!row.database.contains("/")) + } + + /// `pg_dump` ends a refused connection with the generic hint and a permission failure with the + /// query it was running, so keeping the last line alone threw the diagnosis away. + @Test("Every line of the tool's output is kept") + func multilineErrorSurvives() throws { + let stderr = """ + pg_dump: error: connection to server at "127.0.0.1", port 5432 failed: Connection refused + \tIs the server running on that host and accepting TCP/IP connections? + """ + let rows = BackupOutcomeRow.rows(for: [outcome("sales", .failed(message: stderr))]) + let detail = try #require(rows.first?.errorDetail) + #expect(detail.contains("Connection refused")) + #expect(detail.contains("Is the server running")) + } + + @Test("A written database carries its file and size") + func succeededRow() throws { + let rows = BackupOutcomeRow.rows(for: [outcome("production", .succeeded(bytes: 4_512_000))]) + let row = try #require(rows.first) + #expect(row.state == .succeeded) + #expect(row.fileName == "production-2026-09-21-181500.sql") + #expect(row.size != nil) + #expect(row.errorDetail == nil) + } + + @Test("A cancelled database says so and claims no file") + func cancelledRow() throws { + let rows = BackupOutcomeRow.rows(for: [outcome("analytics", .cancelled)]) + let row = try #require(rows.first) + #expect(row.state == .cancelled) + #expect(row.size == nil) + #expect(row.errorDetail == nil) + #expect(BackupResultSheet.stateLabel(for: row) == String(localized: "Cancelled")) + } + + /// A run where one of three failed keeps the other two visible, which is the whole reason the + /// batch reports per database rather than one verdict. + @Test("Every database in the run gets a row, in the order it ran") + func everyDatabaseIsReported() { + let rows = BackupOutcomeRow.rows(for: [ + outcome("Music", .failed(message: "boom")), + outcome("production", .succeeded(bytes: 1_024)), + outcome("analytics", .cancelled) + ]) + #expect(rows.map(\.database) == ["Music", "production", "analytics"]) + #expect(rows.map(\.state) == [.failed, .succeeded, .cancelled]) + #expect(Set(rows.map(\.id)).count == 3) + } +}