diff --git a/source/blender/blenkernel/intern/appdir.cc b/source/blender/blenkernel/intern/appdir.cc index 8d1079c4f15..4ff526881f9 100644 --- a/source/blender/blenkernel/intern/appdir.cc +++ b/source/blender/blenkernel/intern/appdir.cc @@ -70,6 +70,11 @@ static struct { char temp_dirname_base[FILE_MAX]; /** Volatile temporary directory (owned by Blender, removed on exit). */ char temp_dirname_session[FILE_MAX]; + /** + * True when this is a sub-directory owned & created by Blender, + * false when a session directory couldn't be created - in this case don't delete it. + */ + bool temp_dirname_session_can_be_deleted; } g_app{}; /** \} */ @@ -1144,16 +1149,19 @@ void BKE_appdir_app_templates(ListBase *templates) * \param tempdir_maxncpy: The size of the \a tempdir buffer. * \param userdir: Directory specified in user preferences (may be nullptr). * note that by default this is an empty string, only use when non-empty. + * + * \return true if `userdir` is used. */ -static void where_is_temp(char *tempdir, const size_t tempdir_maxncpy, const char *userdir) +static bool where_is_temp(char *tempdir, const size_t tempdir_maxncpy, const char *userdir) { if (userdir && BLI_temp_directory_path_copy_if_valid(tempdir, tempdir_maxncpy, userdir)) { - return; + return true; } BLI_temp_directory_path_get(tempdir, tempdir_maxncpy); + return false; } -static void tempdir_session_create(char *tempdir_session, +static bool tempdir_session_create(char *tempdir_session, const size_t tempdir_session_maxncpy, const char *tempdir) { @@ -1182,15 +1190,12 @@ static void tempdir_session_create(char *tempdir_session, if (BLI_is_dir(tempdir_session)) { BLI_path_slash_ensure(tempdir_session, tempdir_session_maxncpy); /* Success. */ - return; + return true; } } - CLOG_WARN(&LOG, - "Could not generate a temp file name for '%s', falling back to '%s'", - tempdir_session, - tempdir); - BLI_strncpy(tempdir_session, tempdir, tempdir_session_maxncpy); + CLOG_WARN(&LOG, "Could not generate a temp file name for '%s'", tempdir_session); + return false; } void BKE_tempdir_init(const char *userdir) @@ -1200,13 +1205,44 @@ void BKE_tempdir_init(const char *userdir) * Sets #g_app.temp_dirname_session to a #mkdtemp * generated sub-dir of #g_app.temp_dirname_base. */ - where_is_temp(g_app.temp_dirname_base, sizeof(g_app.temp_dirname_base), userdir); - /* Clear existing temp dir, if needed. */ BKE_tempdir_session_purge(); - /* Now that we have a valid temp dir, add system-generated unique sub-dir. */ - tempdir_session_create( - g_app.temp_dirname_session, sizeof(g_app.temp_dirname_session), g_app.temp_dirname_base); + + /* Perform two passes, the first pass for the user preference path, + * then a second pass if the the preferences failed to create the *session* sub-directory. + * + * This avoid problems if the preferences points to a path without write access, + * `C:\` or `/` for example. */ + g_app.temp_dirname_session_can_be_deleted = false; + for (int pass = 0; pass < 2; pass += 1) { + const bool from_userdir = where_is_temp( + g_app.temp_dirname_base, sizeof(g_app.temp_dirname_base), pass == 0 ? userdir : nullptr); + + /* Now that we have a valid temp dir, add system-generated unique sub-dir. */ + if (tempdir_session_create(g_app.temp_dirname_session, + sizeof(g_app.temp_dirname_session), + g_app.temp_dirname_base)) + { + /* Created the session sub-directory. */ + g_app.temp_dirname_session_can_be_deleted = true; + break; + } + + /* Only perform the second pass if the `userdir` was used + * and failed to created the sub-directory. */ + if (from_userdir == false) { + break; + } + } + + if (UNLIKELY(g_app.temp_dirname_session_can_be_deleted == false)) { + /* This should practically never happen as either the preferences or the systems + * default temporary directory should be usable, if not, use the base directory and warn. */ + STRNCPY(g_app.temp_dirname_session, g_app.temp_dirname_base); + CLOG_WARN(&LOG, + "Could not generate a temp session subdirectory, falling back to '%s'", + g_app.temp_dirname_base); + } } const char *BKE_tempdir_session() @@ -1221,6 +1257,11 @@ const char *BKE_tempdir_base() void BKE_tempdir_session_purge() { + if (g_app.temp_dirname_session_can_be_deleted == false) { + /* It's possible this path references an arbitrary location + * in that case *never* recursively remove, see: #139585. */ + return; + } if (g_app.temp_dirname_session[0] && BLI_is_dir(g_app.temp_dirname_session)) { BLI_delete(g_app.temp_dirname_session, true, true); } diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index b7402c6e2c7..731e3fc4bf8 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -226,6 +226,11 @@ endif() # PY API TESTS # ------------------------------------------------------------------------------ +add_blender_test( + script_pyapi_bpy_app_tempdir + --python ${CMAKE_CURRENT_LIST_DIR}/bl_pyapi_bpy_app_tempdir.py +) + add_blender_test( script_pyapi_bpy_path --python ${CMAKE_CURRENT_LIST_DIR}/bl_pyapi_bpy_path.py diff --git a/tests/python/bl_pyapi_bpy_app_tempdir.py b/tests/python/bl_pyapi_bpy_app_tempdir.py new file mode 100644 index 00000000000..b9892a9279f --- /dev/null +++ b/tests/python/bl_pyapi_bpy_app_tempdir.py @@ -0,0 +1,212 @@ +# SPDX-FileCopyrightText: 2025 Blender Authors +# +# SPDX-License-Identifier: GPL-2.0-or-later + +# ./blender.bin --background --factory-startup --python tests/python/bl_tempfile.py -- --verbose + +# NOTE(ideasman42): +# +# - Creating a directory without write permissions across all supported platforms +# is it not trivial, for the purpose of these tests simply create a file as a way of +# pointing to a temporary path which can't have sub-directories created under it. +# +# - Internally, when no temporary path is set the hard coded path `/tmp/` is used. +# For the purpose of the tests being complete - control over this path would be needed too. +# +# - When changing the temp directory & on exit +# Blender's *session* temp directory is recursively removed. +# +# These tests were added as part of a fix for a serious flaw (see #144042) +# which would recursively delete the users `C:\` since the tests aren't sand-boxed +# all of the following tests reference paths within a newly creating temporary directory +# instead of referencing root paths to avoid risks for developers who run tests. +# +# - A related test exists `BLI_tempfile_test.cc` however this doesn't deal with +# Blender's user preferences and fallbacks used in Blender. +# + +__all__ = ( + "main", +) + +import os +import unittest +import tempfile + + +is_win32 = os.name == "nt" +TEMP_ENV = "TEMP" if is_win32 else "TMPDIR" + +# TODO: remove this. Since it's checked, it must be removed! +if os.environ.get("TMP") is not None: + del os.environ["TMP"] + +BASE_DIR = "" + + +def system_temp_set(path: str) -> None: + os.environ[TEMP_ENV] = path + + +def prefs_temp_set(path: str) -> None: + import bpy # type: ignore + bpy.context.preferences.filepaths.temporary_directory = path + + +def prefs_temp_get() -> str: + import bpy + result = bpy.context.preferences.filepaths.temporary_directory + assert isinstance(result, str) + return result + + +def blender_tempdir_session_get() -> str: + import bpy + result = bpy.app.tempdir + assert isinstance(result, str) + return result + + +def empty_file(path: str) -> None: + with open(path, 'wb') as _fh: + pass + + +def commonpath_safe(paths: list[str]) -> str: + if is_win32: + try: + return os.path.commonpath(paths) + except ValueError: + # Workaround error on Windows. + # ValueError: Paths don't have the same drive + return "" + + return os.path.commonpath(paths) + + +class TestTempDir(unittest.TestCase): + + def setUp(self) -> None: + print(prefs_temp_get()) + assert prefs_temp_get() == "" + system_temp_set("") + prefs_temp_set("") + + def tearDown(self) -> None: + prefs_temp_set("") + system_temp_set("") + + def test_fallback(self) -> None: + # Set a file for preferences & system temp, ensure neither are used. + with tempfile.TemporaryDirectory() as tempdir: + + temp_sys = os.path.join(tempdir, "a_sys") + temp_bpy = os.path.join(tempdir, "b_bpy") + + empty_file(temp_sys) + empty_file(temp_bpy) + + system_temp_set(temp_sys) + prefs_temp_set(temp_bpy) + + temp_session = blender_tempdir_session_get() + + # Ensure neither are used. + # This will try to use `/tmp/`. + commonpath_test = commonpath_safe([temp_sys, temp_session]) + self.assertFalse(commonpath_test and os.path.samefile(temp_sys, commonpath_test)) + commonpath_test = commonpath_safe([temp_bpy, temp_session]) + self.assertFalse(commonpath_test and os.path.samefile(temp_bpy, commonpath_test)) + + def test_system(self) -> None: + # Set an file as the preferences temp directory, ensure the system path is used. + with tempfile.TemporaryDirectory() as tempdir: + + temp_sys = os.path.join(tempdir, "a_sys") + temp_bpy = os.path.join(tempdir, "b_bpy") + + os.mkdir(temp_sys) + empty_file(temp_bpy) + + system_temp_set(temp_sys) + prefs_temp_set(temp_bpy) + + temp_session = blender_tempdir_session_get() + + self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session]))) + + def test_prefs(self) -> None: + # Set an file as the system temp directory, ensure the preferences path is used. + with tempfile.TemporaryDirectory() as tempdir: + + temp_sys = os.path.join(tempdir, "a_sys") + temp_bpy = os.path.join(tempdir, "b_bpy") + + empty_file(temp_sys) + os.mkdir(temp_bpy) + + system_temp_set(temp_sys) + prefs_temp_set(temp_bpy) + + temp_session = blender_tempdir_session_get() + + self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session]))) + + def test_system_to_prefs(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + temp_sys = os.path.join(tempdir, "a_sys") + temp_bpy = os.path.join(tempdir, "b_bpy") + + os.mkdir(temp_sys) + empty_file(temp_bpy) + + system_temp_set(temp_sys) + prefs_temp_set(temp_bpy) + + temp_session = blender_tempdir_session_get() + self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session]))) + + # Now set the preferences and ensure the previous directory gets purged and the new one set. + os.unlink(temp_bpy) + os.mkdir(temp_bpy) + + # Ensure the preferences path is now used. + prefs_temp_set(temp_bpy) + + self.assertFalse(os.path.exists(temp_session)) + temp_session = blender_tempdir_session_get() + self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session]))) + + def test_prefs_to_system(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + temp_sys = os.path.join(tempdir, "a_sys") + temp_bpy = os.path.join(tempdir, "b_bpy") + + empty_file(temp_sys) + os.mkdir(temp_bpy) + + system_temp_set(temp_sys) + prefs_temp_set(temp_bpy) + + temp_session = blender_tempdir_session_get() + self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session]))) + + # Now set the preferences and ensure the previous directory gets purged and the new one set. + os.unlink(temp_sys) + os.mkdir(temp_sys) + + # Ensure the system path is now used. + prefs_temp_set(temp_sys) + + self.assertFalse(os.path.exists(temp_session)) + temp_session = blender_tempdir_session_get() + self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session]))) + + +def main(): + import sys + unittest.main(argv=[__file__] + (sys.argv[sys.argv.index("--") + 1:] if "--" in sys.argv else [])) + + +if __name__ == "__main__": + main() diff --git a/tools/check_source/check_mypy_config.py b/tools/check_source/check_mypy_config.py index cb9402d73c8..01b56105024 100644 --- a/tools/check_source/check_mypy_config.py +++ b/tools/check_source/check_mypy_config.py @@ -28,6 +28,7 @@ PATHS: tuple[tuple[str, tuple[Any, ...], dict[str, str]], ...] = ( ("scripts/modules/_bpy_internal/freedesktop.py", (), {}), ("source/blender/nodes/intern/discover_nodes.py", (), {}), ("tests/python/bl_keymap_validate.py", (), {}), + ("tests/python/bl_pyapi_bpy_app_tempdir.py", (), {}), ("tests/utils/blender_headless.py", (), {}), ("tools/check_blender_release/", (), {}), ("tools/check_docs/", (), {}),