From f094704beb568acb354afdfb0ae40be3b60d4f0a Mon Sep 17 00:00:00 2001 From: Doug Walker Date: Fri, 9 Aug 2024 17:23:03 -0400 Subject: [PATCH] Harden unit test folder handling Signed-off-by: Doug Walker --- tests/cpu/OCIOZArchive_tests.cpp | 98 +++++++++++++++++++-------- tests/cpu/UnitTestUtils.cpp | 111 ++++++++++++++++++------------- 2 files changed, 135 insertions(+), 74 deletions(-) diff --git a/tests/cpu/OCIOZArchive_tests.cpp b/tests/cpu/OCIOZArchive_tests.cpp index 4906618b31..1f23550000 100644 --- a/tests/cpu/OCIOZArchive_tests.cpp +++ b/tests/cpu/OCIOZArchive_tests.cpp @@ -9,37 +9,80 @@ namespace OCIO = OCIO_NAMESPACE; namespace { -struct FileCreationGuard -{ - explicit FileCreationGuard(unsigned lineNo) - { - OCIO_CHECK_NO_THROW_FROM(m_filename = OCIO::Platform::CreateTempFilename(""), lineNo); - } - ~FileCreationGuard() + struct FileCreationGuard { - // Even if not strictly required on most OSes, perform the cleanup. - std::remove(m_filename.c_str()); - } + explicit FileCreationGuard(unsigned lineNo) + { + try + { + m_filename = OCIO::Platform::CreateTempFilename(""); + } + catch (...) + { + std::ostringstream ss; + ss << "Temp file creation for line " << lineNo << " failed. Test will fail."; + throw OCIO::Exception(ss.str().c_str()); + } + + if(!m_filename.empty()) + { + m_isCreated = true; + } + } + ~FileCreationGuard() + { + // Even if not strictly required on most OSes, perform the cleanup. + if (m_isCreated) + { + std::remove(m_filename.c_str()); + } + + m_isCreated = false; + } - std::string m_filename; -}; + std::string m_filename; + bool m_isCreated = false; + }; -struct DirectoryCreationGuard -{ - explicit DirectoryCreationGuard(const std::string name, unsigned lineNo) - { - OCIO_CHECK_NO_THROW_FROM( - m_directoryPath = OCIO::CreateTemporaryDirectory(name), lineNo - ); - } - ~DirectoryCreationGuard() + struct DirectoryCreationGuard { - // Even if not strictly required on most OSes, perform the cleanup. - OCIO::RemoveTemporaryDirectory(m_directoryPath); - } + explicit DirectoryCreationGuard(const std::string name, unsigned lineNo) + { + try + { + m_directoryPath = OCIO::CreateTemporaryDirectory(name); + } + catch (...) + { + std::ostringstream ss; + ss << "Temp folder creation for name '" << name << "' for line " << lineNo << " failed. Test will fail."; + throw OCIO::Exception(ss.str().c_str()); + } + + if (!m_directoryPath.empty()) + { + m_isCreated = true; + } + } + ~DirectoryCreationGuard() + { + // Even if not strictly required on most OSes, perform the cleanup. + if (m_isCreated && !m_directoryPath.empty()) + { + OCIO::RemoveTemporaryDirectory(m_directoryPath); + } + + m_isCreated = false; + } + + bool created() const + { + return m_isCreated; + } - std::string m_directoryPath; -}; + std::string m_directoryPath; + bool m_isCreated = false; + }; } //anon. @@ -524,6 +567,9 @@ OCIO_ADD_TEST(OCIOZArchive, extract_config_and_compare_to_original) // 2 - Extract the OCIOZ archive to temporary directory. DirectoryCreationGuard dGuard("context_test1", __LINE__); + + OCIO_REQUIRE_ASSERT(dGuard.created()); // Don't continue if the temp folder creation failed. + OCIO_CHECK_NO_THROW(OCIO::ExtractOCIOZArchive( archivePath.c_str(), dGuard.m_directoryPath.c_str() )); diff --git a/tests/cpu/UnitTestUtils.cpp b/tests/cpu/UnitTestUtils.cpp index f298db0f3e..e09b7f5ec0 100644 --- a/tests/cpu/UnitTestUtils.cpp +++ b/tests/cpu/UnitTestUtils.cpp @@ -71,38 +71,65 @@ ConstProcessorRcPtr GetFileTransformProcessor(const std::string & fileName) return config->getProcessor(fileTransform); } +namespace +{ + constexpr const char* TempDirMagicPrefix = "OCIOTestTemp_"; +} + std::string CreateTemporaryDirectory(const std::string & name) { + std::string extended_name = TempDirMagicPrefix + name; + int nError = 0; #if defined(_WIN32) - std::string sPath = GetEnvVariable("TEMP"); - static const std::string directory = pystring::os::path::join(sPath, name); + char cPath[MAX_PATH]; + DWORD len = GetTempPathA(MAX_PATH, cPath); + if( len>MAX_PATH || len==0 ) + { + throw Exception("Temp path could not be determined."); + } + + static const std::string directory = pystring::os::path::join(cPath, extended_name); nError = _mkdir(directory.c_str()); #else std::string sPath = "/tmp"; - const std::string directory = pystring::os::path::join(sPath, name); + const std::string directory = pystring::os::path::join(sPath, extended_name); nError = mkdir(directory.c_str(), 0777); #endif if (nError != 0) { std::ostringstream error; - error << "Could not create a temporary directory." << " Make sure that the directory do " - << "not already exist or sufficient permissions are set"; + error << "Could not create a temporary directory '" << directory << "'. Make sure that the directory do " + "not already exist or sufficient permissions are set"; throw Exception(error.str().c_str()); } return directory; } -#if defined(_WIN32) -void removeDirectory(const wchar_t* directoryPath) +void RemoveTemporaryDirectory(const std::string & sDirectoryPath) { - std::wstring search_path = std::wstring(directoryPath) + Platform::filenameToUTF("/*.*"); - std::wstring s_p = std::wstring(directoryPath) + Platform::filenameToUTF("/"); - WIN32_FIND_DATA fd; - HANDLE hFind = ::FindFirstFile(search_path.c_str(), &fd); + if(sDirectoryPath.empty()) + { + throw Exception("removeDirectory() is called with empty path."); + } + + // Sanity check: don't delete the folder if we haven't created it. + if(std::string::npos == sDirectoryPath.find(TempDirMagicPrefix)) + { + std::ostringstream error; + error << "removeDirectory() tries to delete folder '"<< sDirectoryPath << "' which was not created by the unit tests."; + throw Exception(error.str().c_str()); + } + +#if defined(_WIN32) + std::string search_pattern = sDirectoryPath + ("/*.*"); + std::string cur_path = sDirectoryPath + "/"; + + WIN32_FIND_DATAA fd; + HANDLE hFind = FindFirstFileA(search_pattern.c_str(), &fd); if (hFind != INVALID_HANDLE_VALUE) { do @@ -110,68 +137,56 @@ void removeDirectory(const wchar_t* directoryPath) if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { // Ignore "." and ".." directory. - if (wcscmp(fd.cFileName, Platform::filenameToUTF(".").c_str()) != 0 && - wcscmp(fd.cFileName, Platform::filenameToUTF("..").c_str()) != 0) + if ( strcmp(fd.cFileName, ".") && strcmp(fd.cFileName, "..") ) { - removeDirectory((wchar_t*)(s_p + fd.cFileName).c_str()); + RemoveTemporaryDirectory(cur_path + fd.cFileName); } } else { - DeleteFile((s_p + fd.cFileName).c_str()); + DeleteFileA((cur_path + fd.cFileName).c_str()); } - } while (::FindNextFile(hFind, &fd)); + } while (FindNextFileA(hFind, &fd)); - ::FindClose(hFind); - _wrmdir(directoryPath); + FindClose(hFind); + RemoveDirectoryA(sDirectoryPath.c_str()); } -} #else -void removeDirectory(const char * directoryPath) -{ - struct dirent *entry = NULL; - DIR *dir = NULL; - dir = opendir(directoryPath); - while((entry = readdir(dir))) - { - DIR *sub_dir = NULL; - FILE *file = NULL; + struct dirent* entry = NULL; + DIR* dir = NULL; + dir = opendir(sDirectoryPath.c_str()); + while ((entry = readdir(dir))) + { + DIR* sub_dir = NULL; + FILE* file = NULL; std::string absPath; // Ignore "." and ".." directory. if (!StringUtils::Compare(".", entry->d_name) && !StringUtils::Compare("..", entry->d_name)) { - absPath = pystring::os::path::join(directoryPath, entry->d_name); + absPath = pystring::os::path::join(sDirectoryPath, entry->d_name); sub_dir = opendir(absPath.c_str()); - if(sub_dir) - { + if (sub_dir) + { closedir(sub_dir); - removeDirectory(absPath.c_str()); - } - else - { + RemoveTemporaryDirectory(absPath); + } + else + { file = fopen(absPath.c_str(), "r"); - if(file) - { + if (file) + { fclose(file); remove(absPath.c_str()); - } - } + } + } } } - remove(directoryPath); + remove(sDirectoryPath.c_str()); closedir(dir); -} #endif -void RemoveTemporaryDirectory(const std::string & directoryPath) -{ -#if defined(_WIN32) - removeDirectory(Platform::filenameToUTF(directoryPath).c_str()); -#else - removeDirectory(directoryPath.c_str()); -#endif } } // namespace OCIO_NAMESPACE