Sanitize Server-Supplied Download File Names (#15955)

* Sanitize Server-Supplied Download File Names

The URL downloader used the file name from the Content-Disposition header
as given, without the cleaning and unused-name search applied to the
URL-derived name.

Reduce the header name to a sanitized base name with the new
sanitize_file_basename helper, which splits on both path separators and
rejects names made only of dots and spaces. Run the result through the
same unused-name search as the URL-derived name, now shared in
find_unused_filename, and fall back to the URL-derived name when nothing
usable remains.

* Sanitize Download Names Before Choosing an Unused One

The unused-name search probed the name as given and sanitized the
result afterwards, so a name whose special characters are replaced
could be mapped onto a file that already exists.

Move the search into libslic3r as find_unused_filename, sanitize first
and probe the name that is actually written. The download marker path
is shared through download_marker_path. Restore the last tried name in
the error reported when no free name is found, and cover the search
with unit tests.

* Keep Downloads on an Unused Name Until They Complete

When the server supplied the name, the download marker stayed under the
URL-derived name, so the adopted name was not reserved against other
downloads. The final rename also replaced any file that took the name
while the download ran.

Move the marker to the adopted name before any data is written, and
check the name again right before the final rename, picking the next
free name if it is taken by then.

* Sanitize the File Name of Model Import Links

The model import took the file name from the link as given and only
avoided an existing file with a substring match on the folder listing.

Reduce the name to a sanitized base name, falling back to untitled.3mf,
choose the name with the shared unused-name search, and check it again
before the final rename.

* Handle Filesystem Errors When Finishing a Model Import Download

Choosing the final name and moving the downloaded project into place
could throw from inside the download callback. Any such error now removes
the temporary file and reports the existing import failure message.
This commit is contained in:
HanifKoh
2026-09-29 02:19:24 +08:00
committed by GitHub
parent 11e9e07f20
commit 41eeaf3883
5 changed files with 220 additions and 79 deletions
+91
View File
@@ -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<std::string, std::string>({
{"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<std::string>{}, "", ".", "..", "../..", "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<char>(file), std::istreambuf_iterator<char>());
}
} // 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<std::string>{}, "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");
}