From 4876a5f56c72f64a54627f9b6d0656878b7ca547 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 31 Oct 2016 18:05:20 +0300 Subject: Check if registered for cleanup path is in test scope working directory --- build2/test/script/runner.cxx | 45 ++++++++++++++++++++++------------- tests/test/script/runner/cleanup.test | 28 +++++++++++++++++++++- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index 2174286..f42299e 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -176,7 +176,7 @@ namespace build2 } void concurrent_runner:: - enter (scope& sp, const location& cl) + enter (scope& sp, const location&) { if (!exists (sp.wd_path)) // @@ Shouldn't we add an optional location parameter to mkdir() and @@ -184,11 +184,11 @@ namespace build2 // location info? // mkdir (sp.wd_path, 2); - else if (!empty (sp.wd_path)) - // @@ Shouldn't we have --wipe or smth? + else + // The working directory is cleaned up by the test rule prior the + // script execution. // - fail (cl) << "directory " << sp.wd_path << " is not empty" << - info << "clean it up and rerun"; + assert (empty (sp.wd_path)); sp.clean (sp.wd_path); } @@ -201,17 +201,26 @@ namespace build2 // // Note that we operate with normalized paths here. // + + // Verify that the path being cleaned up is a sub-path of the scope + // working directory. + // + auto verify = + [&sp, &cl] (const path& p, const path& orig, const char* what) + { + if (!p.sub (sp.wd_path)) + fail (cl) << "registered for cleanup " << what << " " << orig + << " is out of working directory " << sp.wd_path; + }; + for (const auto& p: reverse_iterate (sp.cleanups)) { - // @@ Should we forbid removal of files and directories not inside - // the scope working directory? Inventing recursive removal makes - // cleanup a bit unsafe. - // - // Remove directory if exists and empty. Fail otherwise. // if (p.to_directory ()) { + verify (p, p, "directory"); + dir_path d (path_cast (p)); // @@ If 'd' is a file then will fail with a diagnostics having no @@ -238,19 +247,19 @@ namespace build2 // succeeds. The removal of this entry can be handled at the time of // the containing directory removed. // - const string& s (p.string ()); - size_t n (s.size ()); - - if (n >= 4 && - string::traits_type::compare ( - s.c_str () + n - 4, "/***", 4) == 0) + if (p.leaf ().string () == "***") { + verify (p.directory (), p, "wildcard"); + // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r(). // rmdir_status r ( rmdir_r (p.directory (), true, static_cast (2))); - if (r == rmdir_status::not_empty) // Directory is current. + // Directory is current. That's unlikely to happen but let's keep + // for completeness. + // + if (r == rmdir_status::not_empty) fail (cl) << "registered for cleanup wildcard " << p << " matches the current directory"; @@ -259,6 +268,8 @@ namespace build2 // Remove file if exists. Fail otherwise. // + verify (p, p, "file"); + if (rmfile (p, 2) == rmfile_status::not_exist) fail (cl) << "registered for cleanup file " << p << " does not exist"; diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test index 61a66b4..200beb9 100644 --- a/tests/test/script/runner/cleanup.test +++ b/tests/test/script/runner/cleanup.test @@ -18,7 +18,7 @@ c = cat >>>testscript # # @@ TODO: $c <"$* -f a &a" && $b # -: files +: file : $c <"$* -f a &a"; $b @@ -29,14 +29,17 @@ $c <"$* -d a &a/"; $b : dir2 +: $c <"$* -d a/b &a/ &a/b/"; $b : file-dir +: $c <"$* -d a/b -f a/b/c &a/ &a/b/ &a/b/c"; $b : wildcard1 +: $c <"$* -d a/b -f a/b/c &a/***"; $b @@ -46,10 +49,12 @@ $c <"$* &a/***"; $b : file-dup +: $c <"$* -f a &a &a"; $b : dir-dup +: $c <"$* -d a/b &a/ &a/b/ &a/b/../b/"; $b @@ -62,6 +67,13 @@ $b 2>>EOE != 0 testscript:1: error: registered for cleanup file test/1/a does not exist EOE +: file-out-wd +: +$c <"$* &../a"; +$b 2>>EOE != 0 +testscript:1: error: registered for cleanup file test/a is out of working directory test/1/ +EOE + : dir-not-exists : $c <"$* &a/"; @@ -69,6 +81,13 @@ $b 2>>EOE != 0 testscript:1: error: registered for cleanup directory test/1/a/ does not exist EOE +: dir-out-wd +: +$c <"$* &../a/"; +$b 2>>EOE != 0 +testscript:1: error: registered for cleanup directory test/a/ is out of working directory test/1/ +EOE + : dir-not-empty1 : $c <"$* -d a/b -f a/b/c"; @@ -110,3 +129,10 @@ $c <"$* -f a &a/"; $b 2>>EOE != 0 error: unable to remove directory test/1/a/: Not a directory EOE + +: wildcard-out-wd +: +$c <"$* &../a/***"; +$b 2>>EOE != 0 +testscript:1: error: registered for cleanup wildcard test/a/*** is out of working directory test/1/ +EOE -- cgit v1.1