diff --git a/src/libslic3r/Utils.hpp b/src/libslic3r/Utils.hpp index b21da72fc8..ff14e563fb 100644 --- a/src/libslic3r/Utils.hpp +++ b/src/libslic3r/Utils.hpp @@ -285,6 +285,21 @@ inline std::string sanitize_filename(const std::string &filename){ const std::regex special_chars("[/\\\\:*?\"<>|]"); return std::regex_replace(filename, special_chars, "_"); } +// Reduce an untrusted, possibly path-qualified name to a single sanitized file name. +// Returns an empty string when nothing usable remains. +inline std::string sanitize_file_basename(const std::string &name){ + const size_t sep = name.find_last_of("/\\"); + const std::string base = sanitize_filename(sep == std::string::npos ? name : name.substr(sep + 1)); + // Names made only of dots and spaces refer to the folder or its parent, or are stripped to nothing on Windows. + return base.find_first_not_of(". ") == std::string::npos ? std::string() : base; +} +// Marker file a download of this process writes to before it is renamed to filename. +boost::filesystem::path download_marker_path(const boost::filesystem::path &dest_folder, const std::string &filename); +// Finds a sanitized variant of filename, "name(N).ext" if needed, that neither an entry of dest_folder +// nor the download marker of another download uses. The marker at ignored_marker does not count. +// Returns true and the name in result, or false and the last name tried. +bool find_unused_filename(const boost::filesystem::path &dest_folder, const std::string &filename, + const boost::filesystem::path &ignored_marker, std::string &result); // File path / name / extension splitting utilities, working with UTF-8, // to be published to Perl. namespace PerlUtils { diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 9def5dad17..44fad7dc25 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -1321,6 +1321,31 @@ unsigned get_current_pid() #endif } +boost::filesystem::path download_marker_path(const boost::filesystem::path &dest_folder, const std::string &filename) +{ + return dest_folder / (filename + "." + std::to_string(get_current_pid()) + ".download"); +} + +bool find_unused_filename(const boost::filesystem::path &dest_folder, const std::string &filename, + const boost::filesystem::path &ignored_marker, std::string &result) +{ + // Probe the name that will be written, so a name the sanitizing maps onto an existing file is versioned too. + const std::string sanitized = sanitize_filename(filename); + const std::string extension = boost::filesystem::path(sanitized).extension().string(); + const std::string stem = sanitized.substr(0, sanitized.size() - extension.size()); + auto is_used = [&](const std::string &name) { + const boost::filesystem::path marker = download_marker_path(dest_folder, name); + return boost::filesystem::exists(dest_folder / name) || (marker != ignored_marker && boost::filesystem::exists(marker)); + }; + result = sanitized; + for (size_t version = 1; is_used(result); ++version) { + if (version > 999) + return false; + result = stem + "(" + std::to_string(version) + ")" + extension; + } + return true; +} + std::string per_user_temp_id() { #ifdef WIN32 diff --git a/src/slic3r/GUI/DownloaderFileGet.cpp b/src/slic3r/GUI/DownloaderFileGet.cpp index 6ad07c43d2..4a7269affa 100644 --- a/src/slic3r/GUI/DownloaderFileGet.cpp +++ b/src/slic3r/GUI/DownloaderFileGet.cpp @@ -71,17 +71,6 @@ bool FileGet::is_subdomain(const std::string& url, const std::string& domain) return false; } -namespace { -unsigned get_current_pid() -{ -#ifdef WIN32 - return GetCurrentProcessId(); -#else - return ::getpid(); -#endif -} -} - // int = DOWNLOAD ID; string = file path wxDEFINE_EVENT(EVT_DWNLDR_FILE_COMPLETE, wxCommandEvent); // int = DOWNLOAD ID; string = error msg @@ -144,25 +133,10 @@ void FileGet::priv::get_perform() std::string extension; if (m_written == 0) { - boost::filesystem::path dest_path = m_dest_folder / m_filename; - extension = dest_path.extension().string(); - std::string just_filename = m_filename.substr(0, m_filename.size() - extension.size()); - std::string final_filename = just_filename; - // Find unsed filename + std::string final_filename; + bool found = false; try { - size_t version = 0; - while (boost::filesystem::exists(m_dest_folder / (final_filename + extension)) || boost::filesystem::exists(m_dest_folder / (final_filename + extension + "." + std::to_string(get_current_pid()) + ".download"))) - { - ++version; - if (version > 999) { - wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); - evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / (final_filename + extension)).string())); - evt->SetInt(m_id); - m_evt_handler->QueueEvent(evt); - return; - } - final_filename = GUI::format("%1%(%2%)", just_filename, std::to_string(version)); - } + found = find_unused_filename(m_dest_folder, m_filename, m_tmp_path, final_filename); } catch (const boost::filesystem::filesystem_error& e) { wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); @@ -171,10 +145,18 @@ void FileGet::priv::get_perform() m_evt_handler->QueueEvent(evt); return; } + if (!found) { + wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); + evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / final_filename).string())); + evt->SetInt(m_id); + m_evt_handler->QueueEvent(evt); + return; + } - m_filename = sanitize_filename(final_filename + extension); + m_filename = final_filename; + extension = boost::filesystem::path(m_filename).extension().string(); - m_tmp_path = m_dest_folder / (m_filename + "." + std::to_string(get_current_pid()) + ".download"); + m_tmp_path = download_marker_path(m_dest_folder, m_filename); wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE); evt->SetString(boost::nowide::widen(m_filename)); @@ -221,7 +203,32 @@ void FileGet::priv::get_perform() if(dest_path.empty()) { std::string filename = extract_remote_filename(header); if (!filename.empty()) { - m_filename = filename; + // The name comes from the server: keep it inside the destination folder and never + // replace an existing file. Keep the current name if nothing usable remains. + filename = sanitize_file_basename(filename); + std::string unused; + try { + if (filename.empty() || !find_unused_filename(m_dest_folder, filename, m_tmp_path, unused)) + unused.clear(); + } catch (const boost::filesystem::filesystem_error&) { + unused.clear(); + } + const boost::filesystem::path tmp_path = unused.empty() ? m_tmp_path : download_marker_path(m_dest_folder, unused); + if (tmp_path != m_tmp_path) { + // Move the marker to the adopted name so that other downloads see the name as taken. + // Only before anything is written, so that no downloaded data has to be carried over. + FILE* tmp_file = m_written == 0 ? fopen(wxString(tmp_path.wstring()).c_str(), "wb") : nullptr; + if (tmp_file != nullptr) { + fclose(file); + boost::system::error_code ec; + boost::filesystem::remove(m_tmp_path, ec); + file = tmp_file; + m_tmp_path = tmp_path; + } else + unused.clear(); + } + if (!unused.empty()) + m_filename = unused; dest_path = m_dest_folder / m_filename; wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE); evt->SetString(boost::nowide::widen(m_filename)); @@ -327,6 +334,18 @@ void FileGet::priv::get_perform() m_evt_handler->QueueEvent(evt); } fclose(file); + // Another file may have taken the name while downloading. + if (!dest_path.empty() && boost::filesystem::exists(dest_path)) { + std::string unused; + if (!find_unused_filename(m_dest_folder, m_filename, m_tmp_path, unused)) + throw std::runtime_error("No unused file name."); + m_filename = unused; + dest_path = m_dest_folder / m_filename; + wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE); + evt->SetString(boost::nowide::widen(m_filename)); + evt->SetInt(m_id); + m_evt_handler->QueueEvent(evt); + } boost::filesystem::rename(m_tmp_path, dest_path); } catch (const std::exception& /*e*/) diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 3a61a95dfd..c95d94aba6 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -15909,6 +15909,11 @@ void Plater::import_model_id(wxString download_info) //wxString sError = error.what(); } + // The name comes from the link: reduce it to a plain file name inside the download folder. + filename = from_u8(sanitize_file_basename(into_u8(filename))); + if (filename.empty()) + filename = "untitled.3mf"; + bool download_ok = false; int retry_count = 0; const int max_retries = 3; @@ -15950,51 +15955,28 @@ void Plater::import_model_id(wxString download_info) msg = _L("Preparing 3MF file..."); - //gets the number of files with the same name - std::vector vecFiles; - bool is_already_exist = false; - - target_path = fs::path(wxGetApp().app_config->get("download_path")); - try - { - vecFiles.clear(); - wxString extension = fs::path(filename.wx_str()).extension().c_str(); - - - //check file suffix - if (!extension.Contains(".3mf")) { - msg = _L("Download failed; unknown file format."); - return; - } - - auto name = filename.substr(0, filename.length() - extension.length() - 1); - - for (const auto& iter : boost::filesystem::directory_iterator(target_path)) - { - if (boost::filesystem::is_directory(iter.path())) - continue; - - wxString sFile = iter.path().filename().string().c_str(); - if (strstr(sFile.c_str(), name.c_str()) != NULL) { - vecFiles.push_back(sFile); - } - - if (sFile == filename) is_already_exist = true; - } - } - catch (const std::exception&) - { - //wxString sError = error.what(); + //check file suffix + wxString extension = fs::path(filename.wx_str()).extension().c_str(); + if (!extension.Contains(".3mf")) { + msg = _L("Download failed; unknown file format."); + return; } - //update filename - if (is_already_exist && vecFiles.size() >= 1) { - wxString extension = fs::path(filename.wx_str()).extension().c_str(); - wxString name = filename.substr(0, filename.length() - extension.length()); - filename = wxString::Format("%s(%d)%s", name, vecFiles.size() + 1, extension).ToStdString(); + //never replace an existing file + std::string unused_filename; + try { + if (!find_unused_filename(target_path, into_u8(filename), {}, unused_filename)) + unused_filename.clear(); + } catch (const std::exception&) { + unused_filename.clear(); } + if (unused_filename.empty()) { + msg = _L("Importing to Orca Slicer failed. Please download the file and manually import it."); + return; + } + filename = from_u8(unused_filename); msg = _L("Downloading project..."); @@ -16006,10 +15988,6 @@ void Plater::import_model_id(wxString download_info) boost::uuids::uuid uuid = boost::uuids::random_generator()(); std::string unique = to_string(uuid).substr(0, 6); - if (filename.empty()) { - filename = "untitled.3mf"; - } - //target_path /= (boost::format("%1%_%2%.3mf") % filename % unique).str(); target_path /= fs::path(filename.wc_str()); @@ -16058,13 +16036,26 @@ void Plater::import_model_id(wxString download_info) cont = false; } }) - .on_complete([&cont, &download_ok, tmp_path, target_path](std::string body, unsigned /* http_status */) { + .on_complete([&cont, &download_ok, &msg, tmp_path, &target_path](std::string body, unsigned /* http_status */) { fs::fstream file(tmp_path, std::ios::out | std::ios::binary | std::ios::trunc); file.write(body.c_str(), body.size()); file.close(); - fs::rename(tmp_path, target_path); cont = false; - download_ok = true; + try { + // Another file may have taken the name while downloading. + std::string unused_filename; + if (find_unused_filename(target_path.parent_path(), target_path.filename().string(), {}, unused_filename)) { + target_path = target_path.parent_path() / unused_filename; + fs::rename(tmp_path, target_path); + download_ok = true; + return; + } + } catch (const std::exception &e) { + BOOST_LOG_TRIVIAL(error) << "import_model_id: failed to move the download into place: " << e.what(); + } + boost::system::error_code ec; + fs::remove(tmp_path, ec); + msg = _L("Importing to Orca Slicer failed. Please download the file and manually import it."); }).perform_sync(); // for break while diff --git a/tests/libslic3r/test_utils.cpp b/tests/libslic3r/test_utils.cpp index 7880b783f1..2111173326 100644 --- a/tests/libslic3r/test_utils.cpp +++ b/tests/libslic3r/test_utils.cpp @@ -152,3 +152,94 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha REQUIRE(resolve_cli_input_path("").empty()); } } + +TEST_CASE("sanitize_file_basename keeps only a plain file name from an untrusted name", "[Utils]") { + const std::string unicode = "\xe6\xa8\xa1\xe5\x9e\x8b \xc3\xa9t\xc3\xa9.3mf"; // UTF-8 CJK and accented Latin + const auto [input, expected] = GENERATE_COPY(table({ + {"normal.3mf", "normal.3mf"}, + {"../../x.3mf", "x.3mf"}, + {"..\\..\\x.3mf", "x.3mf"}, + {"C:\\x.3mf", "x.3mf"}, + {"C:x.3mf", "C_x.3mf"}, + {"/etc/x", "x"}, + {"a/b\\c.gcode", "c.gcode"}, + {"x:stream", "x_stream"}, // no NTFS alternate data stream + {"x.", "x."}, + {".3mf", ".3mf"}, + {unicode, unicode}, + })); + CAPTURE(input); + CHECK(sanitize_file_basename(input) == expected); +} + +TEST_CASE("sanitize_file_basename rejects names that do not name a file", "[Utils]") { + const std::string input = GENERATE(as{}, "", ".", "..", "../..", "dir/", "..\\", " ", ". .", "..."); + CAPTURE(input); + CHECK(sanitize_file_basename(input).empty()); +} + +namespace { +void touch(const boost::filesystem::path &path) { std::ofstream(path.string()) << "existing"; } +std::string file_contents(const boost::filesystem::path &path) +{ + std::ifstream file(path.string()); + return std::string(std::istreambuf_iterator(file), std::istreambuf_iterator()); +} +} // namespace + +TEST_CASE("find_unused_filename keeps a name nothing uses", "[Utils]") { + ScopedTemporaryDir dir; + std::string name; + REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name)); + CHECK(name == "model.3mf"); +} + +TEST_CASE("find_unused_filename versions a name an existing file uses", "[Utils]") { + ScopedTemporaryDir dir; + touch(dir.path() / "model.3mf"); + std::string name; + REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name)); + CHECK(name == "model(1).3mf"); +} + +TEST_CASE("find_unused_filename versions a name that maps onto an existing file once sanitized", "[Utils]") { + ScopedTemporaryDir dir; + touch(dir.path() / "my_model.3mf"); + const std::string input = GENERATE(as{}, "my?model.3mf", "my:model.3mf", "my*model.3mf"); + CAPTURE(input); + std::string name; + REQUIRE(find_unused_filename(dir.path(), input, {}, name)); + CHECK(name == "my_model(1).3mf"); + CHECK(file_contents(dir.path() / "my_model.3mf") == "existing"); +} + +TEST_CASE("find_unused_filename treats the marker of another download as used", "[Utils]") { + ScopedTemporaryDir dir; + touch(download_marker_path(dir.path(), "model.3mf")); + std::string name; + REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name)); + CHECK(name == "model(1).3mf"); +} + +TEST_CASE("find_unused_filename ignores the marker of the download asking", "[Utils]") { + ScopedTemporaryDir dir; + const boost::filesystem::path own_marker = download_marker_path(dir.path(), "model.3mf"); + touch(own_marker); + std::string name; + REQUIRE(find_unused_filename(dir.path(), "model.3mf", own_marker, name)); + CHECK(name == "model.3mf"); +} + +TEST_CASE("find_unused_filename gives up after 999 versions", "[Utils]") { + ScopedTemporaryDir dir; + touch(dir.path() / "model.3mf"); + for (int version = 1; version < 999; ++version) + touch(dir.path() / ("model(" + std::to_string(version) + ").3mf")); + std::string name; + REQUIRE(find_unused_filename(dir.path(), "model.3mf", {}, name)); + CHECK(name == "model(999).3mf"); + + touch(dir.path() / name); + REQUIRE_FALSE(find_unused_filename(dir.path(), "model.3mf", {}, name)); + CHECK(name == "model(999).3mf"); +}