mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-14 03:11:19 +00:00
Probe config-supplied paths without throwing on an unreadable path (#15669)
The startup probes for download_path and last_backup_path used the throwing boost::filesystem overloads, so a path that could not be probed at all, such as one whose parent is not searchable, raised filesystem_error instead of falling back. init_download_path() runs from the GUI_App constructor inside GUI_Run()'s try, which left the application showing a message box and exiting with no window and no log file, the throw preceding the log sink. has_restore_data() runs from the queued EVT_RESTORE_PROJECT handler, whose try starts after the call. Five probes in those three functions take the error_code overload instead. Nothing branches on ec, and readable or missing paths are unaffected because both overloads return the same value for them. A new test covers has_restore_data() against a backup directory whose parent has no search permission, and skips on Windows, as root, and where the mode change is refused or not enforced.
This commit is contained in:
@@ -9776,7 +9776,10 @@ bool has_restore_data(std::string & path, std::string& origin)
|
||||
origin = "<lock>";
|
||||
return false;
|
||||
}
|
||||
if (boost::filesystem::exists(path + "/lock.txt")) {
|
||||
// ORCA: non-throwing probes; ec is deliberately unread, a backup path that cannot be
|
||||
// probed is treated as having no restore data rather than aborting the application.
|
||||
boost::system::error_code ec;
|
||||
if (boost::filesystem::exists(path + "/lock.txt", ec)) {
|
||||
std::string pid;
|
||||
load_string_file(path + "/lock.txt", pid);
|
||||
try {
|
||||
@@ -9791,10 +9794,10 @@ bool has_restore_data(std::string & path, std::string& origin)
|
||||
}
|
||||
}
|
||||
std::string file3mf = path + "/.3mf";
|
||||
if (!boost::filesystem::exists(file3mf))
|
||||
if (!boost::filesystem::exists(file3mf, ec))
|
||||
return false;
|
||||
try {
|
||||
if (boost::filesystem::exists(path + "/origin.txt"))
|
||||
if (boost::filesystem::exists(path + "/origin.txt", ec))
|
||||
load_string_file(path + "/origin.txt", origin);
|
||||
}
|
||||
catch (...) {
|
||||
|
||||
@@ -2606,7 +2606,10 @@ void GUI_App::init_download_path()
|
||||
}
|
||||
else {
|
||||
fs::path dp(down_path);
|
||||
if (!fs::exists(dp)) {
|
||||
// ORCA: non-throwing probe; ec is deliberately unread, a download_path that cannot be
|
||||
// probed falls back to the default like a missing one instead of failing startup.
|
||||
boost::system::error_code ec;
|
||||
if (!fs::exists(dp, ec)) {
|
||||
|
||||
std::string user_down_path = wxStandardPaths::Get().GetUserDir(wxStandardPaths::Dir_Downloads).ToUTF8().data();
|
||||
app_config->set("download_path", user_down_path);
|
||||
@@ -10132,7 +10135,9 @@ void GUI_App::start_download(std::string url)
|
||||
}
|
||||
//lets always init so if the download dest folder was changed, new dest is used
|
||||
boost::filesystem::path dest_folder(app_config->get("download_path"));
|
||||
if (dest_folder.empty() || !boost::filesystem::is_directory(dest_folder)) {
|
||||
// ORCA: non-throwing probe, as in init_download_path(); ec is deliberately unread.
|
||||
boost::system::error_code ec;
|
||||
if (dest_folder.empty() || !boost::filesystem::is_directory(dest_folder, ec)) {
|
||||
std::string msg = _u8L("Could not start URL download. Destination folder is not set. Please choose destination folder in Configuration Wizard.");
|
||||
BOOST_LOG_TRIVIAL(error) << msg;
|
||||
show_error(nullptr, msg);
|
||||
|
||||
@@ -36,12 +36,18 @@
|
||||
#include <nlohmann/json.hpp>
|
||||
|
||||
#include <boost/filesystem/operations.hpp>
|
||||
#include <boost/filesystem/file_status.hpp> // no_perms, owner_all
|
||||
#include <boost/nowide/fstream.hpp>
|
||||
#include <boost/algorithm/string/predicate.hpp>
|
||||
#include <algorithm>
|
||||
#include <functional>
|
||||
#include <initializer_list>
|
||||
|
||||
#include <fstream>
|
||||
#ifndef _WIN32
|
||||
#include <unistd.h> // geteuid
|
||||
#endif
|
||||
|
||||
#include <catch2/catch_tostring.hpp>
|
||||
#include <Eigen/Core>
|
||||
#include <Eigen/Geometry>
|
||||
@@ -2313,3 +2319,47 @@ SCENARIO("thumbnail relationships are declared only for thumbnails that were wri
|
||||
}
|
||||
}
|
||||
}
|
||||
// has_restore_data() runs from the queued EVT_RESTORE_PROJECT handler, whose try starts after
|
||||
// the call, so a filesystem_error from probing the stored backup path aborts the application.
|
||||
// The realistic trigger is an absolute path from another user's config whose home is not
|
||||
// searchable, so the parent, not the backup directory, is what blocks.
|
||||
#ifndef _WIN32
|
||||
TEST_CASE("has_restore_data reports no restore data for a path it cannot probe", "[3mf]") {
|
||||
if (::geteuid() == 0)
|
||||
SKIP("root ignores the directory permissions this test depends on");
|
||||
|
||||
ScopedTemporaryDir outer("orca_restore");
|
||||
boost::filesystem::path backup = outer.path() / "backup";
|
||||
REQUIRE(boost::filesystem::create_directory(backup));
|
||||
std::ofstream((backup / ".3mf").string()) << "stand-in for the backed-up project";
|
||||
std::ofstream((backup / "origin.txt").string()) << "project.3mf";
|
||||
|
||||
// Control: while the path is readable the session is found, so the false below really is
|
||||
// the probe failing and not a mis-built fixture.
|
||||
std::string readable = backup.string(), origin;
|
||||
REQUIRE(has_restore_data(readable, origin));
|
||||
REQUIRE(readable == (backup / ".3mf").string());
|
||||
REQUIRE(origin == "project.3mf");
|
||||
|
||||
// Puts the permissions back before ~ScopedTemporaryDir needs them to clean up, and does so
|
||||
// even if an assertion below throws.
|
||||
struct Searchable {
|
||||
boost::filesystem::path dir;
|
||||
~Searchable() { boost::system::error_code ec;
|
||||
boost::filesystem::permissions(dir, boost::filesystem::owner_all, ec); }
|
||||
} restore{ outer.path() };
|
||||
|
||||
// Non-throwing here too: a filesystem that refuses the mode change has to reach the skip
|
||||
// below rather than fail the test. The probe then still succeeds, which is that same case.
|
||||
boost::system::error_code probe_ec;
|
||||
boost::filesystem::permissions(outer.path(), boost::filesystem::no_perms, probe_ec);
|
||||
if (boost::filesystem::exists(backup / ".3mf", probe_ec) || !probe_ec)
|
||||
SKIP("the backup path could not be made unprobeable here");
|
||||
|
||||
std::string unprobeable = backup.string(), origin2;
|
||||
bool has = true;
|
||||
REQUIRE_NOTHROW(has = has_restore_data(unprobeable, origin2));
|
||||
CHECK_FALSE(has);
|
||||
CHECK(unprobeable == backup.string()); // not rewritten to the .3mf it could not confirm
|
||||
}
|
||||
#endif // _WIN32
|
||||
|
||||
Reference in New Issue
Block a user