[lmi-commits] [lmi] master 5567c43 1/3: Begin to tackle an observed unit-test failure

[Greg Chicares]

branch: master
commit 5567c43724778afc4dd96c97c55bf736ec39f73c
Author: Gregory W. Chicares <gchicares@sbcglobal.net>
Commit: Gregory W. Chicares <gchicares@sbcglobal.net>

    Begin to tackle an observed unit-test failure
    
    Two unit tests succeeded on a multi-user server, then failed when run
    by another user. The reason is that a temporary directory was created
    with a static name (the same for all users) and not removed. Thus, the
    first user's directory remained, and the second user lacked permission
    to create and remove files therein.
    
    The approach taken here is to uniquify the directory name. In the shell,
    it would be natural to use a command like mktemp(1). C and C++ offer
    mktemp(3), but the freebsd manpage says it's "particularly dangerous
    from a security perspective", the openbsd linker warns whenever it's
    used, and the linux manpage says "Never use mktemp()". Instead, lmi's
    unique_filepath() function is used here.
    
    One call to fs::create_directory() is not uniquified--see its inline
    comment. That call did not cause a unit-test failure because it was
    paired with an fs::remove() call to remove that directory. The directory
    that's uniquified in this commit should also be removed, of course, and
    will be in the next commit.
---
 path_utility_test.cpp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/path_utility_test.cpp b/path_utility_test.cpp
index 9ba8575..5907f88 100644
--- a/path_utility_test.cpp
+++ b/path_utility_test.cpp
@@ -168,7 +168,8 @@ void test_serial_file_path()
 
 void test_unique_filepath_with_normal_filenames()
 {
-    std::string const tmp = "/tmp/" + fs::basename(__FILE__);
+    fs::path const u = unique_filepath("/tmp/" + fs::basename(__FILE__), "");
+    std::string const tmp = u.string();
     fs::path const tmpdir(fs::complete(tmp));
     fs::create_directory(tmpdir);
 
@@ -291,6 +292,15 @@ void test_path_validation()
     std::string context("Unit test file");
 
     // Create a file and a directory to test.
+    //
+    // Another test that calls fs::create_directory() uses an absolute
+    // path that's uniquified and canonicalized with fs::complete().
+    // This call uses a relative path, with no such safeguards; this
+    // being a unit test, it is appropriate to retain some fragility.
+    // If one user runs this test, and the directory created here
+    // somehow doesn't get deleted, then the test might fail for
+    // another user; that's interesting enough to report.
+
     fs::create_directory("path_utility_test_dir");
     write_dummy_file("path_utility_test_file");