From 5bd7be6ed10d99b5d7f9776616cffc95508a5b28 Mon Sep 17 00:00:00 2001 From: James Turner Date: Thu, 26 Jan 2017 18:01:19 +0000 Subject: [PATCH] Check for file permissions via SGPath. --- simgear/misc/path_test.cxx | 122 ++++++++++++++++++++++++++++++++----- simgear/misc/sg_path.cxx | 69 ++++++++++++++++----- simgear/misc/sg_path.hxx | 2 + 3 files changed, 163 insertions(+), 30 deletions(-) diff --git a/simgear/misc/path_test.cxx b/simgear/misc/path_test.cxx index a544fa9f..2f96817d 100644 --- a/simgear/misc/path_test.cxx +++ b/simgear/misc/path_test.cxx @@ -14,6 +14,12 @@ using std::endl; #include #include +#if defined(SG_WINDOWS) + #include // for _wchmod +#else + #include // for chmod +#endif + void test_dir() { simgear::Dir temp = simgear::Dir::tempDir("foo"); @@ -134,6 +140,104 @@ void test_path_dir() } +void test_permissions() +{ + // start with an empty dir + SGPath p = simgear::Dir::current().path() / "path_test_permissions"; + simgear::Dir pd(p); + if (pd.exists()) { + pd.removeChildren(); + } else { + pd.create(0700); + } + + // windows doesn't seem to actualy create a read-only directory, so this + // test fails in strange ways there. +#if !defined(SG_WINDOWS) + SGPath fileInRO = p / "read-only" / "fileA"; + SG_CHECK_EQUAL(fileInRO.create_dir(0500), 0); + + SG_VERIFY(!fileInRO.exists()); + SG_VERIFY(!fileInRO.canWrite()); + + fileInRO.setPermissionChecker(&validateRead); + SG_CHECK_EQUAL(fileInRO.canRead(), false); + SG_CHECK_EQUAL(fileInRO.canWrite(), false); + + fileInRO.setPermissionChecker(&validateWrite); + SG_CHECK_EQUAL(fileInRO.canRead(), false); + SG_CHECK_EQUAL(fileInRO.canWrite(), false); +#endif + + ///////// + SGPath fileInRW = p / "read-write" / "fileA"; + + fileInRW.setPermissionChecker(&validateRead); + SG_CHECK_EQUAL(fileInRW.canRead(), false); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); + SG_CHECK_EQUAL(fileInRW.create_dir(0700), -3); // not permitted + + fileInRW.setPermissionChecker(nullptr); + SG_CHECK_EQUAL(fileInRW.create_dir(0700), 0); + + SG_VERIFY(!fileInRW.exists()); + SG_VERIFY(fileInRW.canWrite()); + + fileInRW.setPermissionChecker(&validateRead); + SG_CHECK_EQUAL(fileInRW.canRead(), false); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); + + fileInRW.setPermissionChecker(&validateWrite); + SG_CHECK_EQUAL(fileInRW.canRead(), false); + SG_CHECK_EQUAL(fileInRW.canWrite(), true); + + + // create the file + { + sg_ofstream os(fileInRW); + SG_VERIFY(os.is_open()); + os << "nonense" << endl; + } + + // should now be readable + writeable + fileInRW.set_cached(false); + fileInRW.setPermissionChecker(nullptr); + SG_VERIFY(fileInRW.exists()); + SG_VERIFY(fileInRW.canWrite()); + SG_VERIFY(fileInRW.canRead()); + + fileInRW.setPermissionChecker(&validateRead); + SG_CHECK_EQUAL(fileInRW.canRead(), true); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); + + fileInRW.setPermissionChecker(&validateWrite); + SG_CHECK_EQUAL(fileInRW.canRead(), false); + SG_CHECK_EQUAL(fileInRW.canWrite(), true); + + // mark the file as read-only +#if defined(SG_WINDOWS) + std::wstring wp = fileInRW.wstr(); + _wchmod(wp.c_str(), _S_IREAD); +#else + ::chmod(fileInRW.c_str(), 0400); +#endif + + fileInRW.set_cached(false); + fileInRW.setPermissionChecker(nullptr); + + SG_VERIFY(fileInRW.exists()); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); + SG_VERIFY(fileInRW.canRead()); + + fileInRW.setPermissionChecker(&validateRead); + SG_CHECK_EQUAL(fileInRW.canRead(), true); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); + + fileInRW.setPermissionChecker(&validateWrite); + SG_CHECK_EQUAL(fileInRW.canRead(), false); + SG_CHECK_EQUAL(fileInRW.canWrite(), false); +} + int main(int argc, char* argv[]) { SGPath pa; @@ -229,29 +333,19 @@ int main(int argc, char* argv[]) SG_CHECK_EQUAL(pf.lower_extension(), "gz"); SG_CHECK_EQUAL(pf.complete_lower_extension(), "txt.gz"); - SG_CHECK_EQUAL(pf.canRead(), true); - SG_CHECK_EQUAL(pf.canWrite(), true); + SG_CHECK_EQUAL(pf.canRead(), false); + SG_CHECK_EQUAL(pf.canWrite(), false); SGPath pp(&validateNone); SG_CHECK_EQUAL(pp.canRead(), false); SG_CHECK_EQUAL(pp.canWrite(), false); - pp.append("./test-dir/file.txt"); - SG_CHECK_EQUAL(pp.create_dir(0700), -3); - - pp.setPermissionChecker(&validateRead); - SG_CHECK_EQUAL(pp.canRead(), true); - SG_CHECK_EQUAL(pp.canWrite(), false); - SG_CHECK_EQUAL(pp.create_dir(0700), -3); - - pp.setPermissionChecker(&validateWrite); - SG_CHECK_EQUAL(pp.canRead(), false); - SG_CHECK_EQUAL(pp.canWrite(), true); - test_dir(); test_path_dir(); + test_permissions(); + cout << "all tests passed OK" << endl; return 0; // passed } diff --git a/simgear/misc/sg_path.cxx b/simgear/misc/sg_path.cxx index cea104b2..5c84f8b4 100644 --- a/simgear/misc/sg_path.cxx +++ b/simgear/misc/sg_path.cxx @@ -460,10 +460,11 @@ void SGPath::validate() const if (path.empty()) { _exists = false; + _canWrite = _canRead = false; return; } -#ifdef _WIN32 +#if defined(SG_WINDOWS) struct _stat buf ; bool remove_trailing = false; std::wstring statPath(wstr()); @@ -473,12 +474,24 @@ void SGPath::validate() const if (_wstat(statPath.c_str(), &buf ) < 0) { _exists = false; + _canRead = false; + + // check parent directory for write-ability + std::wstring parentPath = simgear::strutils::convertUtf8ToWString(dir()); + struct _stat parentBuf; + if (_wstat(parentPath.c_str(), &parentBuf) >= 0) { + _canWrite = parentBuf.st_mode & _S_IWRITE; + } else { + _canWrite = false; + } } else { _exists = true; _isFile = ((S_IFREG & buf.st_mode ) !=0); _isDir = ((S_IFDIR & buf.st_mode ) !=0); _modTime = buf.st_mtime; _size = buf.st_size; + _canRead = _S_IREAD & buf.st_mode; + _canWrite = _S_IWRITE & buf.st_mode; } #else @@ -486,15 +499,35 @@ void SGPath::validate() const if (stat(path.c_str(), &buf ) < 0) { _exists = false; + _canRead = false; + + // check parent directory for write-ability + std::string parentPath = dir(); + struct stat parentBuf; + if (stat(parentPath.c_str(), &parentBuf) >= 0) { + _canWrite = parentBuf.st_mode & S_IWUSR; + } else { + _canWrite = false; + } } else { _exists = true; _isFile = ((S_ISREG(buf.st_mode )) != 0); _isDir = ((S_ISDIR(buf.st_mode )) != 0); _modTime = buf.st_mtime; _size = buf.st_size; + _canRead = S_IRUSR & buf.st_mode; + _canWrite = S_IWUSR & buf.st_mode; } #endif + // ensure permissions are no less restrictive than what the + // permissions checker offers + if ( _permission_checker ) { + Permissions p = _permission_checker(*this); + _canRead &= p.read; + _canWrite &= p.write; + } + _cached = true; } @@ -504,18 +537,7 @@ void SGPath::checkAccess() const if( _rwCached && _cacheEnabled ) return; - if( _permission_checker ) - { - Permissions p = _permission_checker(*this); - _canRead = p.read; - _canWrite = p.write; - } - else - { - _canRead = true; - _canWrite = true; - } - + validate(); _rwCached = true; } @@ -555,7 +577,7 @@ bool SGPath::isFile() const int SGPath::create_dir(mode_t mode) { - if( !canWrite() ) + if ( !permissionsAllowsWrite() ) { SG_LOG( SG_IO, SG_WARN, "Error creating directory for '" << *this << "'" @@ -703,7 +725,7 @@ std::string SGPath::str_native() const //------------------------------------------------------------------------------ bool SGPath::remove() { - if( !canWrite() ) + if( !permissionsAllowsWrite() ) { SG_LOG( SG_IO, SG_WARN, "file remove failed: (" << *this << ")" " reason: access denied" ); @@ -712,7 +734,15 @@ bool SGPath::remove() #if defined(SG_WINDOWS) std::wstring ps = wstr(); - int err = _wunlink(ps.c_str()); + + // windows forbids removing a read-only file, let's try to deal + // with that case + int err = _wchmod(ps.c_str(), _S_IWRITE | _S_IREAD); + if (err != 0) { + SG_LOG(SG_IO, SG_WARN, "failed to make file writeable prior to remove:" << *this); + } else { + err = _wunlink(ps.c_str()); + } #else std::string ps = local8BitStr(); int err = ::unlink(ps.c_str()); @@ -1018,3 +1048,10 @@ std::wstring SGPath::wstr() const { return simgear::strutils::convertUtf8ToWString(path); } + +//------------------------------------------------------------------------------ +bool SGPath::permissionsAllowsWrite() const +{ + return _permission_checker ? _permission_checker(*this).write : true; +} + diff --git a/simgear/misc/sg_path.hxx b/simgear/misc/sg_path.hxx index 25a8064c..f874182e 100644 --- a/simgear/misc/sg_path.hxx +++ b/simgear/misc/sg_path.hxx @@ -335,6 +335,8 @@ private: void validate() const; void checkAccess() const; + bool permissionsAllowsWrite() const; + std::string path; PermissionChecker _permission_checker;