Skip to content

Commit 6c88e6a

Browse files
zmurzmur
committed
Bug#35983164 NdbProcess treats space, backslash, double quote different on Windows and posix [#1]
When passing arguments to NdbProcess::create it will become important when introducing quoting to distinguish spaces that are port of the argument value or beeing an argument separator. This patch removes current uses of space as separator in arguments to NdbProcess::create. Change-Id: I1d1bab27e183fc33632bfd9974010129a8970365
1 parent 8080c36 commit 6c88e6a

File tree

3 files changed

+49
-22
lines changed

3 files changed

+49
-22
lines changed

‎storage/ndb/include/portlib/NdbProcess.hpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,24 @@ class NdbProcess {
7777

7878
public:
7979
void add(const char *str);
80+
/*
81+
* For '--name=value' options which will be passed as one argument.
82+
* Example: args.add("--id=", 7);
83+
*/
8084
void add(const char *str, const char *str2);
8185
void add(const char *str, int val);
86+
/*
87+
* For '-name value' options which will be passed as two arguments.
88+
* Example: args.add2("-id",7);
89+
*/
90+
void add2(const char *str, const char *str2);
91+
void add2(const char *str, int val);
92+
93+
/*
94+
* Add arguments one by one from another argument list.
95+
*/
8296
void add(const Args &args);
97+
8398
const Vector<BaseString> &args(void) const { return m_args; }
8499
void clear() { m_args.clear(); }
85100
};
@@ -187,12 +202,25 @@ inline void NdbProcess::Args::add(const char *str, const char *str2) {
187202
m_args.push_back(tmp);
188203
}
189204

205+
inline void NdbProcess::Args::add2(const char *str, const char *str2) {
206+
BaseString tmp;
207+
m_args.push_back(str);
208+
m_args.push_back(str2);
209+
}
210+
190211
inline void NdbProcess::Args::add(const char *str, int val) {
191212
BaseString tmp;
192213
tmp.assfmt("%s%d", str, val);
193214
m_args.push_back(tmp);
194215
}
195216

217+
inline void NdbProcess::Args::add2(const char *str, int val) {
218+
m_args.push_back(str);
219+
BaseString tmp;
220+
tmp.assfmt("%d", val);
221+
m_args.push_back(tmp);
222+
}
223+
196224
inline void NdbProcess::Args::add(const Args &args) {
197225
for (unsigned i = 0; i < args.m_args.size(); i++) add(args.m_args[i].c_str());
198226
}

‎storage/ndb/test/ndbapi/testMgmd.cpp

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class Mgmd {
144144
void common_args(NdbProcess::Args &args, const char *working_dir) {
145145
args.add("--no-defaults");
146146
args.add("--configdir=", working_dir);
147-
args.add("-f config.ini");
147+
args.add("--config-file=", "config.ini");
148148
if (with_nodeid) {
149149
args.add("--ndb-nodeid=", m_nodeid);
150150
}
@@ -427,7 +427,7 @@ static bool create_expired_cert(NDBT_Workingdir &wd) {
427427
args.add("--ndb-tls-search-path=", wd.path());
428428
args.add("--passphrase=", "Trondheim");
429429
args.add("-l"); // no-config mode
430-
args.add("-t db"); // type db
430+
args.add("--node-type=", "db"); // type db
431431
args.add("--duration=", "-50000"); // negative seconds; already expired
432432

433433
auto proc = NdbProcess::create("Create Keys", exe, wd.path(), args);
@@ -619,7 +619,7 @@ int runTestBug45495(NDBT_Context *ctx, NDBT_Step *step) {
619619
for (unsigned i = 0; i < mgmds.size(); i++) {
620620
CHECK(mgmds[i]->stop());
621621
// Start from config2.ini
622-
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f config2.ini",
622+
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f", "config2.ini",
623623
"--initial", NULL));
624624

625625
// Wait for mgmd to exit and check return status
@@ -646,7 +646,7 @@ int runTestBug45495(NDBT_Context *ctx, NDBT_Step *step) {
646646
for (unsigned i = 0; i < mgmds.size(); i++) {
647647
CHECK(mgmds[i]->stop());
648648
// Start from config2.ini
649-
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f config2.ini",
649+
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f", "config2.ini",
650650
"--reload", NULL));
651651
CHECK(mgmds[i]->connect(config));
652652
CHECK(mgmds[i]->wait_confirmed_config());
@@ -664,7 +664,7 @@ int runTestBug45495(NDBT_Context *ctx, NDBT_Step *step) {
664664
g_err << "** Reload mgmd initial(from generation=2)" << endl;
665665
for (unsigned i = 0; i < mgmds.size(); i++) {
666666
CHECK(mgmds[i]->stop());
667-
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f config2.ini",
667+
CHECK(mgmds[i]->start_from_config_ini(wd.path(), "-f", "config2.ini",
668668
"--reload", "--initial", NULL));
669669

670670
CHECK(mgmds[i]->connect(config));
@@ -888,8 +888,8 @@ int runTestNowaitNodes(NDBT_Context *ctx, NDBT_Step *step) {
888888
Mgmd *mgmd2 = mgmds[1];
889889
CHECK(mgmd2->stop());
890890
// Start from config2.ini
891-
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f config2.ini", "--reload",
892-
NULL));
891+
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f", "config2.ini",
892+
"--reload", NULL));
893893
CHECK(mgmd2->connect(config));
894894
CHECK(mgmd1->wait_confirmed_config());
895895
CHECK(mgmd2->wait_confirmed_config());
@@ -939,16 +939,18 @@ int runTestNowaitNodes2(NDBT_Context *ctx, NDBT_Step *step) {
939939
g_err << "** Start mgmd2 from config2.ini" << endl;
940940
Mgmd *mgmd2 = new Mgmd(2);
941941
mgmds.push_back(mgmd2);
942-
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f config2.ini", "--initial",
943-
"--nowait-nodes=1-255", NULL));
942+
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f", "config2.ini",
943+
"--initial", "--nowait-nodes=1-255",
944+
NULL));
944945
CHECK(mgmd2->wait(ret));
945946
CHECK(ret == 1);
946947

947948
CHECK(mgmd1->stop());
948949

949950
g_err << "** Start mgmd2 again from config2.ini" << endl;
950-
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f config2.ini", "--initial",
951-
"--nowait-nodes=1-255", NULL));
951+
CHECK(mgmd2->start_from_config_ini(wd.path(), "-f", "config2.ini",
952+
"--initial", "--nowait-nodes=1-255",
953+
NULL));
952954

953955
CHECK(mgmd2->connect(config));
954956
CHECK(mgmd2->wait_confirmed_config());
@@ -1011,7 +1013,7 @@ int runBug56844(NDBT_Context *ctx, NDBT_Step *step) {
10111013
for (unsigned i = 0; i < mgmds.size(); i++) {
10121014
// Start from config2.ini
10131015
CHECK(mgmds[i]->start_from_config_ini(
1014-
wd.path(), (l & 1) == 1 ? "-f config.ini" : "-f config2.ini",
1016+
wd.path(), "-f", (l & 1) == 1 ? "config.ini" : "config2.ini",
10151017
"--reload", NULL));
10161018
}
10171019
for (unsigned i = 0; i < mgmds.size(); i++) {
@@ -1469,7 +1471,7 @@ int runTestMgmdwithoutnodeid(NDBT_Context *ctx, NDBT_Step *step) {
14691471
CHECK(ConfigFactory::write_config_ini(
14701472
config, path(wd.path(), "config2.ini", NULL).c_str()));
14711473
with_nodeid = false;
1472-
CHECK(mgmd.start_from_config_ini(wd.path(), "-f config2.ini", "--initial",
1474+
CHECK(mgmd.start_from_config_ini(wd.path(), "-f", "config2.ini", "--initial",
14731475
NULL));
14741476
CHECK(mgmd.wait(exit_value));
14751477
CHECK(exit_value == 1);
@@ -1493,7 +1495,7 @@ int runTestMgmdwithoutnodeid(NDBT_Context *ctx, NDBT_Step *step) {
14931495
CHECK(ConfigFactory::write_config_ini(
14941496
config3, path(wd.path(), "config3.ini", NULL).c_str()));
14951497
with_nodeid = false;
1496-
CHECK(mgmd.start_from_config_ini(wd.path(), "-f config3.ini", "--initial",
1498+
CHECK(mgmd.start_from_config_ini(wd.path(), "-f", "config3.ini", "--initial",
14971499
NULL));
14981500
CHECK(mgmd.wait(exit_value));
14991501
CHECK(exit_value == 1);

‎storage/ndb/tools/sign_keys.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,13 +1130,10 @@ int remote_signing_method(BaseString &cmd, NdbProcess::Args &args,
11301130
args.add(opt_remote_path ? opt_remote_path : "openssl");
11311131
args.add("x509");
11321132
args.add("-req");
1133-
args.add("-CA");
1134-
args.add(opt_ca_cert); // full pathname on remote server
1135-
args.add("-CAkey");
1136-
args.add(opt_ca_key); // full pathname on remote server
1137-
args.add("-days ", csr->duration() / CertLifetime::SecondsPerDay);
1138-
args.add("-set_serial ");
1139-
args.add(hexSerial.c_str());
1133+
args.add2("-CA", opt_ca_cert); // full pathname on remote server
1134+
args.add2("-CAkey", opt_ca_key); // full pathname on remote server
1135+
args.add2("-days", csr->duration() / CertLifetime::SecondsPerDay);
1136+
args.add2("-set_serial", hexSerial.c_str());
11401137
return 2;
11411138
case SIGN_CO_PROCESS: // 3: Run the signing utility co-process
11421139
cmd.assign(opt_ca_tool);
@@ -1169,7 +1166,7 @@ int fetch_CA_cert_from_remote_openssl(stack_st_X509 *CA_certs) {
11691166
args.add(opt_ca_host);
11701167
args.add(opt_remote_path ? opt_remote_path : "openssl");
11711168
args.add("x509");
1172-
args.add("-in ", opt_ca_cert);
1169+
args.add2("-in", opt_ca_cert);
11731170

11741171
auto proc = NdbProcess::create("OpensslFetchCA", cmd, nullptr, args, &pipes);
11751172
if (!proc) return 133;

0 commit comments

Comments
 (0)