From db95591645a745767fc7b9a77940e579caa1483c Mon Sep 17 00:00:00 2001
From: Malte Langkabel <malte.langkabel@gmail.com>
Date: Thu, 13 Feb 2020 00:29:15 +0100
Subject: [PATCH] util: Fix FilePath::isValid() and add tests (#908)

* fixed bug that caused FilePath to be invalid for files with extension
* added tests for FilePath::isValid()
fixes #907
---
 src/lib/utility/file/FilePath.cpp | 12 +++++-------
 src/test/FilePathTestSuite.cpp    | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/lib/utility/file/FilePath.cpp b/src/lib/utility/file/FilePath.cpp
index 80ed3308..27bb55f5 100644
--- a/src/lib/utility/file/FilePath.cpp
+++ b/src/lib/utility/file/FilePath.cpp
@@ -115,17 +115,19 @@ bool FilePath::isAbsolute() const
 
 bool FilePath::isValid() const
 {
+	boost::filesystem::path::iterator end = m_path->end();
+
 	if (!isDirectory())
 	{
 		if (!boost::filesystem::portable_file_name(m_path->filename().generic_string()))
 		{
 			return false;
 		}
+		end--;
 	}
 
 	boost::filesystem::path::iterator it = m_path->begin();
 
-#if WIN32
 	if (isAbsolute() && m_path->has_root_path())
 	{
 		std::string root = m_path->root_path().string();
@@ -136,14 +138,10 @@ bool FilePath::isValid() const
 			it++;
 		}
 	}
-#else
-	return true; // FIXME: hot fix for issue #907
-#endif
 
-	for (; it != m_path->end(); ++it)
+	for (; it != end; ++it)
 	{
-		std::string ss = it->string();
-		if (!boost::filesystem::portable_directory_name(ss))
+		if (!boost::filesystem::portable_directory_name(it->string()))
 		{
 			return false;
 		}
diff --git a/src/test/FilePathTestSuite.cpp b/src/test/FilePathTestSuite.cpp
index 8602a8b7..586ccf15 100644
--- a/src/test/FilePathTestSuite.cpp
+++ b/src/test/FilePathTestSuite.cpp
@@ -210,3 +210,27 @@ TEST_CASE("file_path_differs_for_existing_and_nonexisting_files")
 
 	REQUIRE(pathA != pathB);
 }
+
+TEST_CASE("file path is valid for absolute and relative existing files and directories paths")
+{
+	REQUIRE(FilePath(L"data/FilePathTestSuite/a.cpp").isValid());
+	REQUIRE(FilePath(L"data/FilePathTestSuite/a.cpp").makeAbsolute().isValid());
+	REQUIRE(FilePath(L"data/FilePathTestSuite").isValid());
+	REQUIRE(FilePath(L"data/FilePathTestSuite").makeAbsolute().isValid());
+}
+
+TEST_CASE("file path is valid for absolute and relative non-existing files and directories paths")
+{
+	REQUIRE(FilePath(L"data/non-existing-file.cpp").isValid());
+	REQUIRE(FilePath(L"data/non-existing-file.cpp").makeAbsolute().isValid());
+	REQUIRE(FilePath(L"data/non-existing-dir").isValid());
+	REQUIRE(FilePath(L"data/non-existing-dir").makeAbsolute().isValid());
+}
+
+TEST_CASE("file path is invalid for absolute and relative paths with invalid characters")
+{
+	REQUIRE(!FilePath(L"data/non-exis\"ting-file.cpp").isValid());
+	REQUIRE(!FilePath(L"data/non-exis\"ting-file.cpp").makeAbsolute().isValid());
+	REQUIRE(!FilePath(L"data/non-exis|ting-dir").isValid());
+	REQUIRE(!FilePath(L"data/non-exis|ting-dir").makeAbsolute().isValid());
+}