Core: Sanitize ID/struct names when used in path templates

In #139438 we added three path template variables that get their values
from user-specified names:

- `scene_name`
- `camera_name`
- `node_name`

These names can contain characters that are illegal in filenames on some
systems, and also characters like "/" that would be interpreted as path
separators in a filepath. This can lead to unexpected outcomes.

For example, if a node's name is "Hue/Saturation" and the `{node_name}`
variable is used in a path, it would result in a "Hue" directory which
in turn contains a "Saturation" file. This is almost certainly not what
the user intended, because the slash in that name is not semantically
supposed to be a path separator.

This PR addresses this problem by splitting string variables into two
types: strings and filepaths. The contents of string variables are
sanitized to be valid filenames when substituted into a path template,
but the contents of filepath variables are left as-is.

Variables such as `blend_name`, `blend_dir`, etc. that actually
represent path components are then added as filepath variables, while
variables such as `scene_name` that represent plain strings without path
semantics are added as string variables.

Pull Request: https://projects.blender.org/blender/blender/pulls/142679
This commit is contained in:
Nathan Vegdahl
2025-07-22 14:47:56 +02:00
committed by Nathan Vegdahl
parent 65c5f09a59
commit d7c6cbc6bf
3 changed files with 162 additions and 63 deletions

View File

@@ -91,12 +91,26 @@ namespace blender::bke::path_templates {
* transient for collecting data that is relevant/available in a given
* templating context.
*
* There are currently three supported variable types: string, integer, and
* float. Names must be unique across all types: you can't have a string *and*
* There are currently four supported variable types:
*
* - String
* - Filepath
* - Integer
* - Float
*
* Names must be unique across all variable types: you can't have a string *and*
* integer both with the name "bob".
*
* A filepath variable can contain either a full or partial filepath. The
* distinction between string and filepath variables exists because non-path
* strings may include phrases like "and/or" or "A:Left", which shouldn't be
* interpreted with path semantics. When used in path templating, the contents
* of string variables are therefore sanitized (replacing "/", etc.), but the
* contents of filepath variables are left as-is.
*/
class VariableMap {
blender::Map<std::string, std::string> strings_;
blender::Map<std::string, std::string> filepaths_;
blender::Map<std::string, int64_t> integers_;
blender::Map<std::string, double> floats_;
@@ -125,6 +139,17 @@ class VariableMap {
*/
bool add_string(blender::StringRef name, blender::StringRef value);
/**
* Add a filepath variable with the given name and value.
*
* If there is already a variable with that name, regardless of type, the new
* variable is *not* added (no overwriting).
*
* \return True if the variable was successfully added, false if there was
* already a variable with that name.
*/
bool add_filepath(blender::StringRef name, blender::StringRef value);
/**
* Add an integer variable with the given name and value.
*
@@ -155,6 +180,14 @@ class VariableMap {
*/
std::optional<blender::StringRefNull> get_string(blender::StringRef name) const;
/**
* Fetch the value of the filepath variable with the given name.
*
* \return The value if a filepath variable with that name exists,
* #std::nullopt otherwise.
*/
std::optional<blender::StringRefNull> get_filepath(blender::StringRef name) const;
/**
* Fetch the value of the integer variable with the given name.
*
@@ -189,9 +222,9 @@ class VariableMap {
* \return True if the variable was successfully added, false if there was
* already a variable with that name.
*/
bool add_filename(blender::StringRef var_name,
blender::StringRefNull full_path,
blender::StringRef fallback);
bool add_filename_only(blender::StringRef var_name,
blender::StringRefNull full_path,
blender::StringRef fallback);
/**
* Add the path up-to-but-not-including the filename as a variable.

View File

@@ -6,6 +6,8 @@
#include "BLT_translation.hh"
#include "BLI_math_base.hh"
#include "BLI_path_utils.hh"
#include "BLI_span.hh"
#include "BKE_context.hh"
@@ -27,6 +29,9 @@ bool VariableMap::contains(blender::StringRef name) const
if (this->strings_.contains(name)) {
return true;
}
if (this->filepaths_.contains(name)) {
return true;
}
if (this->integers_.contains(name)) {
return true;
}
@@ -41,6 +46,9 @@ bool VariableMap::remove(blender::StringRef name)
if (this->strings_.remove(name)) {
return true;
}
if (this->filepaths_.remove(name)) {
return true;
}
if (this->integers_.remove(name)) {
return true;
}
@@ -59,6 +67,15 @@ bool VariableMap::add_string(blender::StringRef name, blender::StringRef value)
return true;
}
bool VariableMap::add_filepath(blender::StringRef name, blender::StringRef value)
{
if (this->contains(name)) {
return false;
}
this->filepaths_.add_new(name, value);
return true;
}
bool VariableMap::add_integer(blender::StringRef name, const int64_t value)
{
if (this->contains(name)) {
@@ -86,6 +103,15 @@ std::optional<blender::StringRefNull> VariableMap::get_string(blender::StringRef
return blender::StringRefNull(*value);
}
std::optional<blender::StringRefNull> VariableMap::get_filepath(blender::StringRef name) const
{
const std::string *value = this->filepaths_.lookup_ptr(name);
if (value == nullptr) {
return std::nullopt;
}
return blender::StringRefNull(*value);
}
std::optional<int64_t> VariableMap::get_integer(blender::StringRef name) const
{
const int64_t *value = this->integers_.lookup_ptr(name);
@@ -104,22 +130,24 @@ std::optional<double> VariableMap::get_float(blender::StringRef name) const
return *value;
}
bool VariableMap::add_filename(StringRef var_name, StringRefNull full_path, StringRef fallback)
bool VariableMap::add_filename_only(StringRef var_name,
StringRefNull full_path,
StringRef fallback)
{
const char *file_name = BLI_path_basename(full_path.c_str());
const char *file_name_end = BLI_path_extension_or_end(file_name);
if (file_name[0] == '\0') {
/* If there is no file name, default to the fallback. */
return this->add_string(var_name, fallback);
return this->add_filepath(var_name, fallback);
}
else if (file_name_end == file_name) {
/* When the filename has no extension, but starts with a period. */
return this->add_string(var_name, StringRef(file_name));
return this->add_filepath(var_name, StringRef(file_name));
}
else {
/* Normal case. */
return this->add_string(var_name, StringRef(file_name, file_name_end));
return this->add_filepath(var_name, StringRef(file_name, file_name_end));
}
}
@@ -129,12 +157,12 @@ bool VariableMap::add_path_up_to_file(StringRef var_name,
{
/* Empty path. */
if (full_path.is_empty()) {
return this->add_string(var_name, fallback);
return this->add_filepath(var_name, fallback);
}
/* No filename at the end. */
if (BLI_path_basename(full_path.c_str()) == full_path.end()) {
return this->add_string(var_name, full_path);
return this->add_filepath(var_name, full_path);
}
Vector<char> dir_path(full_path.size() + 1);
@@ -144,10 +172,10 @@ bool VariableMap::add_path_up_to_file(StringRef var_name,
if (!success || dir_path[0] == '\0') {
/* If no path before the filename, default to the fallback. */
return this->add_string(var_name, fallback);
return this->add_filepath(var_name, fallback);
}
return this->add_string(var_name, dir_path.data());
return this->add_filepath(var_name, dir_path.data());
}
bool operator==(const Error &left, const Error &right)
@@ -249,7 +277,8 @@ void BKE_add_template_variables_general(VariableMap &variables, const ID *path_o
{
const char *g_blend_file_path = BKE_main_blendfile_path_from_global();
variables.add_filename("blend_name", g_blend_file_path, blender::StringRef(DATA_("Unsaved")));
variables.add_filename_only(
"blend_name", g_blend_file_path, blender::StringRef(DATA_("Unsaved")));
/* Note: fallback to "./" for unsaved files, which if used at the start of a
* path is equivalent to the current working directory. This is consistent
@@ -260,7 +289,7 @@ void BKE_add_template_variables_general(VariableMap &variables, const ID *path_o
/* Library blend filepath (a.k.a. path to the blend file that actually owns the ID). */
if (path_owner_id) {
const char *lib_blend_file_path = ID_BLEND_PATH_FROM_GLOBAL(path_owner_id);
variables.add_filename(
variables.add_filename_only(
"blend_name_lib", lib_blend_file_path, blender::StringRef(DATA_("Unsaved")));
/* Note: fallback to "./" for unsaved files, which if used at the start of a
@@ -306,7 +335,7 @@ void BKE_add_template_variables_for_node(blender::bke::path_templates::VariableM
/* -------------------------------------------------------------------- */
#define FORMAT_BUFFER_SIZE 128
#define FORMAT_BUFFER_SIZE 512
namespace {
@@ -859,14 +888,25 @@ static blender::Vector<Error> eval_template(char *out_path,
if (std::optional<blender::StringRefNull> string_value = template_variables.get_string(
token.variable_name))
{
/* String variable found, but we only process it if there's no format
* specifier: string variables do not support format specifiers. */
if (token.format.type != FormatSpecifierType::NONE) {
/* String variables don't take format specifiers: error. */
errors.append({ErrorType::FORMAT_SPECIFIER, token.byte_range});
continue;
}
STRNCPY(replacement_string, string_value->c_str());
BLI_path_make_safe_filename(replacement_string);
break;
}
if (std::optional<blender::StringRefNull> path_value = template_variables.get_filepath(
token.variable_name))
{
if (token.format.type != FormatSpecifierType::NONE) {
/* Path variables don't take format specifiers: error. */
errors.append({ErrorType::FORMAT_SPECIFIER, token.byte_range});
continue;
}
STRNCPY(replacement_string, path_value->c_str());
break;
}

View File

@@ -68,98 +68,119 @@ TEST(path_templates, VariableMap)
EXPECT_FALSE(map.contains("hello"));
EXPECT_FALSE(map.remove("hello"));
EXPECT_EQ(std::nullopt, map.get_string("hello"));
EXPECT_EQ(std::nullopt, map.get_filepath("hello"));
EXPECT_EQ(std::nullopt, map.get_integer("hello"));
EXPECT_EQ(std::nullopt, map.get_float("hello"));
/* Populate the map. */
EXPECT_TRUE(map.add_string("hello", "What a wonderful world."));
EXPECT_TRUE(map.add_filepath("where", "/my/path"));
EXPECT_TRUE(map.add_integer("bye", 42));
EXPECT_TRUE(map.add_float("what", 3.14159));
/* Attempting to add variables with those names again should fail, since they
* already exist now. */
EXPECT_FALSE(map.add_string("hello", "Sup."));
EXPECT_FALSE(map.add_string("where", "Sup."));
EXPECT_FALSE(map.add_string("bye", "Sup."));
EXPECT_FALSE(map.add_string("what", "Sup."));
EXPECT_FALSE(map.add_filepath("hello", "/place"));
EXPECT_FALSE(map.add_filepath("where", "/place"));
EXPECT_FALSE(map.add_filepath("bye", "/place"));
EXPECT_FALSE(map.add_filepath("what", "/place"));
EXPECT_FALSE(map.add_integer("hello", 2));
EXPECT_FALSE(map.add_integer("where", 2));
EXPECT_FALSE(map.add_integer("bye", 2));
EXPECT_FALSE(map.add_integer("what", 2));
EXPECT_FALSE(map.add_float("hello", 2.71828));
EXPECT_FALSE(map.add_float("where", 2.71828));
EXPECT_FALSE(map.add_float("bye", 2.71828));
EXPECT_FALSE(map.add_float("what", 2.71828));
/* Confirm that the right variables exist. */
EXPECT_TRUE(map.contains("hello"));
EXPECT_TRUE(map.contains("where"));
EXPECT_TRUE(map.contains("bye"));
EXPECT_TRUE(map.contains("what"));
EXPECT_FALSE(map.contains("not here"));
/* Fetch the variables we added. */
EXPECT_EQ("What a wonderful world.", map.get_string("hello"));
EXPECT_EQ("/my/path", map.get_filepath("where"));
EXPECT_EQ(42, map.get_integer("bye"));
EXPECT_EQ(3.14159, map.get_float("what"));
/* The same variables shouldn't exist for the other types, despite our attempt
* to add them earlier. */
EXPECT_EQ(std::nullopt, map.get_filepath("hello"));
EXPECT_EQ(std::nullopt, map.get_integer("hello"));
EXPECT_EQ(std::nullopt, map.get_float("hello"));
EXPECT_EQ(std::nullopt, map.get_string("where"));
EXPECT_EQ(std::nullopt, map.get_integer("where"));
EXPECT_EQ(std::nullopt, map.get_float("where"));
EXPECT_EQ(std::nullopt, map.get_string("bye"));
EXPECT_EQ(std::nullopt, map.get_filepath("bye"));
EXPECT_EQ(std::nullopt, map.get_float("bye"));
EXPECT_EQ(std::nullopt, map.get_string("what"));
EXPECT_EQ(std::nullopt, map.get_filepath("what"));
EXPECT_EQ(std::nullopt, map.get_integer("what"));
/* Remove the variables. */
EXPECT_TRUE(map.remove("hello"));
EXPECT_TRUE(map.remove("where"));
EXPECT_TRUE(map.remove("bye"));
EXPECT_TRUE(map.remove("what"));
/* The variables shouldn't exist anymore. */
EXPECT_FALSE(map.contains("hello"));
EXPECT_FALSE(map.contains("where"));
EXPECT_FALSE(map.contains("bye"));
EXPECT_FALSE(map.contains("what"));
EXPECT_EQ(std::nullopt, map.get_string("hello"));
EXPECT_EQ(std::nullopt, map.get_filepath("where"));
EXPECT_EQ(std::nullopt, map.get_integer("bye"));
EXPECT_EQ(std::nullopt, map.get_float("what"));
EXPECT_FALSE(map.remove("hello"));
EXPECT_FALSE(map.remove("where"));
EXPECT_FALSE(map.remove("bye"));
EXPECT_FALSE(map.remove("what"));
}
TEST(path_templates, VariableMap_add_filename)
TEST(path_templates, VariableMap_add_filename_only)
{
VariableMap map;
EXPECT_TRUE(map.add_filename("a", "/home/bob/project_joe/scene_3.blend", "fallback"));
EXPECT_EQ("scene_3", map.get_string("a"));
EXPECT_TRUE(map.add_filename_only("a", "/home/bob/project_joe/scene_3.blend", "fallback"));
EXPECT_EQ("scene_3", map.get_filepath("a"));
EXPECT_TRUE(map.add_filename("b", "/home/bob/project_joe/scene_3", "fallback"));
EXPECT_EQ("scene_3", map.get_string("b"));
EXPECT_TRUE(map.add_filename_only("b", "/home/bob/project_joe/scene_3", "fallback"));
EXPECT_EQ("scene_3", map.get_filepath("b"));
EXPECT_TRUE(map.add_filename("c", "/home/bob/project_joe/scene.03.blend", "fallback"));
EXPECT_EQ("scene.03", map.get_string("c"));
EXPECT_TRUE(map.add_filename_only("c", "/home/bob/project_joe/scene.03.blend", "fallback"));
EXPECT_EQ("scene.03", map.get_filepath("c"));
EXPECT_TRUE(map.add_filename("d", "/home/bob/project_joe/.scene_3.blend", "fallback"));
EXPECT_EQ(".scene_3", map.get_string("d"));
EXPECT_TRUE(map.add_filename_only("d", "/home/bob/project_joe/.scene_3.blend", "fallback"));
EXPECT_EQ(".scene_3", map.get_filepath("d"));
EXPECT_TRUE(map.add_filename("e", "/home/bob/project_joe/.scene_3", "fallback"));
EXPECT_EQ(".scene_3", map.get_string("e"));
EXPECT_TRUE(map.add_filename_only("e", "/home/bob/project_joe/.scene_3", "fallback"));
EXPECT_EQ(".scene_3", map.get_filepath("e"));
EXPECT_TRUE(map.add_filename("f", "scene_3.blend", "fallback"));
EXPECT_EQ("scene_3", map.get_string("f"));
EXPECT_TRUE(map.add_filename_only("f", "scene_3.blend", "fallback"));
EXPECT_EQ("scene_3", map.get_filepath("f"));
EXPECT_TRUE(map.add_filename("g", "scene_3", "fallback"));
EXPECT_EQ("scene_3", map.get_string("g"));
EXPECT_TRUE(map.add_filename_only("g", "scene_3", "fallback"));
EXPECT_EQ("scene_3", map.get_filepath("g"));
/* No filename in path (ending slash means it's a directory). */
EXPECT_TRUE(map.add_filename("h", "/home/bob/project_joe/", "fallback"));
EXPECT_EQ("fallback", map.get_string("h"));
EXPECT_TRUE(map.add_filename_only("h", "/home/bob/project_joe/", "fallback"));
EXPECT_EQ("fallback", map.get_filepath("h"));
/* Empty path. */
EXPECT_TRUE(map.add_filename("i", "", "fallback"));
EXPECT_EQ("fallback", map.get_string("i"));
EXPECT_TRUE(map.add_filename_only("i", "", "fallback"));
EXPECT_EQ("fallback", map.get_filepath("i"));
/* Attempt to add already-added variable. */
EXPECT_FALSE(map.add_filename("i", "", "fallback"));
EXPECT_FALSE(map.add_filename_only("i", "", "fallback"));
}
TEST(path_templates, VariableMap_add_path_up_to_file)
@@ -167,31 +188,31 @@ TEST(path_templates, VariableMap_add_path_up_to_file)
VariableMap map;
EXPECT_TRUE(map.add_path_up_to_file("a", "/home/bob/project_joe/scene_3.blend", "fallback"));
EXPECT_EQ("/home/bob/project_joe/", map.get_string("a"));
EXPECT_EQ("/home/bob/project_joe/", map.get_filepath("a"));
EXPECT_TRUE(map.add_path_up_to_file("b", "project_joe/scene_3.blend", "fallback"));
EXPECT_EQ("project_joe/", map.get_string("b"));
EXPECT_EQ("project_joe/", map.get_filepath("b"));
EXPECT_TRUE(map.add_path_up_to_file("c", "/scene_3.blend", "fallback"));
EXPECT_EQ("/", map.get_string("c"));
EXPECT_EQ("/", map.get_filepath("c"));
/* No filename in path (ending slash means it's a directory). */
EXPECT_TRUE(map.add_path_up_to_file("e", "/home/bob/project_joe/", "fallback"));
EXPECT_EQ("/home/bob/project_joe/", map.get_string("e"));
EXPECT_EQ("/home/bob/project_joe/", map.get_filepath("e"));
EXPECT_TRUE(map.add_path_up_to_file("f", "/", "fallback"));
EXPECT_EQ("/", map.get_string("f"));
EXPECT_EQ("/", map.get_filepath("f"));
/* No leading path. */
EXPECT_TRUE(map.add_path_up_to_file("d", "scene_3.blend", "fallback"));
EXPECT_EQ("fallback", map.get_string("d"));
EXPECT_EQ("fallback", map.get_filepath("d"));
/* Empty path. */
EXPECT_TRUE(map.add_path_up_to_file("g", "", "fallback"));
EXPECT_EQ("fallback", map.get_string("g"));
EXPECT_EQ("fallback", map.get_filepath("g"));
/* Attempt to add already-added variable. */
EXPECT_FALSE(map.add_filename("g", "", "fallback"));
EXPECT_FALSE(map.add_filename_only("g", "", "fallback"));
}
struct PathTemplateTestCase {
@@ -206,7 +227,10 @@ TEST(path_templates, validate_and_apply_template)
{
variables.add_string("hi", "hello");
variables.add_string("bye", "goodbye");
variables.add_string("long", "This string is exactly 32 bytes.");
variables.add_string("empty", "");
variables.add_string("sanitize", "./\\?*:|\"<>");
variables.add_string("long", "This string is exactly 32 bytes_");
variables.add_filepath("path", "C:\\and/or/../nor/");
variables.add_integer("the_answer", 42);
variables.add_integer("prime", 7);
variables.add_integer("i_negative", -7);
@@ -222,10 +246,12 @@ TEST(path_templates, validate_and_apply_template)
const Vector<PathTemplateTestCase> test_cases = {
/* Simple case, testing all variables. */
{
"{hi}_{bye}_{the_answer}_{prime}_{i_negative}_{pi}_{e}_{ntsc}_{two}_{f_negative}_{huge}_"
"{tiny}",
"hello_goodbye_42_7_-7_3.141592653589793_2.718281828459045_29.970029970029973_2.0_-3."
"141592653589793_2e+32_2e-33",
"{hi}_{bye}_{empty}_{sanitize}_{path}"
"_{the_answer}_{prime}_{i_negative}_{pi}_{e}_{ntsc}_{two}"
"_{f_negative}_{huge}_{tiny}",
"hello_goodbye__.__________C:\\and/or/../nor/"
"_42_7_-7_3.141592653589793_2.718281828459045_29.970029970029973_2.0"
"_-3.141592653589793_2e+32_2e-33",
{},
},
@@ -379,18 +405,18 @@ TEST(path_templates, validate_and_apply_template)
"long}{long}{long}{long}{long}{long}{long}{long}{long}{long}{long}{long}{long}{long}{"
"long}{long}",
"___This string is exactly 32 bytes.This string is exactly 32 bytes.This string is "
"exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This "
"string is exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 "
"bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This string is "
"exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This "
"string is exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 "
"bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This string is "
"exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This "
"string is exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 "
"bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This string is "
"exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 bytes.This "
"string is exactly 32 bytes.This string is exactly 32 bytes.This string is exactly 32 "
"___This string is exactly 32 bytes_This string is exactly 32 bytes_This string is "
"exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This "
"string is exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 "
"bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This string is "
"exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This "
"string is exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 "
"bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This string is "
"exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This "
"string is exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 "
"bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This string is "
"exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 bytes_This "
"string is exactly 32 bytes_This string is exactly 32 bytes_This string is exactly 32 "
"by",
{},