Merge pull request #11579 from ethereum/better-errors-about-bad-paths-in-tests-with-external-sources

Better errors about bad paths in tests with external sources

libsolutil/CommonIO.cpp

@@ -43,9 +43,17 @@ namespace
 template <typename T>
 inline T readFile(std::string const& _file)
 {
+	assertThrow(boost::filesystem::exists(_file), FileNotFound, _file);
+
+	// ifstream does not always fail when the path leads to a directory. Instead it might succeed
+	// with tellg() returning a nonsensical value so that std::length_error gets raised in resize().
+	assertThrow(boost::filesystem::is_regular_file(_file), NotAFile, _file);
+
 	T ret;
 	size_t const c_elementSize = sizeof(typename T::value_type);
 	std::ifstream is(_file, std::ifstream::binary);
+
+	// Technically, this can still fail even though we checked above because FS content can change at any time.
 	assertThrow(is, FileNotFound, _file);
 
 	// get length of file:

libsolutil/CommonIO.h

@@ -33,6 +33,7 @@ namespace solidity::util
 
 /// Retrieve and returns the contents of the given file as a std::string.
 /// If the file doesn't exist, it will throw a FileNotFound exception.
+/// If the file exists but is not a regular file, it will throw NotAFile exception.
 /// If the file is empty, returns an empty string.
 std::string readFileAsString(std::string const& _file);
 

libsolutil/Exceptions.h

@@ -49,6 +49,7 @@ DEV_SIMPLE_EXCEPTION(InvalidAddress);
 DEV_SIMPLE_EXCEPTION(BadHexCharacter);
 DEV_SIMPLE_EXCEPTION(BadHexCase);
 DEV_SIMPLE_EXCEPTION(FileNotFound);
+DEV_SIMPLE_EXCEPTION(NotAFile);
 DEV_SIMPLE_EXCEPTION(DataTooLong);
 DEV_SIMPLE_EXCEPTION(StringTooLong);
 

solc/CommandLineInterface.cpp

@@ -668,6 +668,10 @@ bool CommandLineInterface::parseLibraryOption(string const& _input)
 	{
 		// Should not happen if `fs::is_regular_file` is correct.
 	}
+	catch (NotAFile const&)
+	{
+		// Should not happen if `fs::is_regular_file` is correct.
+	}
 
 	vector<string> libraries;
 	boost::split(libraries, data, boost::is_space() || boost::is_any_of(","), boost::token_compress_on);
@@ -1263,6 +1267,11 @@ bool CommandLineInterface::processInput()
 				serr() << "File not found: " << jsonFile << endl;
 				return false;
 			}
+			catch (NotAFile const&)
+			{
+				serr() << "Not a regular file: " << jsonFile << endl;
+				return false;
+			}
 		}
 		StandardCompiler compiler(m_fileReader.reader());
 		sout() << compiler.compile(std::move(input)) << endl;

test/CMakeLists.txt

@@ -8,6 +8,8 @@ set(sources
     EVMHost.h
     ExecutionFramework.cpp
     ExecutionFramework.h
+    FilesystemUtils.cpp
+    FilesystemUtils.h
     InteractiveTests.h
     Metadata.cpp
     Metadata.h
@@ -31,6 +33,7 @@ detect_stray_source_files("${contracts_sources}" "contracts/")
 set(libsolutil_sources
     libsolutil/Checksum.cpp
     libsolutil/CommonData.cpp
+    libsolutil/CommonIO.cpp
     libsolutil/FixedHash.cpp
     libsolutil/IndentedWriter.cpp
     libsolutil/IpfsHash.cpp

test/FilesystemUtils.cpp

@@ -0,0 +1,60 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+#include <test/FilesystemUtils.h>
+
+#include <test/libsolidity/util/SoltestErrors.h>
+
+using namespace std;
+using namespace solidity;
+using namespace solidity::test;
+
+void solidity::test::createFileWithContent(boost::filesystem::path const& _path, string const& content)
+{
+	if (boost::filesystem::is_regular_file(_path))
+		BOOST_THROW_EXCEPTION(runtime_error("File already exists: \"" + _path.string() + "\".")); \
+
+	// Use binary mode to avoid line ending conversion on Windows.
+	ofstream newFile(_path.string(), std::ofstream::binary);
+	if (newFile.fail() || !boost::filesystem::is_regular_file(_path))
+		BOOST_THROW_EXCEPTION(runtime_error("Failed to create a file: \"" + _path.string() + "\".")); \
+
+	newFile << content;
+}
+
+bool solidity::test::createSymlinkIfSupportedByFilesystem(
+	boost::filesystem::path const& _targetPath,
+	boost::filesystem::path const& _linkName
+)
+{
+	boost::system::error_code symlinkCreationError;
+	boost::filesystem::create_symlink(_targetPath, _linkName, symlinkCreationError);
+
+	if (!symlinkCreationError)
+		return true;
+	else if (
+		symlinkCreationError == boost::system::errc::not_supported ||
+		symlinkCreationError == boost::system::errc::operation_not_supported
+	)
+		return false;
+	else
+		BOOST_THROW_EXCEPTION(runtime_error(
+			"Failed to create a symbolic link: \"" + _linkName.string() + "\""
+			" -> " + _targetPath.string() + "\"."
+		));
+}

test/FilesystemUtils.h

@@ -0,0 +1,45 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+/**
+ * Helpers for common filesystem operations used in multiple tests.
+ */
+
+#pragma once
+
+#include <boost/filesystem.hpp>
+
+#include <string>
+
+namespace solidity::test
+{
+
+/// Creates a file with the exact content specified in the second argument.
+/// Throws an exception if the file already exists or if the parent directory of the file does not.
+void createFileWithContent(boost::filesystem::path const& _path, std::string const& content);
+
+/// Creates a symlink between two paths.
+/// The target does not have to exist.
+/// @returns true if the symlink has been successfully created, false if the filesystem does not
+/// support symlinks.
+/// Throws an exception of the operation fails for a different reason.
+bool createSymlinkIfSupportedByFilesystem(
+	boost::filesystem::path const& _targetPath,
+	boost::filesystem::path const& _linkName
+);
+
+}

test/TestCaseReader.cpp

@@ -159,7 +159,9 @@ pair<SourceMap, size_t> TestCaseReader::parseSourcesAndSettingsWithLineNumber(is
 				soltestAssert(!externalSourceName.empty(), "");
 				fs::path externalSourceTarget(externalSourceString);
 				fs::path testCaseParentDir = m_fileName.parent_path();
-				if (!externalSourceTarget.is_relative())
+				if (!externalSourceTarget.is_relative() || !externalSourceTarget.root_path().empty())
+					// NOTE: UNC paths (ones starting with // or \\) are considered relative by Boost
+					// since they have an empty root directory (but non-empty root name).
 					BOOST_THROW_EXCEPTION(runtime_error("External Source paths need to be relative to the location of the test case."));
 				fs::path externalSourceFullPath = testCaseParentDir / externalSourceTarget;
 				string externalSourceContent;

test/libsolutil/CommonData.cpp

@@ -16,7 +16,7 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 /**
- * Unit tests for the StringUtils routines.
+ * Unit tests for the CommonData routines.
  */
 
 #include <libsolutil/Common.h>

test/libsolutil/CommonIO.cpp

@@ -0,0 +1,69 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+*/
+// SPDX-License-Identifier: GPL-3.0
+
+/// Unit tests for the CommonIO routines.
+
+#include <libsolutil/CommonIO.h>
+
+#include <test/Common.h>
+#include <test/FilesystemUtils.h>
+#include <test/TemporaryDirectory.h>
+#include <test/libsolidity/util/SoltestErrors.h>
+
+#include <boost/test/unit_test.hpp>
+#include <boost/filesystem.hpp>
+
+#include <fstream>
+#include <string>
+
+using namespace std;
+using namespace solidity::test;
+
+namespace solidity::util::test
+{
+
+BOOST_AUTO_TEST_SUITE(CommonIOTest)
+
+BOOST_AUTO_TEST_CASE(readFileAsString_regular_file)
+{
+	TemporaryDirectory tempDir("common-io-test-");
+	createFileWithContent(tempDir.path() / "test.txt", "ABC\ndef\n");
+
+	BOOST_TEST(readFileAsString((tempDir.path() / "test.txt").string()) == "ABC\ndef\n");
+}
+
+BOOST_AUTO_TEST_CASE(readFileAsString_directory)
+{
+	TemporaryDirectory tempDir("common-io-test-");
+	BOOST_CHECK_THROW(readFileAsString(tempDir.path().string()), NotAFile);
+}
+
+BOOST_AUTO_TEST_CASE(readFileAsString_symlink)
+{
+	TemporaryDirectory tempDir("common-io-test-");
+	createFileWithContent(tempDir.path() / "test.txt", "ABC\ndef\n");
+
+	if (!createSymlinkIfSupportedByFilesystem("test.txt", tempDir.path() / "symlink.txt"))
+		return;
+
+	BOOST_TEST(readFileAsString((tempDir.path() / "symlink.txt").string()) == "ABC\ndef\n");
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
+} // namespace solidity::util::test

test/tools/yulopti.cpp

@@ -282,6 +282,11 @@ Allowed options)",
 		cerr << "File not found:" << _exception.comment() << endl;
 		return 1;
 	}
+	catch (NotAFile const& _exception)
+	{
+		cerr << "Not a regular file:" << _exception.comment() << endl;
+		return 1;
+	}
 
 	if (arguments.count("input-file"))
 		YulOpti{}.runInteractive(input);

test/tools/yulrun.cpp

@@ -148,6 +148,11 @@ Allowed options)",
 					cerr << "File not found: " << path << endl;
 					return 1;
 				}
+				catch (NotAFile const&)
+				{
+					cerr << "Not a regular file: " << path << endl;
+					return 1;
+				}
 			}
 		else
 			input = readStandardInput();