diff --git a/applications/test/fileName/Test-fileName.C b/applications/test/fileName/Test-fileName.C index 5de8912bd880e5c6c78dc80442abe76661031cd4..9ccd801245c3cb5d52dfa2b194e144b6809ba8b4 100644 --- a/applications/test/fileName/Test-fileName.C +++ b/applications/test/fileName/Test-fileName.C @@ -3,7 +3,7 @@ \\ / F ield | OpenFOAM: The Open Source CFD Toolbox \\ / O peration | \\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation - \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd. + \\/ M anipulation | Copyright (C) 2016-2017 OpenCFD Ltd. ------------------------------------------------------------------------------- License This file is part of OpenFOAM. @@ -39,15 +39,135 @@ Description #include "POSIX.H" #include "Switch.H" #include "etcFiles.H" +#include "Pair.H" +#include "Tuple2.H" +#include <fstream> using namespace Foam; +// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // + +unsigned testStrip +( + const bool doClean, + std::initializer_list + < + Tuple2<bool, std::string> + > tests +) +{ + Info<< nl << "Checking with clean=" << Switch(doClean) << nl << endl; + + unsigned nFail = 0; + + for (const Tuple2<bool, std::string>& test : tests) + { + const bool expected = test.first(); + const std::string& input = test.second(); + + fileName output(fileName::validate(input, doClean)); + + // Check for real failure (invalid chars) vs. + // spurious failure (removed double slashes with 'doClean'). + + const bool same = + ( + doClean + ? fileName::equals(input, output) + : (input == output) + ); + + if (same) + { + if (expected) + { + Info<< "(pass) validated " << input << " = " << output << nl; + } + else + { + ++nFail; + Info<< "(fail) unexpected success for " << input << nl; + } + } + else + { + if (expected) + { + ++nFail; + Info<< "(fail) unexpected"; + } + else + { + Info<< "(pass) expected"; + } + + Info<< " failure for " << input << nl; + } + } + + return nFail; +} + + +unsigned testEquals +( + std::initializer_list + < + Tuple2<bool, Pair<std::string>> + > tests +) +{ + Info<< nl << "Checking fileName::equals()" << nl << endl; + + unsigned nFail = 0; + + for (const Tuple2<bool, Pair<std::string>>& test : tests) + { + const bool expected = test.first(); + const std::string& s1 = test.second().first(); + const std::string& s2 = test.second().second(); + + const bool same = fileName::equals(s1, s2); + + if (same) + { + if (expected) + { + Info<< "(pass) success"; + } + else + { + ++nFail; + Info<< "(fail) unexpected success"; + } + } + else + { + if (expected) + { + ++nFail; + Info<< "(fail) unexpected failure"; + } + else + { + Info<< "(pass) expected failure"; + } + + } + + Info<< " for " << s1 << " == " << s2 << nl; + } + return nFail; +} + + // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * // // Main program: int main(int argc, char *argv[]) { argList::noParallel(); + argList::addBoolOption("validate", "test fileName::validate"); argList::addBoolOption("ext", "test handing of file extensions"); argList::addBoolOption("construct", "test constructors"); argList::addBoolOption("default", "reinstate default tests"); @@ -235,6 +355,69 @@ int main(int argc, char *argv[]) Info<< nl; } + + if (args.optionFound("validate")) + { + unsigned nFail = 0; + Info<< nl << "Test fileName::validate" << nl; + + // Without clean + nFail += testEquals + ( + { + { true, { "abc", "abc/" } }, + { true, { "///abc/", "//abc///" } }, + { false, { " ab //c/", "ab/c" } }, + } + ); + + Info<< nl << "Test fileName::validate" << nl; + + // Without clean + nFail += testStrip + ( + false, + { + { true, "abc/" }, + { true, "/", }, + { true, "//", }, + { true, "/abc/def", }, + { true, "/abc/def/", }, + { false, "/abc def" }, + { true, "/abc////def///", }, + { false, "/abc//// def///" }, + } + ); + + // With clean + nFail += testStrip + ( + true, + { + { true, "abc/" }, + { true, "/" }, + { true, "//" }, + { true, "/abc/def" }, + { true, "/abc/def/" }, + { false, "/abc def" }, + { true, "/abc////def///" }, + { false, "/abc//// def///" }, + } + ); + + Info<< nl; + if (nFail) + { + Info<< "failed " << nFail; + } + else + { + Info<< "passed all"; + } + Info<< " fileName::validate tests" << nl; + } + + if (!defaultTests) { return 0; @@ -312,10 +495,32 @@ int main(int argc, char *argv[]) Foam::rm(lnB); Foam::rmDir(dirB); + Info<< nl << "=========================" << nl + << "Test some copying and deletion" << endl; + Info<< "Creating directory " << dirA << endl; Foam::mkDir(dirA); + Info<< "Populating with various files" << endl; + for + ( + const std::string name + : { "file-1", "file-2", "bad name one", "bad name 2" } + ) + { + // Full path, but without any stripping + const fileName file + ( + (static_cast<const std::string&>(dirA) + "/" + name), + false + ); + + Info<<" create: " << file << endl; + + std::ofstream os(file); + os << "file=<" << file << ">" << nl; + } const int oldPosix = POSIX::debug; POSIX::debug = 1; @@ -362,7 +567,7 @@ int main(int argc, char *argv[]) << " but is " << lnB.type(true) << exit(FatalError); } - // Delete + // Delete (link) Foam::rm(lnB); } @@ -379,12 +584,13 @@ int main(int argc, char *argv[]) << " but is " << lnB.type(false) << exit(FatalError); } - // Delete - Foam::rm(lnB); + // Delete (directory, not link) + Foam::rmDir(lnB); } POSIX::debug = oldPosix; + // Verify that rmDir works with bad names too Foam::rmDir(dirA); Foam::rm(lnA); } diff --git a/applications/test/primitives/Test-primitives.C b/applications/test/primitives/Test-primitives.C index acdb71fb05235fd99dfe44c7f8692a4a96f68be9..d45a9feb73676b2c9a67ea7c0bfb404f465f9b83 100644 --- a/applications/test/primitives/Test-primitives.C +++ b/applications/test/primitives/Test-primitives.C @@ -51,7 +51,10 @@ template<class TYPE> unsigned testParsing ( TYPE (*function)(const std::string&), - const List<Tuple2<std::string, bool>>& tests + std::initializer_list + < + Tuple2<bool, std::string> + > tests ) { unsigned nFail = 0; @@ -60,10 +63,10 @@ unsigned testParsing // Expect some failures const bool prev = FatalIOError.throwExceptions(); - for (const Tuple2<std::string, bool>& test : tests) + for (const Tuple2<bool, std::string>& test : tests) { - const std::string& str = test.first(); - const bool expected = test.second(); + const bool expected = test.first(); + const std::string& str = test.second(); bool parsed = true; @@ -124,18 +127,18 @@ int main(int argc, char *argv[]) ( &readDouble, { - { "", false }, - { " ", false }, - { " xxx ", false }, - { " 1234E-", false }, - { " 1234E junk", false }, - { " 3.14159 ", true }, - { " 31.4159E-1 " , true }, - { " 100E1000 " , false }, - { " 1E-40 " , true }, - { " 1E-305 " , true }, - { " 1E-37 " , true }, - { " 1E-300 " , true }, + { false, "" }, + { false, " " }, + { false, " xxx " }, + { false, " 1234E-" }, + { false, " 1234E junk" }, + { true, " 3.14159 " }, + { true, " 31.4159E-1 " }, + { false, " 100E1000 " }, + { true, " 1E-40 " }, + { true, " 1E-305 " }, + { true, " 1E-37 " }, + { true, " 1E-300 " }, } ); } @@ -148,14 +151,14 @@ int main(int argc, char *argv[]) ( &readFloat, { - { " 3.14159 ", true }, - { " 31.4159E-1 " , true }, - { " 31.4159E200 " , false }, - { " 31.4159E20 " , true }, - { " 1E-40 " , true }, - { " 1E-305 " , true }, - { " 1E-37 " , true }, - { " 1E-300 " , true }, + { true, " 3.14159 " }, + { true, " 31.4159E-1 " }, + { false, " 31.4159E200 " }, + { true, " 31.4159E20 " }, + { true, " 1E-40 " }, + { true, " 1E-305 " }, + { true, " 1E-37 " }, + { true, " 1E-300 " }, } ); } @@ -166,15 +169,15 @@ int main(int argc, char *argv[]) ( &readNasScalar, { - { " 3.14159 ", true }, - { " 31.4159E-1 " , true }, - { " 314.159-2 " , true }, - { " 31.4159E200 " , true }, - { " 31.4159E20 " , true }, - { " 1E-40 " , true }, - { " 1E-305 " , true }, - { " 1E-37 " , true }, - { " 1E-300 " , true }, + { true, " 3.14159 " }, + { true, " 31.4159E-1 " }, + { true, " 314.159-2 " }, + { true, " 31.4159E200 " }, + { true, " 31.4159E20 " }, + { true, " 1E-40 " }, + { true, " 1E-305 " }, + { true, " 1E-37 " }, + { true, " 1E-300 " }, } ); } @@ -185,12 +188,12 @@ int main(int argc, char *argv[]) ( &readInt32, { - { " 3.14159 ", false }, - { " 31E1 ", false }, - { " 31.4159E-1 " , false }, - { "100" , true }, - { " 2147483644" , true }, - { " 2147483700 " , false }, + { false, " 3.14159 " }, + { false, " 31E1 " }, + { false, " 31.4159E-1 " }, + { true, "100" }, + { true, " 2147483644" }, + { false, " 2147483700 " }, } ); } @@ -202,10 +205,10 @@ int main(int argc, char *argv[]) ( &readUint32, { - { " 2147483644" , true }, - { " 2147483700 " , true }, - { " 4294967295 " , true }, - { " 4294968000 " , false }, + { true, "\t2147483644" }, + { true, " 2147483700 " }, + { true, " 4294967295 " }, + { false, " 4294968000 " }, } ); } diff --git a/src/OSspecific/POSIX/POSIX.C b/src/OSspecific/POSIX/POSIX.C index 708375bfadd8a8ce87dd64e8aa48943d983d222a..432dc8ccef4b3d683b0894ae6e35b12d55335692 100644 --- a/src/OSspecific/POSIX/POSIX.C +++ b/src/OSspecific/POSIX/POSIX.C @@ -109,6 +109,35 @@ static inline bool isBackupName(const Foam::fileName& name) ); } + +// Like fileName "/" global operator, but retain any invalid characters +static inline Foam::fileName fileNameConcat +( + const std::string& a, + const std::string& b +) +{ + if (a.size()) + { + if (b.size()) + { + // Two non-empty strings: can concatenate + return Foam::fileName((a + '/' + b), false); + } + + return Foam::fileName(a, false); + } + + // Or, if the first string is empty + + if (b.size()) + { + return Foam::fileName(b, false); + } + + // Both strings are empty + return Foam::fileName(); +} //! \endcond @@ -148,12 +177,10 @@ Foam::string Foam::getEnv(const std::string& envName) { return string(env); } - else - { - // Return null-constructed string rather than string::null - // to avoid cyclic dependencies in the construction of globals - return string(); - } + + // Return null-constructed string rather than string::null + // to avoid cyclic dependencies in the construction of globals + return string(); } @@ -220,10 +247,8 @@ Foam::string Foam::userName() { return pw->pw_name; } - else - { - return string(); - } + + return string(); } @@ -246,10 +271,8 @@ Foam::fileName Foam::home() { return pw->pw_dir; } - else - { - return fileName(); - } + + return fileName(); } @@ -266,10 +289,8 @@ Foam::fileName Foam::home(const std::string& userName) { return pw->pw_dir; } - else - { - return fileName(); - } + + return fileName(); } @@ -708,14 +729,15 @@ Foam::fileNameList Foam::readDir const bool followLink ) { - // Initial filename list size - // also used as increment if initial size found to be insufficient + // Initial filename list size and the increment when resizing the list static const int maxNnames = 100; // Basic sanity: cannot strip '.gz' from directory names const bool stripgz = filtergz && (type != fileName::DIRECTORY); const word extgz("gz"); + fileNameList dirEntries; + // Open directory and set the structure pointer // Do not attempt to open an empty directory name DIR *source; @@ -731,12 +753,12 @@ Foam::fileNameList Foam::readDir << "cannot open directory " << directory << endl; } - return fileNameList(); + return dirEntries; } if (POSIX::debug) { - //InfoInFunction + // InfoInFunction Pout<< FUNCTION_NAME << " : reading directory " << directory << endl; if ((POSIX::debug & 2) && !Pstream::master()) { @@ -744,22 +766,31 @@ Foam::fileNameList Foam::readDir } } - label nEntries = 0; - fileNameList dirEntries(maxNnames); + label nFailed = 0; // Entries with invalid characters + label nEntries = 0; // Number of selected entries + dirEntries.setSize(maxNnames); // Read and parse all the entries in the directory for (struct dirent *list; (list = ::readdir(source)) != nullptr; /*nil*/) { - const fileName name(list->d_name); + const std::string item(list->d_name); // Ignore files/directories beginning with "." // These are the ".", ".." directories and any hidden files/dirs - if (name.empty() || name[0] == '.') + if (item.empty() || item[0] == '.') { continue; } - if + // Validate filename without spaces, quotes, etc in the name. + // No duplicate slashes to strip - dirent will not have them anyhow. + + const fileName name(fileName::validate(item)); + if (name != item) + { + ++nFailed; + } + else if ( (type == fileName::DIRECTORY) || (type == fileName::FILE && !isBackupName(name)) @@ -783,10 +814,18 @@ Foam::fileNameList Foam::readDir } } } + ::closedir(source); - // Reset the length of the entries list + // Finalize the length of the entries list dirEntries.setSize(nEntries); - ::closedir(source); + + if (nFailed && POSIX::debug) + { + std::cerr + << "Foam::readDir() : reading directory " << directory << nl + << nFailed << " entries with invalid characters in their name" + << std::endl; + } return dirEntries; } @@ -911,22 +950,22 @@ bool Foam::cp(const fileName& src, const fileName& dest, const bool followLink) } // Copy files - fileNameList contents = readDir(src, fileName::FILE, false, followLink); - forAll(contents, i) + fileNameList files = readDir(src, fileName::FILE, false, followLink); + for (const fileName& item : files) { if (POSIX::debug) { InfoInFunction - << "Copying : " << src/contents[i] - << " to " << destFile/contents[i] << endl; + << "Copying : " << src/item + << " to " << destFile/item << endl; } // File to file. - cp(src/contents[i], destFile/contents[i], followLink); + cp(src/item, destFile/item, followLink); } // Copy sub directories. - fileNameList subdirs = readDir + fileNameList dirs = readDir ( src, fileName::DIRECTORY, @@ -934,17 +973,17 @@ bool Foam::cp(const fileName& src, const fileName& dest, const bool followLink) followLink ); - forAll(subdirs, i) + for (const fileName& item : dirs) { if (POSIX::debug) { InfoInFunction - << "Copying : " << src/subdirs[i] + << "Copying : " << src/item << " to " << destFile << endl; } // Dir to Dir. - cp(src/subdirs[i], destFile, followLink); + cp(src/item, destFile, followLink); } } else @@ -1002,12 +1041,10 @@ bool Foam::ln(const fileName& src, const fileName& dst) { return true; } - else - { - WarningInFunction - << "symlink from " << src << " to " << dst << " failed." << endl; - return false; - } + + WarningInFunction + << "symlink from " << src << " to " << dst << " failed." << endl; + return false; } @@ -1157,14 +1194,20 @@ bool Foam::rmDir(const fileName& directory, const bool silent) label nErrors = 0; for (struct dirent *list; (list = ::readdir(source)) != nullptr; /*nil*/) { - const fileName name(list->d_name); - if (name.empty() || name == "." || name == "..") + const std::string item(list->d_name); + + // Ignore "." and ".." directories + if (item.empty() || item == "." || item == "..") { - // Ignore "." and ".." directories continue; } - const fileName path = directory/name; + // Allow invalid characters (spaces, quotes, etc), + // otherwise we cannot subdirs with these types of names. + // -> const fileName path = directory/name; <- + + const fileName path(fileNameConcat(directory, item)); + if (path.type(false) == fileName::DIRECTORY) { if (!rmDir(path, true)) // Only report errors at the top-level @@ -1548,10 +1591,8 @@ bool Foam::dlSymFound(void* handle, const std::string& symbol) // symbol can be found if there was no error return !::dlerror(); } - else - { - return false; - } + + return false; } @@ -1680,7 +1721,7 @@ Foam::label Foam::allocateThread() } } - label index = threads_.size(); + const label index = threads_.size(); if (POSIX::debug) { Pout<< "allocateThread : new index:" << index << endl; @@ -1750,7 +1791,7 @@ Foam::label Foam::allocateMutex() } } - label index = mutexes_.size(); + const label index = mutexes_.size(); if (POSIX::debug) { diff --git a/src/OpenFOAM/primitives/strings/fileName/fileName.C b/src/OpenFOAM/primitives/strings/fileName/fileName.C index d191c46d5e4e6d27614f29863474d89cbc4a0e00..8ca6388330cd449c69735d63bfa8b3d3d1ad0db6 100644 --- a/src/OpenFOAM/primitives/strings/fileName/fileName.C +++ b/src/OpenFOAM/primitives/strings/fileName/fileName.C @@ -37,20 +37,105 @@ int Foam::fileName::debug(debug::debugSwitch(fileName::typeName, 0)); const Foam::fileName Foam::fileName::null; +// * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * // + +Foam::fileName Foam::fileName::validate +( + const std::string& s, + const bool doClean +) +{ + fileName out; + out.resize(s.size()); + + char prev = 0; + std::string::size_type count = 0; + + // Largely as per stripInvalid + for (auto iter = s.cbegin(); iter != s.cend(); ++iter) + { + const char c = *iter; + + if (fileName::valid(c)) + { + if (doClean && prev == '/' && c == '/') + { + // Avoid repeated '/'; + continue; + } + + // Only track valid chars + out[count++] = prev = c; + } + } + + if (doClean && prev == '/' && count > 1) + { + // Avoid trailing '/' + --count; + } + + out.resize(count); + + return out; +} + + +bool Foam::fileName::equals(const std::string& s1, const std::string& s2) +{ + // Do not use (s1 == s2) or s1.compare(s2) first since this would + // potentially be doing the comparison twice. + + std::string::size_type i1 = 0; + std::string::size_type i2 = 0; + + const auto n1 = s1.size(); + const auto n2 = s2.size(); + + //Info<< "compare " << s1 << " == " << s2 << endl; + while (i1 < n1 && i2 < n2) + { + //Info<< "check '" << s1[i1] << "' vs '" << s2[i2] << "'" << endl; + + if (s1[i1] != s2[i2]) + { + return false; + } + + // Increment to next positions and also skip repeated slashes + do + { + ++i1; + } + while (s1[i1] == '/'); + + do + { + ++i2; + } + while (s2[i2] == '/'); + } + //Info<< "return: " << Switch(i1 == n1 && i2 == n2) << endl; + + // Equal if it made it all the way through both strings + return (i1 == n1 && i2 == n2); +} + + // * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * // Foam::fileName::fileName(const UList<word>& lst) { // Estimate overall size size_type sz = lst.size(); // Approx number of '/' needed - for (const auto& item : lst) + for (const word& item : lst) { sz += item.size(); } reserve(sz); sz = 0; - for (const auto& item : lst) + for (const word& item : lst) { if (item.size()) { @@ -65,14 +150,14 @@ Foam::fileName::fileName(std::initializer_list<word> lst) { // Estimate overall size size_type sz = lst.size(); // Approx number of '/' needed - for (const auto& item : lst) + for (const word& item : lst) { sz += item.size(); } reserve(sz); sz = 0; - for (const auto& item : lst) + for (const word& item : lst) { if (item.size()) { @@ -217,10 +302,8 @@ std::string Foam::fileName::name(const std::string& str) { return str; } - else - { - return str.substr(beg+1); - } + + return str.substr(beg+1); } @@ -253,10 +336,8 @@ std::string Foam::fileName::nameLessExt(const std::string& str) { return str.substr(beg); } - else - { - return str.substr(beg, dot - beg); - } + + return str.substr(beg, dot - beg); } @@ -299,10 +380,8 @@ Foam::fileName Foam::fileName::lessExt() const { return *this; } - else - { - return substr(0, i); - } + + return substr(0, i); } @@ -407,28 +486,26 @@ void Foam::fileName::operator=(const char* str) Foam::fileName Foam::operator/(const string& a, const string& b) { - if (a.size()) // First string non-null + if (a.size()) { - if (b.size()) // Second string non-null + if (b.size()) { + // Two non-empty strings: can concatenate return fileName(a + '/' + b); } - else // Second string null - { - return a; - } + + return a; } - else // First string null + + // Or, if the first string is empty + + if (b.size()) { - if (b.size()) // Second string non-null - { - return b; - } - else // Second string null - { - return fileName(); - } + return b; } + + // Both strings are empty + return fileName(); } @@ -438,19 +515,19 @@ Foam::fileName Foam::search(const word& file, const fileName& directory) { // Search the current directory for the file fileNameList files(fileHandler().readDir(directory)); - forAll(files, i) + for (const fileName& item : files) { - if (files[i] == file) + if (item == file) { - return directory/file; + return directory/item; } } // If not found search each of the sub-directories fileNameList dirs(fileHandler().readDir(directory, fileName::DIRECTORY)); - forAll(dirs, i) + for (const fileName& item : dirs) { - fileName path = search(file, directory/dirs[i]); + fileName path = search(file, directory/item); if (path != fileName::null) { return path; diff --git a/src/OpenFOAM/primitives/strings/fileName/fileName.H b/src/OpenFOAM/primitives/strings/fileName/fileName.H index a0a3ece3ce8db34de9b568c5285af945e5a53e0f..8d8d7999b97f90d8c488a24332b3fbde3d7c91bc 100644 --- a/src/OpenFOAM/primitives/strings/fileName/fileName.H +++ b/src/OpenFOAM/primitives/strings/fileName/fileName.H @@ -135,6 +135,20 @@ public: //- Is this character valid for a fileName? inline static bool valid(char c); + //- Construct validated fileName (no invalid characters). + // Optionally perform some additional cleanup such as removing + // duplicate or trailing slashes. + static fileName validate + ( + const std::string& s, + const bool doClean=false + ); + + //- This is a specialized (possibly slower) version of compare() + // that ignores duplicate or trailing slashes. + static bool equals(const std::string& s1, const std::string& s2); + + //- Cleanup filename // // Removes trailing \c /