From 171b0120249337805ccb5a368c113ce31a400198 Mon Sep 17 00:00:00 2001 From: Andrey Kravchenko Date: Fri, 10 Jan 2025 16:36:24 +0300 Subject: [PATCH] Do not connect to the database if the replication configuration is incorrect Configuration errors can lead to loss of synchronization between master and replica. --- src/jrd/replication/Config.cpp | 138 ++++++++++++++++++++++++++------- 1 file changed, 111 insertions(+), 27 deletions(-) diff --git a/src/jrd/replication/Config.cpp b/src/jrd/replication/Config.cpp index 5a0257f5b4d..2efbba27671 100644 --- a/src/jrd/replication/Config.cpp +++ b/src/jrd/replication/Config.cpp @@ -79,38 +79,53 @@ namespace output = false; } - void configError(const string& type, const string& key, const string& value) + void configError(CheckStatusWrapper* status, const string& type, const string& key, const string& value) { string msg; + if (!(status->getState() & IStatus::STATE_ERRORS)) + { + msg.printf("Incorrect entry in %s", REPLICATION_CFGFILE); + (Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status); + } + msg.printf("%s specifies %s: %s", key.c_str(), type.c_str(), value.c_str()); - raiseError(msg.c_str()); + (Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status); } - void checkAccess(const PathName& path, const string& key) + bool checkAccess(CheckStatusWrapper* status, const PathName& path, const string& key) { if (path.hasData() && !PathUtils::canAccess(path, 6)) - configError("missing or inaccessible directory", key, path.c_str()); + { + configError(status, "missing or inaccessible directory", key, path.c_str()); + return false; + } + return true; } void composeError(CheckStatusWrapper* status, const Exception& ex) { - string prefix; - prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE); - Arg::StatusVector sv; - sv << Arg::Gds(isc_random) << Arg::Str(prefix); + + if (!(status->getState() & IStatus::STATE_ERRORS)) + { + string prefix; + prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE); + sv << Arg::Gds(isc_random) << Arg::Str(prefix); + } + + sv << Arg::StatusVector(status); sv << Arg::StatusVector(ex); status->setErrors(sv.value()); } - void parseExternalValue(const string& key, const string& value, string& output) + bool parseExternalValue(CheckStatusWrapper* status, const string& key, const string& value, string& output) { const auto pos = key.rfind('_'); if (pos == string::npos) { output = value.c_str(); - return; + return true; } string temp; @@ -120,7 +135,10 @@ namespace { fb_utils::readenv(value.c_str(), temp); if (temp.isEmpty()) - configError("missing environment variable", key, value); + { + configError(status, "missing environment variable", key, value); + return false; + } } else if (key_source.equals(KEY_SUFFIX_FILE)) { @@ -131,7 +149,10 @@ namespace AutoPtr file(os_utils::fopen(filename.c_str(), "rt")); if (!file) - configError("missing or inaccessible file", key, filename.c_str()); + { + configError(status, "missing or inaccessible file", key, filename.c_str()); + return false; + } // skip first empty lines do @@ -146,37 +167,55 @@ namespace } while (temp.isEmpty()); if (temp.isEmpty()) - configError("empty file", key, filename.c_str()); + { + configError(status, "empty file", key, filename.c_str()); + return false; + } } output = temp.c_str(); + return true; } - void parseSyncReplica(const ConfigFile::Parameters& params, SyncReplica& output) + bool parseSyncReplica(CheckStatusWrapper* status, const ConfigFile::Parameters& params, SyncReplica& output) { + bool result = true; for (const auto& el : params) { const string key(el.name.c_str()); const string value(el.value); if (value.isEmpty()) - continue; + { + configError(status, "empty value", output.database, key); + result = false; + } if (key.find("username") == 0) { if (output.username.hasData()) - configError("multiple values", output.database, "username"); - parseExternalValue(key, value, output.username); + { + configError(status, "multiple values", output.database, "username"); + result = false; + } + result &= parseExternalValue(status, key, value, output.username); } else if (key.find("password") == 0) { if (output.password.hasData()) - configError("multiple values", output.database, "password"); - parseExternalValue(key, value, output.password); + { + configError(status, "multiple values", output.database, "password"); + result = false; + } + result &= parseExternalValue(status, key, value, output.password); } else - configError("unknown parameter", output.database, key); + { + configError(status, "unknown key", output.database, key); + result = false; + } } + return result; } } @@ -253,7 +292,8 @@ Config* Config::get(const PathName& lookupName) AutoPtr config(FB_NEW Config); - bool defaultFound = false, exactMatch = false; + FbLocalStatus localStatus; + bool defaultFound = false, exactMatch = false, replicaSkip = false; for (const auto& section : cfgFile.getParameters()) { @@ -291,7 +331,12 @@ Config* Config::get(const PathName& lookupName) string value(el.value); if (value.isEmpty()) + { + configError(&localStatus, "empty value of key", + exactMatch ? lookupName.c_str() : section.name.c_str(), + key); continue; + } if (key == "sync_replica") { @@ -299,7 +344,11 @@ Config* Config::get(const PathName& lookupName) if (el.sub) { syncReplica.database = value; - parseSyncReplica(el.sub->getParameters(), syncReplica); + if (!parseSyncReplica(&localStatus, el.sub->getParameters(), syncReplica)) + { + replicaSkip = true; + continue; + } } else splitConnectionString(value, syncReplica.database, syncReplica.username, syncReplica.password); @@ -332,7 +381,12 @@ Config* Config::get(const PathName& lookupName) { config->journalDirectory = value.c_str(); PathUtils::ensureSeparator(config->journalDirectory); - checkAccess(config->journalDirectory, key); + if (!checkAccess(&localStatus, config->journalDirectory, key)) + { + config->journalDirectory.erase(); + replicaSkip = true; + continue; + } } else if (key == "journal_file_prefix") { @@ -346,7 +400,11 @@ Config* Config::get(const PathName& lookupName) { config->archiveDirectory = value.c_str(); PathUtils::ensureSeparator(config->archiveDirectory); - checkAccess(config->archiveDirectory, key); + if (!checkAccess(&localStatus, config->archiveDirectory, key)) + { + config->archiveDirectory.erase(); + continue; + } } else if (key == "journal_archive_command") { @@ -376,12 +434,28 @@ Config* Config::get(const PathName& lookupName) { parseBoolean(value, config->cascadeReplication); } + else if ((key != "journal_source_directory") && + (key != "source_guid") && + (key != "verbose_logging") && + (key != "apply_idle_timeout") && + (key != "apply_error_timeout")) + { + configError(&localStatus, "unknown key", + exactMatch ? lookupName.c_str() : section.name.c_str(), + key); + } } if (exactMatch) break; } + if (localStatus->getState() & IStatus::STATE_ERRORS) + logPrimaryStatus(lookupName, &localStatus); + + if (replicaSkip && !config->disableOnError) + raiseError("One or more replicas configured with errors"); + // TODO: As soon as plugin name is moved into RDB$PUBLICATIONS, // delay config parse until real replication start if (config->pluginName.hasData()) @@ -411,6 +485,7 @@ Config* Config::get(const PathName& lookupName) composeError(&localStatus, ex); logPrimaryStatus(lookupName, &localStatus); + localStatus.raise(); } return nullptr; @@ -422,6 +497,7 @@ Config* Config::get(const PathName& lookupName) void Config::enumerate(ReplicaList& replicas) { PathName dbName; + FbLocalStatus localStatus; try { @@ -474,12 +550,20 @@ void Config::enumerate(ReplicaList& replicas) { config->sourceDirectory = value.c_str(); PathUtils::ensureSeparator(config->sourceDirectory); + if (!checkAccess(&localStatus, config->sourceDirectory, key)) + { + config->sourceDirectory.erase(); + continue; + } } else if (key == "source_guid") { config->sourceGuid = Guid::fromString(value); if (!config->sourceGuid) - configError("invalid (misformatted) value", key, value); + { + configError(&localStatus, "invalid (misformatted) value", key, value); + continue; + } } else if (key == "verbose_logging") { @@ -509,11 +593,11 @@ void Config::enumerate(ReplicaList& replicas) } catch (const Exception& ex) { - FbLocalStatus localStatus; composeError(&localStatus, ex); + } + if (localStatus->getState() & IStatus::STATE_ERRORS) logReplicaStatus(dbName, &localStatus); - } } // This routine is used for split input connection string to parts