From 06a03d1ada3d7b0598aa02ea536d83baf2b46215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnter=20Obiltschnig?= Date: Sat, 14 Oct 2023 11:34:59 +0200 Subject: [PATCH] Poco::TemporaryFile: fix possible naming collisions due to random zero increment --- Foundation/include/Poco/TemporaryFile.h | 4 ++++ Foundation/src/TemporaryFile.cpp | 2 +- Foundation/testsuite/src/FileTest.cpp | 23 +++++++++++++++++++++-- Foundation/testsuite/src/FileTest.h | 1 + 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Foundation/include/Poco/TemporaryFile.h b/Foundation/include/Poco/TemporaryFile.h index 9246846c2..9314613b3 100644 --- a/Foundation/include/Poco/TemporaryFile.h +++ b/Foundation/include/Poco/TemporaryFile.h @@ -40,6 +40,10 @@ class Foundation_API TemporaryFile: public File /// The class does, however, delete the temporary /// file - either in the destructor, or immediately /// before the application terminates. + /// + /// Note: Due to the way the temporary file names are generated, + /// collisions may be possible after 400-500 Million + /// file names generated by a single process. { public: TemporaryFile(); diff --git a/Foundation/src/TemporaryFile.cpp b/Foundation/src/TemporaryFile.cpp index a3777499b..463ee32d1 100644 --- a/Foundation/src/TemporaryFile.cpp +++ b/Foundation/src/TemporaryFile.cpp @@ -161,7 +161,7 @@ std::string TemporaryFile::tempName(const std::string& tempDir) randomChars.append(alphabet); } } - n = (count += random.next(1000)); + n = (count += random.next(1000) + 1); } std::ostringstream name; diff --git a/Foundation/testsuite/src/FileTest.cpp b/Foundation/testsuite/src/FileTest.cpp index 2663953fc..a42899cc8 100644 --- a/Foundation/testsuite/src/FileTest.cpp +++ b/Foundation/testsuite/src/FileTest.cpp @@ -380,6 +380,7 @@ void FileTest::testCopy() f1.setWriteable().remove(); } + void FileTest::testCopyFailIfDestinationFileExists() { std::ofstream ostr("testfile.dat"); @@ -414,6 +415,7 @@ void FileTest::testMove() assertTrue (f1 == f2); } + void FileTest::testMoveFailIfDestinationFileExists() { std::ofstream ostr("testfile.dat"); ostr << "Hello, world!" << std::endl; @@ -430,6 +432,7 @@ void FileTest::testMoveFailIfDestinationFileExists() { f1.setWriteable().remove(); } + void FileTest::testCopyDirectory() { Path pd1("testdir"); @@ -499,6 +502,7 @@ void FileTest::testCopyDirectory() fd3.remove(true); } + void FileTest::testCopyDirectoryFailIfExists() { Path pd1("testdir"); @@ -538,6 +542,7 @@ void FileTest::testCopyDirectoryFailIfExists() fd2.remove(true); } + void FileTest::testRename() { std::ofstream ostr("testfile.dat"); @@ -555,6 +560,7 @@ void FileTest::testRename() f2.remove(); } + void FileTest::testRenameFailIfExists() { std::ofstream ostr("testfile.dat"); ostr << "Hello, world!" << std::endl; @@ -580,8 +586,6 @@ void FileTest::testRenameFailIfExists() { } - - void FileTest::testLongPath() { #if defined(_WIN32) && !defined(_WIN32_WCE) @@ -620,6 +624,20 @@ void FileTest::testUnixFileExtension() assertEqual("", path2.getExtension()); } + +void FileTest::testTemporaryFile() +{ + const int COUNT = 10000; + std::set paths; + for (int i = 0; i < COUNT; i++) + { + Poco::TemporaryFile f; + paths.insert(f.path()); + } + assertTrue (paths.size() == COUNT); +} + + void FileTest::setUp() { File f("testfile.dat"); @@ -669,6 +687,7 @@ CppUnit::Test* FileTest::suite() CppUnit_addTest(pSuite, FileTest, testRootDir); CppUnit_addTest(pSuite, FileTest, testLongPath); CppUnit_addTest(pSuite, FileTest, testUnixFileExtension); + CppUnit_addTest(pSuite, FileTest, testTemporaryFile); return pSuite; } diff --git a/Foundation/testsuite/src/FileTest.h b/Foundation/testsuite/src/FileTest.h index c5448b457..65acb507c 100644 --- a/Foundation/testsuite/src/FileTest.h +++ b/Foundation/testsuite/src/FileTest.h @@ -43,6 +43,7 @@ public: void testRootDir(); void testLongPath(); void testUnixFileExtension(); + void testTemporaryFile(); void setUp(); void tearDown();