From ec3417898138e93d801290d063ee3da7c5f10d08 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 01:55:12 +0100 Subject: [PATCH 1/7] Add a QString-based variant of interpret_tilde --- utility/shared.cpp | 14 ++++++++++++++ utility/shared.h | 7 +++++-- utility/tests/test_paths.cpp | 10 ++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/utility/shared.cpp b/utility/shared.cpp index 3be886ab0b..2865ff1a0d 100644 --- a/utility/shared.cpp +++ b/utility/shared.cpp @@ -1166,6 +1166,20 @@ char *interpret_tilde_alloc(const char *filename) } } +/** + * Interpret ~/ in filename as home dir + */ +QString interpret_tilde(const QString &filename) +{ + if (filename == QLatin1String("~")) { + return QDir::homePath(); + } else if (filename.startsWith(QLatin1String("~/"))) { + return QDir::homePath() + filename.midRef(1); + } else { + return filename; + } +} + /** If the directory "pathname" does not exist, recursively create all directories until it does. diff --git a/utility/shared.h b/utility/shared.h index 4aca5202b2..8b9a22b7c8 100644 --- a/utility/shared.h +++ b/utility/shared.h @@ -179,8 +179,11 @@ enum m_pre_result match_prefix_full(m_pre_accessor_fn_t accessor_fn, char *get_multicast_group(bool ipv6_preferred); void free_multicast_group(); -void interpret_tilde(char *buf, size_t buf_size, const QString &filename); -char *interpret_tilde_alloc(const char *filename); + +[[deprecated]] void interpret_tilde(char *buf, size_t buf_size, + const QString &filename); +[[deprecated]] char *interpret_tilde_alloc(const char *filename); +QString interpret_tilde(const QString &filename); bool make_dir(const char *pathname); diff --git a/utility/tests/test_paths.cpp b/utility/tests/test_paths.cpp index 8c10d7c21d..78d14a63fa 100644 --- a/utility/tests/test_paths.cpp +++ b/utility/tests/test_paths.cpp @@ -9,9 +9,19 @@ class test_paths : public QObject { Q_OBJECT private slots: + void interpret_tilde(); void is_safe_filename(); }; +/** + * Tests \ref ::interpret_tilde + */ +void test_paths::interpret_tilde() +{ + QCOMPARE(::interpret_tilde(QLatin1String("~")), QDir::homePath()); + QCOMPARE(::interpret_tilde(QLatin1String("test")), QLatin1String("test")); +} + /** * Tests \ref ::is_safe_filename */ From c340627ebb2a2472473a4648dea4da4b74c60c5d Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 01:57:12 +0100 Subject: [PATCH 2/7] Replace interpret_tilde in make_dir --- utility/shared.cpp | 10 +++------- utility/shared.h | 2 +- utility/tests/test_paths.cpp | 11 +++++++++++ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/utility/shared.cpp b/utility/shared.cpp index 2865ff1a0d..c0cb749fb1 100644 --- a/utility/shared.cpp +++ b/utility/shared.cpp @@ -1167,7 +1167,7 @@ char *interpret_tilde_alloc(const char *filename) } /** - * Interpret ~/ in filename as home dir + * Interpret ~ in filename as home dir */ QString interpret_tilde(const QString &filename) { @@ -1184,15 +1184,11 @@ QString interpret_tilde(const QString &filename) If the directory "pathname" does not exist, recursively create all directories until it does. */ -bool make_dir(const char *pathname) +bool make_dir(const QString &pathname) { - auto *path = interpret_tilde_alloc(pathname); - auto str = QString::fromUtf8(path); // We can always create a directory with an empty name -- it's the current // folder. - auto r = str.isEmpty() || QDir().mkpath(str); - delete[] path; - return r; + return pathname.isEmpty() || QDir().mkpath(interpret_tilde(pathname)); } /** diff --git a/utility/shared.h b/utility/shared.h index 8b9a22b7c8..70287469cb 100644 --- a/utility/shared.h +++ b/utility/shared.h @@ -185,7 +185,7 @@ void free_multicast_group(); [[deprecated]] char *interpret_tilde_alloc(const char *filename); QString interpret_tilde(const QString &filename); -bool make_dir(const char *pathname); +bool make_dir(const QString &pathname); char scanin(char **buf, char *delimiters, char *dest, int size); diff --git a/utility/tests/test_paths.cpp b/utility/tests/test_paths.cpp index 78d14a63fa..83b270ecdd 100644 --- a/utility/tests/test_paths.cpp +++ b/utility/tests/test_paths.cpp @@ -11,6 +11,7 @@ class test_paths : public QObject { private slots: void interpret_tilde(); void is_safe_filename(); + void make_dir(); }; /** @@ -33,5 +34,15 @@ void test_paths::is_safe_filename() QCOMPARE(::is_safe_filename(QLatin1String("..")), false); } +/** + * Tests \ref ::make_dir + */ +void test_paths::make_dir() +{ + // The main difference between make_dir and QDir()::mkpath is that make_dir + // ignores empty paths. We don't check that QDir works as expected. + QVERIFY(::make_dir(QLatin1String(""))); +} + QTEST_MAIN(test_paths) #include "test_paths.moc" From 12cfebee007a7e453b1ddfc23cad8da23020275a Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 02:11:02 +0100 Subject: [PATCH 3/7] Port registry_ini to the new interpret_tilde --- utility/registry_ini.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/utility/registry_ini.cpp b/utility/registry_ini.cpp index 791a7c21c3..b55cd9914e 100644 --- a/utility/registry_ini.cpp +++ b/utility/registry_ini.cpp @@ -568,9 +568,7 @@ struct section_file *secfile_load_section(const QString &filename, const QString §ion, bool allow_duplicates) { - char real_filename[1024]; - - interpret_tilde(real_filename, sizeof(real_filename), filename); + const auto real_filename = interpret_tilde(filename); return secfile_from_input_file(inf_from_file(real_filename, datafilename), filename, section, allow_duplicates); } @@ -609,7 +607,6 @@ static bool is_legal_table_entry_name(char c, bool num) */ bool secfile_save(const struct section_file *secfile, QString filename) { - char real_filename[1024]; char pentry_name[128]; const char *col_entry_name; const struct entry_list_link *ent_iter, *save_iter, *col_iter; @@ -622,13 +619,13 @@ bool secfile_save(const struct section_file *secfile, QString filename) filename = secfile->name; } - interpret_tilde(real_filename, sizeof(real_filename), filename); + auto real_filename = interpret_tilde(filename); auto fs = std::make_unique(real_filename); fs->open(QIODevice::WriteOnly); if (!fs->isOpen()) { SECFILE_LOG(secfile, nullptr, _("Could not open %s for writing"), - real_filename); + qUtf8Printable((real_filename))); return false; } @@ -766,7 +763,8 @@ bool secfile_save(const struct section_file *secfile, QString filename) "a less efficient non-tabular format will be used.\n" "To avoid this make sure all rows of a table are\n" "filled out with an entry for every column.", - real_filename, section_name(psection), expect); + qUtf8Printable(real_filename), section_name(psection), + expect); fc_assert_ret_val(fs->write("\n") > 0, false); } fc_assert_ret_val(fs->write("}\n") > 0, false); @@ -835,7 +833,8 @@ bool secfile_save(const struct section_file *secfile, QString filename) if (fs->error() != 0) { SECFILE_LOG(secfile, nullptr, "Error before closing %s: %s", - real_filename, qUtf8Printable(fs->errorString())); + qUtf8Printable(real_filename), + qUtf8Printable(fs->errorString())); return false; } From 2a1a58cd485a473a50fa6372b793242d84b818dc Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 02:12:40 +0100 Subject: [PATCH 4/7] Port read_init_script_real to the new interpret_tilde --- server/stdinhand.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/server/stdinhand.cpp b/server/stdinhand.cpp index 7b4b6ba1be..050a1e6209 100644 --- a/server/stdinhand.cpp +++ b/server/stdinhand.cpp @@ -33,6 +33,7 @@ #include "rand.h" #include "registry.h" #include "section_file.h" +#include "shared.h" #include "support.h" // fc__attribute, bool type, etc. #include "timing.h" @@ -1134,10 +1135,7 @@ static bool read_init_script_real(struct connection *caller, int read_recursion) { FILE *script_file; - const char extension[] = ".serv"; - char serv_filename[strlen(extension) + qstrlen(script_filename) + 2]; - char tilde_filename[4096]; - QString real_filename; + const auto extension = QLatin1String(".serv"); // check recursion depth if (read_recursion > GAME_MAX_READ_RECURSION) { @@ -1145,33 +1143,31 @@ static bool read_init_script_real(struct connection *caller, return false; } - // abuse real_filename to find if we already have a .serv extension - real_filename = QString(script_filename); - if (!real_filename.endsWith(extension)) { - fc_snprintf(serv_filename, sizeof(serv_filename), "%s%s", - script_filename, extension); - } else { - sz_strlcpy(serv_filename, script_filename); + // Find if we already have a .serv extension + QString serv_filename = script_filename; + if (!serv_filename.endsWith(extension)) { + serv_filename += extension; } + QString tilde_filename; if (is_restricted(caller) && !from_cmdline) { if (!is_safe_filename(serv_filename)) { cmd_reply(CMD_READ_SCRIPT, caller, C_FAIL, _("Name \"%s\" disallowed for security reasons."), - serv_filename); + qUtf8Printable(serv_filename)); return false; } - sz_strlcpy(tilde_filename, serv_filename); + tilde_filename = serv_filename; } else { - interpret_tilde(tilde_filename, sizeof(tilde_filename), serv_filename); + tilde_filename = interpret_tilde(serv_filename); } - real_filename = fileinfoname(get_data_dirs(), tilde_filename); + auto real_filename = fileinfoname(get_data_dirs(), tilde_filename); if (real_filename.isEmpty()) { if (is_restricted(caller) && !from_cmdline) { cmd_reply(CMD_READ_SCRIPT, caller, C_FAIL, _("No command script found by the name \"%s\"."), - serv_filename); + qUtf8Printable(serv_filename)); return false; } // File is outside data directories From fbd72499d82f8a93cc890e623b8101af7be68978 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 02:13:03 +0100 Subject: [PATCH 5/7] Port write_init_script to the new interpret_tilde --- server/stdinhand.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/stdinhand.cpp b/server/stdinhand.cpp index 050a1e6209..8ab7ab46bd 100644 --- a/server/stdinhand.cpp +++ b/server/stdinhand.cpp @@ -1227,13 +1227,13 @@ QVector *get_init_script_choices() */ static void write_init_script(char *script_filename) { - char real_filename[1024], buf[256]; + char buf[256]; FILE *script_file; - interpret_tilde(real_filename, sizeof(real_filename), script_filename); + const auto real_filename = interpret_tilde(script_filename); if (QFile::exists(real_filename) - && (script_file = fc_fopen(real_filename, "w"))) { + && (script_file = fc_fopen(qUtf8Printable(real_filename), "w"))) { fprintf(script_file, "#FREECIV SERVER COMMAND FILE, version %s\n", freeciv21_version()); fputs( @@ -1285,7 +1285,8 @@ static void write_init_script(char *script_filename) fclose(script_file); } else { - qCritical(_("Could not write script file '%s'."), real_filename); + qCritical(_("Could not write script file '%s'."), + qUtf8Printable((real_filename))); } } From 967b8b5268d23e23a0e0cbb9d8e183340036c082 Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 02:13:22 +0100 Subject: [PATCH 6/7] Port lua_command to the new interpret_tilde --- server/stdinhand.cpp | 49 ++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/server/stdinhand.cpp b/server/stdinhand.cpp index 8ab7ab46bd..5fe9f514a2 100644 --- a/server/stdinhand.cpp +++ b/server/stdinhand.cpp @@ -5017,8 +5017,8 @@ static bool lua_command(struct connection *caller, char *arg, bool check, int read_recursion) { FILE *script_file; - const char extension[] = ".lua", *real_filename = nullptr; - char luafile[4096], tilde_filename[4096]; + const auto extension = QLatin1String(".lua"); + QString luafile, tilde_filename, real_filename; char *luaarg = nullptr; QStringList tokens; int ind; @@ -5093,13 +5093,10 @@ static bool lua_command(struct connection *caller, char *arg, bool check, } // Fall through. case LUA_FILE: - // Abuse real_filename to find if we already have a .lua extension. - real_filename = - luaarg + qstrlen(luaarg) - MIN(strlen(extension), qstrlen(luaarg)); - if (strcmp(real_filename, extension) != 0) { - fc_snprintf(luafile, sizeof(luafile), "%s%s", luaarg, extension); - } else { - sz_strlcpy(luafile, luaarg); + // find if we already have a .lua extension. + luafile = luaarg; + if (!luafile.endsWith(extension)) { + luafile += extension; } if (is_restricted(caller)) { @@ -5107,22 +5104,21 @@ static bool lua_command(struct connection *caller, char *arg, bool check, cmd_reply( CMD_LUA, caller, C_FAIL, _("Freeciv21 script '%s' disallowed for security reasons."), - luafile); + qUtf8Printable(luafile)); return false; ; } - sz_strlcpy(tilde_filename, luafile); + tilde_filename = luafile; } else { - interpret_tilde(tilde_filename, sizeof(tilde_filename), luafile); + tilde_filename = interpret_tilde(luafile); } - real_filename = - qUtf8Printable(fileinfoname(get_data_dirs(), tilde_filename)); - if (!real_filename) { + real_filename = fileinfoname(get_data_dirs(), tilde_filename); + if (real_filename.isEmpty()) { if (is_restricted(caller)) { cmd_reply(CMD_LUA, caller, C_FAIL, _("No Freeciv21 script found by the name '%s'."), - tilde_filename); + qUtf8Printable(tilde_filename)); return false; } // File is outside data directories @@ -5144,31 +5140,36 @@ static bool lua_command(struct connection *caller, char *arg, bool check, break; case LUA_FILE: cmd_reply(CMD_LUA, caller, C_COMMENT, - _("Loading Freeciv21 script file '%s'."), real_filename); + _("Loading Freeciv21 script file '%s'."), + qUtf8Printable(real_filename)); if (QFile::exists(real_filename) - && (script_file = fc_fopen(real_filename, "r"))) { - ret = script_server_do_file(caller, real_filename); + && (script_file = fc_fopen(qUtf8Printable(real_filename), "r"))) { + ret = script_server_do_file(caller, qUtf8Printable(real_filename)); fclose(script_file); return ret; } else { cmd_reply(CMD_LUA, caller, C_FAIL, - _("Cannot read Freeciv21 script '%s'."), real_filename); + _("Cannot read Freeciv21 script '%s'."), + qUtf8Printable(real_filename)); return false; } break; case LUA_UNSAFE_FILE: cmd_reply(CMD_LUA, caller, C_COMMENT, - _("Loading Freeciv21 script file '%s'."), real_filename); + _("Loading Freeciv21 script file '%s'."), + qUtf8Printable(real_filename)); if (QFile::exists(real_filename) - && (script_file = fc_fopen(real_filename, "r"))) { + && (script_file = fc_fopen(qUtf8Printable(real_filename), "r"))) { fclose(script_file); - ret = script_server_unsafe_do_file(caller, real_filename); + ret = script_server_unsafe_do_file(caller, + qUtf8Printable(real_filename)); return ret; } else { cmd_reply(CMD_LUA, caller, C_FAIL, - _("Cannot read Freeciv21 script '%s'."), real_filename); + _("Cannot read Freeciv21 script '%s'."), + qUtf8Printable(real_filename)); return false; } break; From 365d1eb34dd41315b322bcb77acdca63060e533d Mon Sep 17 00:00:00 2001 From: Louis Moureaux Date: Tue, 2 Jan 2024 02:14:17 +0100 Subject: [PATCH 7/7] Remove the old interpret_tilde functions --- utility/shared.cpp | 44 -------------------------------------------- utility/shared.h | 3 --- 2 files changed, 47 deletions(-) diff --git a/utility/shared.cpp b/utility/shared.cpp index c0cb749fb1..d7debef602 100644 --- a/utility/shared.cpp +++ b/utility/shared.cpp @@ -1122,50 +1122,6 @@ void free_multicast_group() mc_group = nullptr; } -/** - Interpret ~/ in filename as home dir - New path is returned in buf of size buf_size - - This may fail if the path is too long. It is better to use - interpret_tilde_alloc. - */ -void interpret_tilde(char *buf, size_t buf_size, const QString &filename) -{ - if (filename.startsWith(QLatin1String("~/"))) { - fc_snprintf(buf, buf_size, "%s/%s", qUtf8Printable(QDir::homePath()), - qUtf8Printable(filename.right(filename.length() - 2))); - } else if (filename == QLatin1String("~")) { - qstrncpy(buf, qUtf8Printable(QDir::homePath()), buf_size); - } else { - qstrncpy(buf, qUtf8Printable(filename), buf_size); - } -} - -/** - Interpret ~/ in filename as home dir - - The new path is returned in buf, as a newly allocated buffer. The new - path will always be allocated and written, even if there is no ~ present. - */ -char *interpret_tilde_alloc(const char *filename) -{ - if (filename[0] == '~' && filename[1] == '/') { - QString home = QDir::homePath(); - size_t sz; - char *buf; - - filename += 2; /* Skip past "~/" */ - sz = home.length() + qstrlen(filename) + 2; - buf = static_cast(fc_malloc(sz)); - fc_snprintf(buf, sz, "%s/%s", qUtf8Printable(home), filename); - return buf; - } else if (filename[0] == '~' && filename[1] == '\0') { - return fc_strdup(qUtf8Printable(QDir::homePath())); - } else { - return fc_strdup(filename); - } -} - /** * Interpret ~ in filename as home dir */ diff --git a/utility/shared.h b/utility/shared.h index 70287469cb..eec31edf4a 100644 --- a/utility/shared.h +++ b/utility/shared.h @@ -180,9 +180,6 @@ enum m_pre_result match_prefix_full(m_pre_accessor_fn_t accessor_fn, char *get_multicast_group(bool ipv6_preferred); void free_multicast_group(); -[[deprecated]] void interpret_tilde(char *buf, size_t buf_size, - const QString &filename); -[[deprecated]] char *interpret_tilde_alloc(const char *filename); QString interpret_tilde(const QString &filename); bool make_dir(const QString &pathname);