Merge pull request #664 from cryptoknight/WriteOnCopy

Don't automatically make copied files writable on Windows

Foundation/src/File_WIN32.cpp

@@ -34,17 +34,17 @@ public:
 			FileImpl::handleLastErrorImpl(path);
 		}
 	}
-	
+
 	~FileHandle()
 	{
 		if (_h != INVALID_HANDLE_VALUE) CloseHandle(_h);
 	}
-	
+
 	HANDLE get() const
 	{
 		return _h;
 	}
-	
+
 private:
 	HANDLE _h;
 };
@@ -112,7 +112,7 @@ bool FileImpl::existsImpl() const
 bool FileImpl::canReadImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributes(_path.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 	{
@@ -131,7 +131,7 @@ bool FileImpl::canReadImpl() const
 bool FileImpl::canWriteImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributes(_path.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 		handleLastErrorImpl(_path);
@@ -201,7 +201,7 @@ Timestamp FileImpl::createdImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftCreationTime.dwLowDateTime, fad.ftCreationTime.dwHighDateTime);
 }
@@ -212,7 +212,7 @@ Timestamp FileImpl::getLastModifiedImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftLastWriteTime.dwLowDateTime, fad.ftLastWriteTime.dwHighDateTime);
 }
@@ -239,7 +239,7 @@ FileImpl::FileSizeImpl FileImpl::getSizeImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesEx(_path.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	LARGE_INTEGER li;
 	li.LowPart  = fad.nFileSizeLow;
@@ -257,7 +257,7 @@ void FileImpl::setSizeImpl(FileSizeImpl size)
 	li.QuadPart = size;
 	if (SetFilePointer(fh.get(), li.LowPart, &li.HighPart, FILE_BEGIN) == INVALID_SET_FILE_POINTER)
 		handleLastErrorImpl(_path);
-	if (SetEndOfFile(fh.get()) == 0) 
+	if (SetEndOfFile(fh.get()) == 0)
 		handleLastErrorImpl(_path);
 }
 
@@ -288,12 +288,8 @@ void FileImpl::copyToImpl(const std::string& path) const
 {
 	poco_assert (!_path.empty());
 
-	if (CopyFileA(_path.c_str(), path.c_str(), FALSE) != 0) 
+	if (CopyFileA(_path.c_str(), path.c_str(), FALSE) == 0)
-	{
+		handleLastErrorImpl(_path);
-		FileImpl copy(path);
-		copy.setWriteableImpl(true);
-	}
-	else handleLastErrorImpl(_path);
 }
 
 
@@ -301,7 +297,7 @@ void FileImpl::renameToImpl(const std::string& path)
 {
 	poco_assert (!_path.empty());
 
-	if (MoveFileA(_path.c_str(), path.c_str()) == 0) 
+	if (MoveFileA(_path.c_str(), path.c_str()) == 0)
 		handleLastErrorImpl(_path);
 }
 
@@ -312,7 +308,7 @@ void FileImpl::removeImpl()
 
 	if (isDirectoryImpl())
 	{
-		if (RemoveDirectoryA(_path.c_str()) == 0) 
+		if (RemoveDirectoryA(_path.c_str()) == 0)
 			handleLastErrorImpl(_path);
 	}
 	else
@@ -344,7 +340,7 @@ bool FileImpl::createFileImpl()
 bool FileImpl::createDirectoryImpl()
 {
 	poco_assert (!_path.empty());
-	
+
 	if (existsImpl() && isDirectoryImpl())
 		return false;
 	if (CreateDirectoryA(_path.c_str(), 0) == 0)

Foundation/src/File_WIN32U.cpp

@@ -35,17 +35,17 @@ public:
 			FileImpl::handleLastErrorImpl(path);
 		}
 	}
-	
+
 	~FileHandle()
 	{
 		if (_h != INVALID_HANDLE_VALUE) CloseHandle(_h);
 	}
-	
+
 	HANDLE get() const
 	{
 		return _h;
 	}
-	
+
 private:
 	HANDLE _h;
 };
@@ -116,7 +116,7 @@ bool FileImpl::existsImpl() const
 bool FileImpl::canReadImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributesW(_upath.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 	{
@@ -135,7 +135,7 @@ bool FileImpl::canReadImpl() const
 bool FileImpl::canWriteImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributesW(_upath.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 		handleLastErrorImpl(_path);
@@ -205,7 +205,7 @@ Timestamp FileImpl::createdImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftCreationTime.dwLowDateTime, fad.ftCreationTime.dwHighDateTime);
 }
@@ -216,7 +216,7 @@ Timestamp FileImpl::getLastModifiedImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftLastWriteTime.dwLowDateTime, fad.ftLastWriteTime.dwHighDateTime);
 }
@@ -243,7 +243,7 @@ FileImpl::FileSizeImpl FileImpl::getSizeImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	LARGE_INTEGER li;
 	li.LowPart  = fad.nFileSizeLow;
@@ -261,12 +261,12 @@ void FileImpl::setSizeImpl(FileSizeImpl size)
 	li.QuadPart = size;
 	if (SetFilePointer(fh.get(), li.LowPart, &li.HighPart, FILE_BEGIN) == INVALID_SET_FILE_POINTER)
 		handleLastErrorImpl(_path);
-	if (SetEndOfFile(fh.get()) == 0) 
+	if (SetEndOfFile(fh.get()) == 0)
 		handleLastErrorImpl(_path);
 }
 
 
-void FileImpl::setWriteableImpl(bool flag)		
+void FileImpl::setWriteableImpl(bool flag)
 {
 	poco_assert (!_path.empty());
 
@@ -294,12 +294,8 @@ void FileImpl::copyToImpl(const std::string& path) const
 
 	std::wstring upath;
 	UnicodeConverter::toUTF16(path, upath);
-	if (CopyFileW(_upath.c_str(), upath.c_str(), FALSE) != 0)
+	if (CopyFileW(_upath.c_str(), upath.c_str(), FALSE) == 0)
-	{
+		handleLastErrorImpl(_path);
-		FileImpl copy(path);
-		copy.setWriteableImpl(true);
-	}
-	else handleLastErrorImpl(_path);
 }
 
 
@@ -309,7 +305,7 @@ void FileImpl::renameToImpl(const std::string& path)
 
 	std::wstring upath;
 	UnicodeConverter::toUTF16(path, upath);
-	if (MoveFileW(_upath.c_str(), upath.c_str()) == 0) 
+	if (MoveFileW(_upath.c_str(), upath.c_str()) == 0)
 		handleLastErrorImpl(_path);
 }
 
@@ -320,7 +316,7 @@ void FileImpl::removeImpl()
 
 	if (isDirectoryImpl())
 	{
-		if (RemoveDirectoryW(_upath.c_str()) == 0) 
+		if (RemoveDirectoryW(_upath.c_str()) == 0)
 			handleLastErrorImpl(_path);
 	}
 	else
@@ -352,7 +348,7 @@ bool FileImpl::createFileImpl()
 bool FileImpl::createDirectoryImpl()
 {
 	poco_assert (!_path.empty());
-	
+
 	if (existsImpl() && isDirectoryImpl())
 		return false;
 	if (CreateDirectoryW(_upath.c_str(), 0) == 0)

Foundation/src/File_WINCE.cpp

@@ -36,17 +36,17 @@ public:
 			FileImpl::handleLastErrorImpl(path);
 		}
 	}
-	
+
 	~FileHandle()
 	{
 		if (_h != INVALID_HANDLE_VALUE) CloseHandle(_h);
 	}
-	
+
 	HANDLE get() const
 	{
 		return _h;
 	}
-	
+
 private:
 	HANDLE _h;
 };
@@ -117,7 +117,7 @@ bool FileImpl::existsImpl() const
 bool FileImpl::canReadImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributesW(_upath.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 	{
@@ -136,7 +136,7 @@ bool FileImpl::canReadImpl() const
 bool FileImpl::canWriteImpl() const
 {
 	poco_assert (!_path.empty());
-	
+
 	DWORD attr = GetFileAttributesW(_upath.c_str());
 	if (attr == INVALID_FILE_ATTRIBUTES)
 		handleLastErrorImpl(_path);
@@ -196,7 +196,7 @@ Timestamp FileImpl::createdImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftCreationTime.dwLowDateTime, fad.ftCreationTime.dwHighDateTime);
 }
@@ -207,7 +207,7 @@ Timestamp FileImpl::getLastModifiedImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	return Timestamp::fromFileTimeNP(fad.ftLastWriteTime.dwLowDateTime, fad.ftLastWriteTime.dwHighDateTime);
 }
@@ -234,7 +234,7 @@ FileImpl::FileSizeImpl FileImpl::getSizeImpl() const
 	poco_assert (!_path.empty());
 
 	WIN32_FILE_ATTRIBUTE_DATA fad;
-	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0) 
+	if (GetFileAttributesExW(_upath.c_str(), GetFileExInfoStandard, &fad) == 0)
 		handleLastErrorImpl(_path);
 	LARGE_INTEGER li;
 	li.LowPart  = fad.nFileSizeLow;
@@ -252,12 +252,12 @@ void FileImpl::setSizeImpl(FileSizeImpl size)
 	li.QuadPart = size;
 	if (SetFilePointer(fh.get(), li.LowPart, &li.HighPart, FILE_BEGIN) == INVALID_SET_FILE_POINTER)
 		handleLastErrorImpl(_path);
-	if (SetEndOfFile(fh.get()) == 0) 
+	if (SetEndOfFile(fh.get()) == 0)
 		handleLastErrorImpl(_path);
 }
 
 
-void FileImpl::setWriteableImpl(bool flag)		
+void FileImpl::setWriteableImpl(bool flag)
 {
 	poco_assert (!_path.empty());
 
@@ -285,12 +285,8 @@ void FileImpl::copyToImpl(const std::string& path) const
 
 	std::wstring upath;
 	UnicodeConverter::toUTF16(path, upath);
-	if (CopyFileW(_upath.c_str(), upath.c_str(), FALSE) != 0)
+	if (CopyFileW(_upath.c_str(), upath.c_str(), FALSE) == 0)
-	{
+		handleLastErrorImpl(_path);
-		FileImpl copy(path);
-		copy.setWriteableImpl(true);
-	}
-	else handleLastErrorImpl(_path);
 }
 
 
@@ -300,7 +296,7 @@ void FileImpl::renameToImpl(const std::string& path)
 
 	std::wstring upath;
 	UnicodeConverter::toUTF16(path, upath);
-	if (MoveFileW(_upath.c_str(), upath.c_str()) == 0) 
+	if (MoveFileW(_upath.c_str(), upath.c_str()) == 0)
 		handleLastErrorImpl(_path);
 }
 
@@ -311,7 +307,7 @@ void FileImpl::removeImpl()
 
 	if (isDirectoryImpl())
 	{
-		if (RemoveDirectoryW(_upath.c_str()) == 0) 
+		if (RemoveDirectoryW(_upath.c_str()) == 0)
 			handleLastErrorImpl(_path);
 	}
 	else
@@ -343,7 +339,7 @@ bool FileImpl::createFileImpl()
 bool FileImpl::createDirectoryImpl()
 {
 	poco_assert (!_path.empty());
-	
+
 	if (existsImpl() && isDirectoryImpl())
 		return false;
 	if (CreateDirectoryW(_upath.c_str(), 0) == 0)

Foundation/testsuite/src/FileTest.cpp

@@ -45,7 +45,7 @@ void FileTest::testFileAttributes1()
 {
 	File f("testfile.dat");
 	assert (!f.exists());
-	
+
 	try
 	{
 		bool flag = f.canRead();
@@ -201,7 +201,7 @@ void FileTest::testFileAttributes2()
 	bool created = f.createFile();
 	Timestamp ts;
 	assert (created);
-	
+
 	assert (f.exists());
 	assert (f.canRead());
 	assert (f.canWrite());
@@ -211,15 +211,15 @@ void FileTest::testFileAttributes2()
 	Timestamp tsm = f.getLastModified();
 	assert (tsc - ts >= -2000000 && tsc - ts <= 2000000);
 	assert (tsm - ts >= -2000000 && tsm - ts <= 2000000);
-	
+
 	f.setWriteable(false);
 	assert (!f.canWrite());
 	assert (f.canRead());
 
-	f.setReadOnly(false);	
+	f.setReadOnly(false);
 	assert (f.canWrite());
 	assert (f.canRead());
-	
+
 	ts = Timestamp::fromEpochTime(1000000);
 	f.setLastModified(ts);
 	assert (f.getLastModified() == ts);
@@ -247,7 +247,7 @@ void FileTest::testCompare()
 	File f1("abc.txt");
 	File f2("def.txt");
 	File f3("abc.txt");
-	
+
 	assert (f1 == f3);
 	assert (!(f1 == f2));
 	assert (f1 != f2);
@@ -261,7 +261,7 @@ void FileTest::testCompare()
 	assert (f2 >= f1);
 	assert (!(f1 > f2));
 	assert (!(f1 >= f2));
-	
+
 	assert (f1 <= f3);
 	assert (f1 >= f3);
 }
@@ -325,7 +325,7 @@ void FileTest::testDirectory()
 	{
 	}
 	TemporaryFile::registerForDeletion("testdir");
-	
+
 	bool created = d.createDirectory();
 	assert (created);
 	assert (d.isDirectory());
@@ -333,23 +333,23 @@ void FileTest::testDirectory()
 	std::vector<std::string> files;
 	d.list(files);
 	assert (files.empty());
-	
+
 	File f = Path("testdir/file1", Path::PATH_UNIX);
 	f.createFile();
 	f = Path("testdir/file2", Path::PATH_UNIX);
 	f.createFile();
 	f = Path("testdir/file3", Path::PATH_UNIX);
 	f.createFile();
-	
+
 	d.list(files);
 	assert (files.size() == 3);
-	
+
 	std::set<std::string> fs;
 	fs.insert(files.begin(), files.end());
 	assert (fs.find("file1") != fs.end());
 	assert (fs.find("file2") != fs.end());
 	assert (fs.find("file3") != fs.end());
-	
+
 	File dd(Path("testdir/testdir2/testdir3", Path::PATH_UNIX));
 	dd.createDirectories();
 	assert (dd.exists());
@@ -359,7 +359,7 @@ void FileTest::testDirectory()
 	ddd.createDirectories();
 	assert (ddd.exists());
 	assert (ddd.isDirectory());
-	
+
 	d.remove(true);
 }
 
@@ -372,9 +372,11 @@ void FileTest::testCopy()
 
 	File f1("testfile.dat");
 	TemporaryFile f2;
-	f1.copyTo(f2.path());
+	f1.setReadOnly().copyTo(f2.path());
 	assert (f2.exists());
+	assert (!f2.canWrite());
 	assert (f1.getSize() == f2.getSize());
+	f1.setWriteable().remove();
 }
 
 
@@ -422,9 +424,9 @@ void FileTest::testCopyDirectory()
 	std::ofstream ostr3(pf3.toString().c_str());
 	ostr3 << "Hello, world!" << std::endl;
 	ostr3.close();
-	
+
 	File fd3("testdir2");
-	
+
 	try
 	{
 		fd3.remove(true);
@@ -432,19 +434,19 @@ void FileTest::testCopyDirectory()
 	catch (...)
 	{
 	}
-	
+
 	fd1.copyTo("testdir2");
-	
+
 	Path pd1t("testdir2");
 	File fd1t(pd1t);
 	assert (fd1t.exists());
 	assert (fd1t.isDirectory());
-	
+
 	Path pd2t(pd1t, "subdir");
 	File fd2t(pd2t);
 	assert (fd2t.exists());
 	assert (fd2t.isDirectory());
-	
+
 	Path pf1t(pd1t, "testfile1.dat");
 	File ff1t(pf1t);
 	assert (ff1t.exists());
@@ -459,7 +461,7 @@ void FileTest::testCopyDirectory()
 	File ff3t(pf3t);
 	assert (ff3t.exists());
 	assert (ff3t.isFile());
-	
+
 	fd1.remove(true);
 	fd3.remove(true);
 }
@@ -478,7 +480,7 @@ void FileTest::testRename()
 	assert (f2.exists());
 	assert (f1.exists());
 	assert (f1 == f2);
-	
+
 	f2.remove();
 }