Skip to content

Commit 171b012

Browse files
XaBbl4XaBbl4
committed
Do not connect to the database if the replication configuration is incorrect
Configuration errors can lead to loss of synchronization between master and replica.
1 parent 81bba14 commit 171b012

File tree

1 file changed

+111
-27
lines changed

1 file changed

+111
-27
lines changed

‎src/jrd/replication/Config.cpp‎

Lines changed: 111 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -79,38 +79,53 @@ namespace
7979
output = false;
8080
}
8181

82-
void configError(const string& type, const string& key, const string& value)
82+
void configError(CheckStatusWrapper* status, const string& type, const string& key, const string& value)
8383
{
8484
string msg;
85+
if (!(status->getState() & IStatus::STATE_ERRORS))
86+
{
87+
msg.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
88+
(Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status);
89+
}
90+
8591
msg.printf("%s specifies %s: %s", key.c_str(), type.c_str(), value.c_str());
86-
raiseError(msg.c_str());
92+
(Arg::Gds(isc_random) << Arg::Str(msg)).appendTo(status);
8793
}
8894

89-
void checkAccess(const PathName& path, const string& key)
95+
bool checkAccess(CheckStatusWrapper* status, const PathName& path, const string& key)
9096
{
9197
if (path.hasData() && !PathUtils::canAccess(path, 6))
92-
configError("missing or inaccessible directory", key, path.c_str());
98+
{
99+
configError(status, "missing or inaccessible directory", key, path.c_str());
100+
return false;
101+
}
102+
return true;
93103
}
94104

95105
void composeError(CheckStatusWrapper* status, const Exception& ex)
96106
{
97-
string prefix;
98-
prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
99-
100107
Arg::StatusVector sv;
101-
sv << Arg::Gds(isc_random) << Arg::Str(prefix);
108+
109+
if (!(status->getState() & IStatus::STATE_ERRORS))
110+
{
111+
string prefix;
112+
prefix.printf("Incorrect entry in %s", REPLICATION_CFGFILE);
113+
sv << Arg::Gds(isc_random) << Arg::Str(prefix);
114+
}
115+
116+
sv << Arg::StatusVector(status);
102117
sv << Arg::StatusVector(ex);
103118

104119
status->setErrors(sv.value());
105120
}
106121

107-
void parseExternalValue(const string& key, const string& value, string& output)
122+
bool parseExternalValue(CheckStatusWrapper* status, const string& key, const string& value, string& output)
108123
{
109124
const auto pos = key.rfind('_');
110125
if (pos == string::npos)
111126
{
112127
output = value.c_str();
113-
return;
128+
return true;
114129
}
115130

116131
string temp;
@@ -120,7 +135,10 @@ namespace
120135
{
121136
fb_utils::readenv(value.c_str(), temp);
122137
if (temp.isEmpty())
123-
configError("missing environment variable", key, value);
138+
{
139+
configError(status, "missing environment variable", key, value);
140+
return false;
141+
}
124142
}
125143
else if (key_source.equals(KEY_SUFFIX_FILE))
126144
{
@@ -131,7 +149,10 @@ namespace
131149

132150
AutoPtr<FILE> file(os_utils::fopen(filename.c_str(), "rt"));
133151
if (!file)
134-
configError("missing or inaccessible file", key, filename.c_str());
152+
{
153+
configError(status, "missing or inaccessible file", key, filename.c_str());
154+
return false;
155+
}
135156

136157
// skip first empty lines
137158
do
@@ -146,37 +167,55 @@ namespace
146167
} while (temp.isEmpty());
147168

148169
if (temp.isEmpty())
149-
configError("empty file", key, filename.c_str());
170+
{
171+
configError(status, "empty file", key, filename.c_str());
172+
return false;
173+
}
150174
}
151175

152176
output = temp.c_str();
177+
return true;
153178
}
154179

155-
void parseSyncReplica(const ConfigFile::Parameters& params, SyncReplica& output)
180+
bool parseSyncReplica(CheckStatusWrapper* status, const ConfigFile::Parameters& params, SyncReplica& output)
156181
{
182+
bool result = true;
157183
for (const auto& el : params)
158184
{
159185
const string key(el.name.c_str());
160186
const string value(el.value);
161187

162188
if (value.isEmpty())
163-
continue;
189+
{
190+
configError(status, "empty value", output.database, key);
191+
result = false;
192+
}
164193

165194
if (key.find("username") == 0)
166195
{
167196
if (output.username.hasData())
168-
configError("multiple values", output.database, "username");
169-
parseExternalValue(key, value, output.username);
197+
{
198+
configError(status, "multiple values", output.database, "username");
199+
result = false;
200+
}
201+
result &= parseExternalValue(status, key, value, output.username);
170202
}
171203
else if (key.find("password") == 0)
172204
{
173205
if (output.password.hasData())
174-
configError("multiple values", output.database, "password");
175-
parseExternalValue(key, value, output.password);
206+
{
207+
configError(status, "multiple values", output.database, "password");
208+
result = false;
209+
}
210+
result &= parseExternalValue(status, key, value, output.password);
176211
}
177212
else
178-
configError("unknown parameter", output.database, key);
213+
{
214+
configError(status, "unknown key", output.database, key);
215+
result = false;
216+
}
179217
}
218+
return result;
180219
}
181220
}
182221

@@ -253,7 +292,8 @@ Config* Config::get(const PathName& lookupName)
253292

254293
AutoPtr<Config> config(FB_NEW Config);
255294

256-
bool defaultFound = false, exactMatch = false;
295+
FbLocalStatus localStatus;
296+
bool defaultFound = false, exactMatch = false, replicaSkip = false;
257297

258298
for (const auto& section : cfgFile.getParameters())
259299
{
@@ -291,15 +331,24 @@ Config* Config::get(const PathName& lookupName)
291331
string value(el.value);
292332

293333
if (value.isEmpty())
334+
{
335+
configError(&localStatus, "empty value of key",
336+
exactMatch ? lookupName.c_str() : section.name.c_str(),
337+
key);
294338
continue;
339+
}
295340

296341
if (key == "sync_replica")
297342
{
298343
SyncReplica syncReplica(config->getPool());
299344
if (el.sub)
300345
{
301346
syncReplica.database = value;
302-
parseSyncReplica(el.sub->getParameters(), syncReplica);
347+
if (!parseSyncReplica(&localStatus, el.sub->getParameters(), syncReplica))
348+
{
349+
replicaSkip = true;
350+
continue;
351+
}
303352
}
304353
else
305354
splitConnectionString(value, syncReplica.database, syncReplica.username, syncReplica.password);
@@ -332,7 +381,12 @@ Config* Config::get(const PathName& lookupName)
332381
{
333382
config->journalDirectory = value.c_str();
334383
PathUtils::ensureSeparator(config->journalDirectory);
335-
checkAccess(config->journalDirectory, key);
384+
if (!checkAccess(&localStatus, config->journalDirectory, key))
385+
{
386+
config->journalDirectory.erase();
387+
replicaSkip = true;
388+
continue;
389+
}
336390
}
337391
else if (key == "journal_file_prefix")
338392
{
@@ -346,7 +400,11 @@ Config* Config::get(const PathName& lookupName)
346400
{
347401
config->archiveDirectory = value.c_str();
348402
PathUtils::ensureSeparator(config->archiveDirectory);
349-
checkAccess(config->archiveDirectory, key);
403+
if (!checkAccess(&localStatus, config->archiveDirectory, key))
404+
{
405+
config->archiveDirectory.erase();
406+
continue;
407+
}
350408
}
351409
else if (key == "journal_archive_command")
352410
{
@@ -376,12 +434,28 @@ Config* Config::get(const PathName& lookupName)
376434
{
377435
parseBoolean(value, config->cascadeReplication);
378436
}
437+
else if ((key != "journal_source_directory") &&
438+
(key != "source_guid") &&
439+
(key != "verbose_logging") &&
440+
(key != "apply_idle_timeout") &&
441+
(key != "apply_error_timeout"))
442+
{
443+
configError(&localStatus, "unknown key",
444+
exactMatch ? lookupName.c_str() : section.name.c_str(),
445+
key);
446+
}
379447
}
380448

381449
if (exactMatch)
382450
break;
383451
}
384452

453+
if (localStatus->getState() & IStatus::STATE_ERRORS)
454+
logPrimaryStatus(lookupName, &localStatus);
455+
456+
if (replicaSkip && !config->disableOnError)
457+
raiseError("One or more replicas configured with errors");
458+
385459
// TODO: As soon as plugin name is moved into RDB$PUBLICATIONS,
386460
// delay config parse until real replication start
387461
if (config->pluginName.hasData())
@@ -411,6 +485,7 @@ Config* Config::get(const PathName& lookupName)
411485
composeError(&localStatus, ex);
412486

413487
logPrimaryStatus(lookupName, &localStatus);
488+
localStatus.raise();
414489
}
415490

416491
return nullptr;
@@ -422,6 +497,7 @@ Config* Config::get(const PathName& lookupName)
422497
void Config::enumerate(ReplicaList& replicas)
423498
{
424499
PathName dbName;
500+
FbLocalStatus localStatus;
425501

426502
try
427503
{
@@ -474,12 +550,20 @@ void Config::enumerate(ReplicaList& replicas)
474550
{
475551
config->sourceDirectory = value.c_str();
476552
PathUtils::ensureSeparator(config->sourceDirectory);
553+
if (!checkAccess(&localStatus, config->sourceDirectory, key))
554+
{
555+
config->sourceDirectory.erase();
556+
continue;
557+
}
477558
}
478559
else if (key == "source_guid")
479560
{
480561
config->sourceGuid = Guid::fromString(value);
481562
if (!config->sourceGuid)
482-
configError("invalid (misformatted) value", key, value);
563+
{
564+
configError(&localStatus, "invalid (misformatted) value", key, value);
565+
continue;
566+
}
483567
}
484568
else if (key == "verbose_logging")
485569
{
@@ -509,11 +593,11 @@ void Config::enumerate(ReplicaList& replicas)
509593
}
510594
catch (const Exception& ex)
511595
{
512-
FbLocalStatus localStatus;
513596
composeError(&localStatus, ex);
597+
}
514598

599+
if (localStatus->getState() & IStatus::STATE_ERRORS)
515600
logReplicaStatus(dbName, &localStatus);
516-
}
517601
}
518602

519603
// This routine is used for split input connection string to parts

0 commit comments

Comments
 (0)