diff --git a/Changelog.md b/Changelog.md index 9543bddda..ee2b6ec97 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,8 +7,9 @@ Language Features: Compiler Features: + * Commandline Interface: Add ``--include-path`` option for specifying extra directories that may contain importable code (e.g. packaged third-party libraries). * Commandline Interface: Do not implicitly run evm bytecode generation unless needed for the requested output. - * Commandline Interface: Normalize paths specified on the command line and make them relative for files located inside base path. + * Commandline Interface: Normalize paths specified on the command line and make them relative for files located inside base path and/or include paths. * Immutable variables can be read at construction time once they are initialized. * SMTChecker: Add constraints to better correlate ``address(this).balance`` and ``msg.value``. * SMTChecker: Support constants via modules. diff --git a/docs/path-resolution.rst b/docs/path-resolution.rst index 0ace3a461..40a727068 100644 --- a/docs/path-resolution.rst +++ b/docs/path-resolution.rst @@ -72,8 +72,9 @@ The initial content of the VFS depends on how you invoke the compiler: solc contract.sol /usr/local/dapp-bin/token.sol The source unit name of a file loaded this way is constructed by converting its path to a - canonical form and making it relative to the base path if it is located inside. - See :ref:`Base Path Normalization and Stripping ` for + canonical form and, if possible, making it relative to either the base path or one of the + include paths. + See :ref:`CLI Path Normalization and Stripping ` for a detailed description of this process. .. index:: standard JSON @@ -295,16 +296,36 @@ Here are some examples of what you can expect if they are not: The use of relative imports containing leading ``..`` segments is not recommended. The same effect can be achieved in a more reliable way by using direct imports with - :ref:`base path ` and :ref:`import remapping `. + :ref:`base path and include paths `. -.. index:: ! base path, ! --base-path -.. _base-path: +.. index:: ! base path, ! --base-path, ! include paths, ! --include-path +.. _base-and-include-paths: -Base Path -========= +Base Path and Include Paths +=========================== -The base path specifies the directory that the Host Filesystem Loader will load files from. -It is simply prepended to a source unit name before the filesystem lookup is performed. +The base path and include paths represent directories that the Host Filesystem Loader will load files from. +When a source unit name is passed to the loader, it prepends the base path to it and performs a +filesystem lookup. +If the lookup does not succeed, the same is done with all directories on the include path list. + +It is recommended to set the base path to the root directory of your project and use include paths to +specify additional locations that may contain libraries your project depends on. +This lets you import from these libraries in a uniform way, no matter where they are located in the +filesystem relative to your project. +For example, if you use npm to install packages and your contract imports +``@openzeppelin/contracts/utils/Strings.sol``, you can use these options to tell the compiler that +the library can be found in one of the npm package directories: + +.. code-block:: bash + + solc contract.sol \ + --base-path . \ + --include-path node_modules/ \ + --include-path /usr/local/lib/node_modules/ + +Your contract will compile (with the same exact metadata) no matter whether you install the library +in the local or global package directory or even directly under your project root. By default the base path is empty, which leaves the source unit name unchanged. When the source unit name is a relative path, this results in the file being looked up in the @@ -314,10 +335,23 @@ interpreted as absolute paths on disk. If the base path itself is relative, it is interpreted as relative to the current working directory of the compiler. -.. _base-path-normalization-and-stripping: +.. note:: -Base Path Normalization and Stripping -------------------------------------- + Include paths cannot have empty values and must be used together with a non-empty base path. + +.. note:: + + Include paths and base path can overlap as long as it does not make import resolution ambiguous. + For example, you can specify a directory inside base path as an include directory or have an + include directory that is a subdirectory of another include directory. + The compiler will only issue an error if the source unit name passed to the Host Filesystem + Loader represents an existing path when combined with multiple include paths or an include path + and base path. + +.. _cli-path-normalization-and-stripping: + +CLI Path Normalization and Stripping +------------------------------------ On the command line the compiler behaves just as you would expect from any other program: it accepts paths in a format native to the platform and relative paths are relative to the current @@ -326,7 +360,7 @@ The source unit names assigned to files whose paths are specified on the command should not change just because the project is being compiled on a different platform or because the compiler happens to have been invoked from a different directory. To achieve this, paths to source files coming from the command line must be converted to a canonical -form, and, if possible, made relative to the base path. +form, and, if possible, made relative to the base path or one of the include paths. The normalization rules are as follows: @@ -357,7 +391,8 @@ The normalization rules are as follows: You can avoid such situations by ensuring that all the files are available within a single directory tree on the same drive. -Once canonicalized, the base path is stripped from all source file paths that start with it. +After normalization the compiler attempts to make the source file path relative. +It tries the base path first and then the include paths in the order they were given. If the base path is empty or not specified, it is treated as if it was equal to the path to the current working directory (with all symbolic links resolved). The result is accepted only if the normalized directory path is the exact prefix of the normalized @@ -366,6 +401,16 @@ Otherwise the file path remains absolute. This makes the conversion unambiguous and ensures that the relative path does not start with ``../``. The resulting file path becomes the source unit name. +.. note:: + + The relative path produced by stripping must remain unique within the base path and include paths. + For example the compiler will issue an error for the following command if both + ``/project/contract.sol`` and ``/lib/contract.sol`` exist: + + .. code-block:: bash + + solc /project/contract.sol --base-path /project --include-path /lib + .. note:: Prior to version 0.8.8, CLI path stripping was not performed and the only normalization applied @@ -388,11 +433,11 @@ locations that are considered safe by default: - The directories used as :ref:`remapping ` targets. If the target is not a directory (i.e does not end with ``/``, ``/.`` or ``/..``) the directory containing the target is used instead. - - Base path. + - Base path and include paths. - In Standard JSON mode: - - Base path. + - Base path and include paths. Additional directories can be whitelisted using the ``--allow-paths`` option. The option accepts a comma-separated list of paths: @@ -403,6 +448,7 @@ The option accepts a comma-separated list of paths: solc token/contract.sol \ lib/util.sol=libs/util.sol \ --base-path=token/ \ + --include-path=/lib/ \ --allow-paths=../utils/,/tmp/libraries When the compiler is invoked with the command shown above, the Host Filesystem Loader will allow @@ -410,6 +456,7 @@ importing files from the following directories: - ``/home/user/project/token/`` (because ``token/`` contains the input file and also because it is the base path), +- ``/lib/`` (because ``/lib/`` is one of the include paths), - ``/home/user/project/libs/`` (because ``libs/`` is a directory containing a remapping target), - ``/home/user/utils/`` (because of ``../utils/`` passed to ``--allow-paths``), - ``/tmp/libraries/`` (because of ``/tmp/libraries`` passed to ``--allow-paths``), @@ -492,6 +539,13 @@ Loader, which will then look in ``/project/dapp-bin/library/iterable_mapping.sol would need to recreate parts of your local directory structure in the VFS and (if you rely on Host Filesystem Loader) also in the host filesystem. + To avoid having your local directory structure embedded in the metadata, it is recommended to + designate the directories containing libraries as *include paths* instead. + For example, in the example above ``--include-path /home/user/packages/`` would let you use + imports starting with ``mymath/``. + Unlike remapping, the option on its own will not make ``mymath`` appear as ``@math`` but this + can be achieved by creating a symbolic link or renaming the package subdirectory. + As a more complex example, suppose you rely on a module that uses an old version of dapp-bin that you checked out to ``/project/dapp-bin_old``, then you can run: diff --git a/docs/using-the-compiler.rst b/docs/using-the-compiler.rst index 406af7c9b..1b14187ca 100644 --- a/docs/using-the-compiler.rst +++ b/docs/using-the-compiler.rst @@ -33,7 +33,7 @@ This parameter has effects on the following (this might change in the future): - the size of the binary search in the function dispatch routine - the way constants like large numbers or strings are stored -.. index:: allowed paths, --allow-paths, base path, --base-path +.. index:: allowed paths, --allow-paths, base path, --base-path, include paths, --include-path Base Path and Import Remapping ------------------------------ @@ -49,9 +49,9 @@ This essentially instructs the compiler to search for anything starting with ``github.com/ethereum/dapp-bin/`` under ``/usr/local/lib/dapp-bin``. When accessing the filesystem to search for imports, :ref:`paths that do not start with ./ -or ../ ` are treated as relative to the directory specified using -``--base-path`` option (or the current working directory if base path is not specified). -Furthermore, the part added via ``--base-path`` will not appear in the contract metadata. +or ../ ` are treated as relative to the directories specified using +``--base-path`` and ``--include-path`` options (or the current working directory if base path is not specified). +Furthermore, the part of the path added via these options will not appear in the contract metadata. For security reasons the compiler has :ref:`restrictions on what directories it can access `. Directories of source files specified on the command line and target paths of diff --git a/libsolidity/interface/FileReader.cpp b/libsolidity/interface/FileReader.cpp index 06a0381cf..2a1859d21 100644 --- a/libsolidity/interface/FileReader.cpp +++ b/libsolidity/interface/FileReader.cpp @@ -21,33 +21,61 @@ #include #include +#include #include +#include + +#include + +#include using solidity::frontend::ReadCallback; using solidity::langutil::InternalCompilerError; using solidity::util::errinfo_comment; using solidity::util::readFileAsString; +using solidity::util::joinHumanReadable; +using std::map; +using std::reference_wrapper; using std::string; +using std::vector; namespace solidity::frontend { FileReader::FileReader( boost::filesystem::path _basePath, + vector const& _includePaths, FileSystemPathSet _allowedDirectories ): m_allowedDirectories(std::move(_allowedDirectories)), m_sourceCodes() { setBasePath(_basePath); + for (boost::filesystem::path const& includePath: _includePaths) + addIncludePath(includePath); + for (boost::filesystem::path const& allowedDir: m_allowedDirectories) solAssert(!allowedDir.empty(), ""); } void FileReader::setBasePath(boost::filesystem::path const& _path) { - m_basePath = (_path.empty() ? "" : normalizeCLIPathForVFS(_path)); + if (_path.empty()) + { + // Empty base path is a special case that does not make sense when include paths are used. + solAssert(m_includePaths.empty(), ""); + m_basePath = ""; + } + else + m_basePath = normalizeCLIPathForVFS(_path); +} + +void FileReader::addIncludePath(boost::filesystem::path const& _path) +{ + solAssert(!m_basePath.empty(), ""); + solAssert(!_path.empty(), ""); + m_includePaths.push_back(normalizeCLIPathForVFS(_path)); } void FileReader::allowDirectory(boost::filesystem::path _path) @@ -58,10 +86,7 @@ void FileReader::allowDirectory(boost::filesystem::path _path) void FileReader::setSource(boost::filesystem::path const& _path, SourceCode _source) { - boost::filesystem::path normalizedPath = normalizeCLIPathForVFS(_path); - boost::filesystem::path prefix = (m_basePath.empty() ? normalizeCLIPathForVFS(".") : m_basePath); - - m_sourceCodes[stripPrefixIfPresent(prefix, normalizedPath).generic_string()] = std::move(_source); + m_sourceCodes[cliPathToSourceUnitName(_path)] = std::move(_source); } void FileReader::setStdin(SourceCode _source) @@ -87,12 +112,38 @@ ReadCallback::Result FileReader::readFile(string const& _kind, string const& _so if (strippedSourceUnitName.find("file://") == 0) strippedSourceUnitName.erase(0, 7); - auto canonicalPath = normalizeCLIPathForVFS(m_basePath / strippedSourceUnitName, SymlinkResolution::Enabled); + vector candidates; + vector> prefixes = {m_basePath}; + prefixes += (m_includePaths | ranges::to>>); + + for (auto const& prefix: prefixes) + { + boost::filesystem::path canonicalPath = normalizeCLIPathForVFS(prefix / strippedSourceUnitName, SymlinkResolution::Enabled); + + if (boost::filesystem::exists(canonicalPath)) + candidates.push_back(std::move(canonicalPath)); + } + + auto pathToQuotedString = [](boost::filesystem::path const& _path){ return "\"" + _path.string() + "\""; }; + + if (candidates.empty()) + return ReadCallback::Result{false, "File not found."}; + + if (candidates.size() >= 2) + return ReadCallback::Result{ + false, + "Ambiguous import. " + "Multiple matching files found inside base path and/or include paths: " + + joinHumanReadable(candidates | ranges::views::transform(pathToQuotedString), ", ") + + "." + }; + FileSystemPathSet extraAllowedPaths = {m_basePath.empty() ? "." : m_basePath}; + extraAllowedPaths += m_includePaths; bool isAllowed = false; for (boost::filesystem::path const& allowedDir: m_allowedDirectories + extraAllowedPaths) - if (isPathPrefix(normalizeCLIPathForVFS(allowedDir, SymlinkResolution::Enabled), canonicalPath)) + if (isPathPrefix(normalizeCLIPathForVFS(allowedDir, SymlinkResolution::Enabled), candidates[0])) { isAllowed = true; break; @@ -101,14 +152,12 @@ ReadCallback::Result FileReader::readFile(string const& _kind, string const& _so if (!isAllowed) return ReadCallback::Result{false, "File outside of allowed directories."}; - if (!boost::filesystem::exists(canonicalPath)) - return ReadCallback::Result{false, "File not found."}; - - if (!boost::filesystem::is_regular_file(canonicalPath)) + if (!boost::filesystem::is_regular_file(candidates[0])) return ReadCallback::Result{false, "Not a valid file."}; // NOTE: we ignore the FileNotFound exception as we manually check above - auto contents = readFileAsString(canonicalPath); + auto contents = readFileAsString(candidates[0]); + solAssert(m_sourceCodes.count(_sourceUnitName) == 0, ""); m_sourceCodes[_sourceUnitName] = contents; return ReadCallback::Result{true, contents}; } @@ -126,6 +175,41 @@ ReadCallback::Result FileReader::readFile(string const& _kind, string const& _so } } +string FileReader::cliPathToSourceUnitName(boost::filesystem::path const& _cliPath) +{ + vector prefixes = {m_basePath.empty() ? normalizeCLIPathForVFS(".") : m_basePath}; + prefixes += m_includePaths; + + boost::filesystem::path normalizedPath = normalizeCLIPathForVFS(_cliPath); + for (boost::filesystem::path const& prefix: prefixes) + if (isPathPrefix(prefix, normalizedPath)) + { + // Multiple prefixes can potentially match the path. We take the first one. + normalizedPath = stripPrefixIfPresent(prefix, normalizedPath); + break; + } + + return normalizedPath.generic_string(); +} + +map FileReader::detectSourceUnitNameCollisions(FileSystemPathSet const& _cliPaths) +{ + map nameToPaths; + for (boost::filesystem::path const& cliPath: _cliPaths) + { + string sourceUnitName = cliPathToSourceUnitName(cliPath); + boost::filesystem::path normalizedPath = normalizeCLIPathForVFS(cliPath); + nameToPaths[sourceUnitName].insert(normalizedPath); + } + + map collisions; + for (auto&& [sourceUnitName, cliPaths]: nameToPaths) + if (cliPaths.size() >= 2) + collisions[sourceUnitName] = std::move(cliPaths); + + return collisions; +} + boost::filesystem::path FileReader::normalizeCLIPathForVFS( boost::filesystem::path const& _path, SymlinkResolution _symlinkResolution diff --git a/libsolidity/interface/FileReader.h b/libsolidity/interface/FileReader.h index fe59464ad..71f5819df 100644 --- a/libsolidity/interface/FileReader.h +++ b/libsolidity/interface/FileReader.h @@ -44,13 +44,20 @@ public: Enabled, ///< Follow symbolic links. The path should contain no symlinks. }; - /// Constructs a FileReader with a base path and a set of allowed directories that - /// will be used when requesting files from this file reader instance. - explicit FileReader(boost::filesystem::path _basePath = {}, FileSystemPathSet _allowedDirectories = {}); + /// Constructs a FileReader with a base path and sets of include paths and allowed directories + /// that will be used when requesting files from this file reader instance. + explicit FileReader( + boost::filesystem::path _basePath = {}, + std::vector const& _includePaths = {}, + FileSystemPathSet _allowedDirectories = {} + ); void setBasePath(boost::filesystem::path const& _path); boost::filesystem::path const& basePath() const noexcept { return m_basePath; } + void addIncludePath(boost::filesystem::path const& _path); + std::vector const& includePaths() const noexcept { return m_includePaths; } + void allowDirectory(boost::filesystem::path _path); FileSystemPathSet const& allowedDirectories() const noexcept { return m_allowedDirectories; } @@ -85,6 +92,17 @@ public: return [this](std::string const& _kind, std::string const& _path) { return readFile(_kind, _path); }; } + /// Creates a source unit name by normalizing a path given on the command line and, if possible, + /// making it relative to base path or one of the include directories. + std::string cliPathToSourceUnitName(boost::filesystem::path const& _cliPath); + + /// Checks if a set contains any paths that lead to different files but would receive identical + /// source unit names. Files are considered the same if their paths are exactly the same after + /// normalization (without following symlinks). + /// @returns a map containing all the conflicting source unit names and the paths that would + /// receive them. The returned paths are normalized. + std::map detectSourceUnitNameCollisions(FileSystemPathSet const& _cliPaths); + /// Normalizes a filesystem path to make it include all components up to the filesystem root, /// remove small, inconsequential differences that do not affect the meaning and make it look /// the same on all platforms (if possible). @@ -130,6 +148,9 @@ private: /// Base path, used for resolving relative paths in imports. boost::filesystem::path m_basePath; + /// Additional directories used for resolving relative paths in imports. + std::vector m_includePaths; + /// list of allowed directories to read files from FileSystemPathSet m_allowedDirectories; diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 3399a86fb..dfe5cfc5c 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -422,9 +422,30 @@ bool CommandLineInterface::readInputFiles() } } + for (boost::filesystem::path const& includePath: m_options.input.includePaths) + m_fileReader.addIncludePath(includePath); + for (boost::filesystem::path const& allowedDirectory: m_options.input.allowedDirectories) m_fileReader.allowDirectory(allowedDirectory); + map> collisions = + m_fileReader.detectSourceUnitNameCollisions(m_options.input.paths); + if (!collisions.empty()) + { + auto pathToQuotedString = [](boost::filesystem::path const& _path){ return "\"" + _path.string() + "\""; }; + + serr() << "Source unit name collision detected. "; + serr() << "The specified values of base path and/or include paths would result in multiple "; + serr() << "input files being assigned the same source unit name:" << endl; + for (auto const& [sourceUnitName, normalizedInputPaths]: collisions) + { + serr() << sourceUnitName << " matches: "; + serr() << joinHumanReadable(normalizedInputPaths | ranges::views::transform(pathToQuotedString)) << endl; + } + + return false; + } + for (boost::filesystem::path const& infile: m_options.input.paths) { if (!boost::filesystem::exists(infile)) diff --git a/solc/CommandLineParser.cpp b/solc/CommandLineParser.cpp index d74bcbf04..059a15a5f 100644 --- a/solc/CommandLineParser.cpp +++ b/solc/CommandLineParser.cpp @@ -54,6 +54,7 @@ ostream& CommandLineParser::serr() static string const g_strAbi = "abi"; static string const g_strAllowPaths = "allow-paths"; static string const g_strBasePath = "base-path"; +static string const g_strIncludePath = "include-path"; static string const g_strAsm = "asm"; static string const g_strAsmJson = "asm-json"; static string const g_strAssemble = "assemble"; @@ -273,6 +274,7 @@ bool CommandLineOptions::operator==(CommandLineOptions const& _other) const noex input.remappings == _other.input.remappings && input.addStdin == _other.input.addStdin && input.basePath == _other.input.basePath && + input.includePaths == _other.input.includePaths && input.allowedDirectories == _other.input.allowedDirectories && input.ignoreMissingFiles == _other.input.ignoreMissingFiles && input.errorRecovery == _other.input.errorRecovery && @@ -532,6 +534,15 @@ General Information)").c_str(), po::value()->value_name("path"), "Use the given path as the root of the source tree instead of the root of the filesystem." ) + ( + g_strIncludePath.c_str(), + po::value>()->value_name("path"), + "Make an additional source directory available to the default import callback. " + "Use this option if you want to import contracts whose location is not fixed in relation " + "to your main source tree, e.g. third-party libraries installed using a package manager. " + "Can be used multiple times. " + "Can only be used if base path has a non-empty value." + ) ( g_strAllowPaths.c_str(), po::value()->value_name("path(s)"), @@ -983,6 +994,25 @@ bool CommandLineParser::processArgs() if (m_args.count(g_strBasePath)) m_options.input.basePath = m_args[g_strBasePath].as(); + if (m_args.count(g_strIncludePath) > 0) + { + if (m_options.input.basePath.empty()) + { + serr() << "--" << g_strIncludePath << " option requires a non-empty base path." << endl; + return false; + } + + for (string const& includePath: m_args[g_strIncludePath].as>()) + { + if (includePath.empty()) + { + serr() << "Empty values are not allowed in --" << g_strIncludePath << "." << endl; + return false; + } + m_options.input.includePaths.push_back(includePath); + } + } + if (m_args.count(g_strAllowPaths)) { vector paths; diff --git a/solc/CommandLineParser.h b/solc/CommandLineParser.h index 41aaab71b..4159e2ac2 100644 --- a/solc/CommandLineParser.h +++ b/solc/CommandLineParser.h @@ -111,6 +111,7 @@ struct CommandLineOptions std::vector remappings; bool addStdin = false; boost::filesystem::path basePath = ""; + std::vector includePaths; FileReader::FileSystemPathSet allowedDirectories; bool ignoreMissingFiles = false; bool errorRecovery = false; diff --git a/test/FilesystemUtils.cpp b/test/FilesystemUtils.cpp index 159568c62..0235b1147 100644 --- a/test/FilesystemUtils.cpp +++ b/test/FilesystemUtils.cpp @@ -31,7 +31,8 @@ void solidity::test::createFilesWithParentDirs(set cons if (!path.parent_path().empty()) boost::filesystem::create_directories(path.parent_path()); - ofstream newFile(path.string()); + // Use binary mode to avoid line ending conversion on Windows. + ofstream newFile(path.string(), std::ofstream::binary); newFile << _content; if (newFile.fail() || !boost::filesystem::exists(path)) diff --git a/test/solc/CommandLineInterface.cpp b/test/solc/CommandLineInterface.cpp index b76d86ee8..126a1cd09 100644 --- a/test/solc/CommandLineInterface.cpp +++ b/test/solc/CommandLineInterface.cpp @@ -27,6 +27,8 @@ #include #include +#include + #include #include @@ -868,6 +870,427 @@ BOOST_AUTO_TEST_CASE(cli_paths_to_source_unit_names_base_path_and_stdin) BOOST_TEST(result.reader.basePath() == expectedWorkDir / "base"); } +BOOST_AUTO_TEST_CASE(cli_include_paths) +{ + TemporaryDirectory tempDir({"base/", "include/", "lib/nested/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + + string const preamble = + "// SPDX-License-Identifier: GPL-3.0\n" + "pragma solidity >=0.0;\n"; + string const mainContractSource = preamble + + "import \"contract.sol\";\n" + "import \"contract_via_callback.sol\";\n" + "import \"include.sol\";\n" + "import \"include_via_callback.sol\";\n" + "import \"nested.sol\";\n" + "import \"nested_via_callback.sol\";\n" + "import \"lib.sol\";\n" + "import \"lib_via_callback.sol\";\n"; + + createFilesWithParentDirs( + { + tempDir.path() / "base/contract.sol", + tempDir.path() / "base/contract_via_callback.sol", + tempDir.path() / "include/include.sol", + tempDir.path() / "include/include_via_callback.sol", + tempDir.path() / "lib/nested/nested.sol", + tempDir.path() / "lib/nested/nested_via_callback.sol", + tempDir.path() / "lib/lib.sol", + tempDir.path() / "lib/lib_via_callback.sol", + }, + preamble + ); + createFilesWithParentDirs({tempDir.path() / "base/main.sol"}, mainContractSource); + + boost::filesystem::path canonicalWorkDir = boost::filesystem::canonical(tempDir); + boost::filesystem::path expectedWorkDir = "/" / canonicalWorkDir.relative_path(); + + vector commandLine = { + "solc", + "--no-color", + "--base-path=base/", + "--include-path=include/", + "--include-path=lib/nested", + "--include-path=lib/", + "base/main.sol", + "base/contract.sol", + "include/include.sol", + "lib/nested/nested.sol", + "lib/lib.sol", + }; + + CommandLineOptions expectedOptions; + expectedOptions.input.paths = { + "base/main.sol", + "base/contract.sol", + "include/include.sol", + "lib/nested/nested.sol", + "lib/lib.sol", + }; + expectedOptions.input.basePath = "base/"; + expectedOptions.input.includePaths = { + "include/", + "lib/nested", + "lib/", + }; + expectedOptions.formatting.coloredOutput = false; + expectedOptions.modelChecker.initialize = true; + + map expectedSources = { + {"main.sol", mainContractSource}, + {"contract.sol", preamble}, + {"contract_via_callback.sol", preamble}, + {"include.sol", preamble}, + {"include_via_callback.sol", preamble}, + {"nested.sol", preamble}, + {"nested_via_callback.sol", preamble}, + {"lib.sol", preamble}, + {"lib_via_callback.sol", preamble}, + }; + + vector expectedIncludePaths = { + expectedWorkDir / "include/", + expectedWorkDir / "lib/nested", + expectedWorkDir / "lib/", + }; + + FileReader::FileSystemPathSet expectedAllowedDirectories = { + canonicalWorkDir / "base", + canonicalWorkDir / "include", + canonicalWorkDir / "lib/nested", + canonicalWorkDir / "lib", + }; + + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles( + commandLine, + "", + true /* _processInput */ + ); + + BOOST_TEST(result.stderrContent == ""); + BOOST_TEST(result.stdoutContent == ""); + BOOST_REQUIRE(result.success); + BOOST_TEST(result.options == expectedOptions); + BOOST_TEST(result.reader.sourceCodes() == expectedSources); + BOOST_TEST(result.reader.includePaths() == expectedIncludePaths); + BOOST_TEST(result.reader.allowedDirectories() == expectedAllowedDirectories); + BOOST_TEST(result.reader.basePath() == expectedWorkDir / "base/"); +} + +BOOST_AUTO_TEST_CASE(standard_json_include_paths) +{ + TemporaryDirectory tempDir({"base/", "include/", "lib/nested/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + + string const preamble = + "// SPDX-License-Identifier: GPL-3.0\n" + "pragma solidity >=0.0;\n"; + string const mainContractSource = preamble + + "import 'contract_via_callback.sol';\n" + "import 'include_via_callback.sol';\n" + "import 'nested_via_callback.sol';\n" + "import 'lib_via_callback.sol';\n"; + + string const standardJsonInput = R"( + { + "language": "Solidity", + "sources": { + "main.sol": {"content": ")" + mainContractSource + R"("} + } + } + )"; + + createFilesWithParentDirs( + { + tempDir.path() / "base/contract_via_callback.sol", + tempDir.path() / "include/include_via_callback.sol", + tempDir.path() / "lib/nested/nested_via_callback.sol", + tempDir.path() / "lib/lib_via_callback.sol", + }, + preamble + ); + + boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path(); + + vector commandLine = { + "solc", + "--base-path=base/", + "--include-path=include/", + "--include-path=lib/nested", + "--include-path=lib/", + "--standard-json", + }; + + CommandLineOptions expectedOptions; + expectedOptions.input.mode = InputMode::StandardJson; + expectedOptions.input.paths = {}; + expectedOptions.input.addStdin = true; + expectedOptions.input.basePath = "base/"; + expectedOptions.input.includePaths = { + "include/", + "lib/nested", + "lib/", + }; + expectedOptions.modelChecker.initialize = false; + + // NOTE: Source code from Standard JSON does not end up in FileReader. This is not a problem + // because FileReader is only used once to initialize the compiler stack and after that + // its sources are irrelevant (even though the callback still stores everything it loads). + map expectedSources = { + {"contract_via_callback.sol", preamble}, + {"include_via_callback.sol", preamble}, + {"nested_via_callback.sol", preamble}, + {"lib_via_callback.sol", preamble}, + }; + + vector expectedIncludePaths = { + expectedWorkDir / "include/", + expectedWorkDir / "lib/nested", + expectedWorkDir / "lib/", + }; + + FileReader::FileSystemPathSet expectedAllowedDirectories = {}; + + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles( + commandLine, + standardJsonInput, + true /* _processInput */ + ); + + Json::Value parsedStdout; + string jsonParsingErrors; + BOOST_TEST(util::jsonParseStrict(result.stdoutContent, parsedStdout, &jsonParsingErrors)); + BOOST_TEST(jsonParsingErrors == ""); + for (Json::Value const& errorDict: parsedStdout["errors"]) + // The error list might contain pre-release compiler warning + BOOST_TEST(errorDict["severity"] != "error"); + BOOST_TEST( + (parsedStdout["sources"].getMemberNames() | ranges::to) == + (expectedSources | ranges::views::keys | ranges::to) + set{"main.sol"} + ); + + BOOST_REQUIRE(result.success); + BOOST_TEST(result.options == expectedOptions); + BOOST_TEST(result.reader.sourceCodes() == expectedSources); + BOOST_TEST(result.reader.includePaths() == expectedIncludePaths); + BOOST_TEST(result.reader.allowedDirectories() == expectedAllowedDirectories); + BOOST_TEST(result.reader.basePath() == expectedWorkDir / "base/"); +} + +BOOST_AUTO_TEST_CASE(cli_include_paths_empty_path) +{ + TemporaryDirectory tempDir({"base/", "include/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + createFilesWithParentDirs({tempDir.path() / "base/main.sol"}); + + string expectedMessage = "Empty values are not allowed in --include-path.\n"; + + vector commandLine = { + "solc", + "--base-path=base/", + "--include-path", "include/", + "--include-path", "", + "base/main.sol", + }; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(!result.success); + BOOST_TEST(result.stderrContent == expectedMessage); +} + +BOOST_AUTO_TEST_CASE(cli_include_paths_without_base_path) +{ + TemporaryDirectory tempDir(TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + createFilesWithParentDirs({tempDir.path() / "contract.sol"}); + + string expectedMessage = "--include-path option requires a non-empty base path.\n"; + + vector commandLine = {"solc", "--include-path", "include/", "contract.sol"}; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(!result.success); + BOOST_TEST(result.stderrContent == expectedMessage); +} + +BOOST_AUTO_TEST_CASE(cli_include_paths_should_detect_source_unit_name_collisions) +{ + TemporaryDirectory tempDir({"dir1/", "dir2/", "dir3/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + createFilesWithParentDirs({ + "dir1/contract1.sol", + "dir1/contract2.sol", + "dir2/contract1.sol", + "dir2/contract2.sol", + }); + + boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path(); + + string expectedMessage = + "Source unit name collision detected. " + "The specified values of base path and/or include paths would result in multiple " + "input files being assigned the same source unit name:\n" + "contract1.sol matches: " + "\"" + (expectedWorkDir / "dir1/contract1.sol").generic_string() + "\", " + "\"" + (expectedWorkDir / "dir2/contract1.sol").generic_string() + "\"\n" + "contract2.sol matches: " + "\"" + (expectedWorkDir / "dir1/contract2.sol").generic_string() + "\", " + "\"" + (expectedWorkDir / "dir2/contract2.sol").generic_string() + "\"\n"; + + { + // import "contract1.sol" and import "contract2.sol" would be ambiguous: + vector commandLine = { + "solc", + "--base-path=dir1/", + "--include-path=dir2/", + "dir1/contract1.sol", + "dir2/contract1.sol", + "dir1/contract2.sol", + "dir2/contract2.sol", + }; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(result.stderrContent == expectedMessage); + BOOST_REQUIRE(!result.success); + } + + { + // import "contract1.sol" and import "contract2.sol" would be ambiguous: + vector commandLine = { + "solc", + "--base-path=dir3/", + "--include-path=dir1/", + "--include-path=dir2/", + "dir1/contract1.sol", + "dir2/contract1.sol", + "dir1/contract2.sol", + "dir2/contract2.sol", + }; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(result.stderrContent == expectedMessage); + BOOST_REQUIRE(!result.success); + } + + { + // No conflict if files with the same name exist but only one is given to the compiler. + vector commandLine = { + "solc", + "--base-path=dir3/", + "--include-path=dir1/", + "--include-path=dir2/", + "dir1/contract1.sol", + "dir1/contract2.sol", + }; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(result.stderrContent == ""); + BOOST_REQUIRE(result.success); + } + + { + // The same file specified multiple times is not a conflict. + vector commandLine = { + "solc", + "--base-path=dir3/", + "--include-path=dir1/", + "--include-path=dir2/", + "dir1/contract1.sol", + "dir1/contract1.sol", + "./dir1/contract1.sol", + }; + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(result.stderrContent == ""); + BOOST_REQUIRE(result.success); + } +} + +BOOST_AUTO_TEST_CASE(cli_include_paths_should_allow_duplicate_paths) +{ + TemporaryDirectory tempDir({"dir1/", "dir2/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + createFilesWithParentDirs({"dir1/contract.sol"}); + + boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path(); + boost::filesystem::path expectedTempDir = "/" / tempDir.path().relative_path(); + + vector commandLine = { + "solc", + "--base-path=dir1/", + "--include-path", "dir1", + "--include-path", "dir1", + "--include-path", "dir1/", + "--include-path", "dir1/", + "--include-path", "./dir1/", + "--include-path", "dir2/../dir1/", + "--include-path", (tempDir.path() / "dir1/").string(), + "--include-path", (expectedWorkDir / "dir1/").string(), + "--include-path", "dir1/", + "dir1/contract.sol", + }; + + // Duplicates do not affect the result but are not removed from the include path list. + vector expectedIncludePaths = { + expectedWorkDir / "dir1", + expectedWorkDir / "dir1", + expectedWorkDir / "dir1/", + expectedWorkDir / "dir1/", + expectedWorkDir / "dir1/", + expectedWorkDir / "dir1/", + // NOTE: On macOS expectedTempDir usually contains a symlink and therefore for us it's + // different from expectedWorkDir. + expectedTempDir / "dir1/", + expectedWorkDir / "dir1/", + expectedWorkDir / "dir1/", + }; + + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles(commandLine); + BOOST_TEST(result.stderrContent == ""); + BOOST_REQUIRE(result.success); + BOOST_TEST(result.reader.includePaths() == expectedIncludePaths); + BOOST_TEST(result.reader.basePath() == expectedWorkDir / "dir1/"); +} + +BOOST_AUTO_TEST_CASE(cli_include_paths_ambiguous_import) +{ + TemporaryDirectory tempDir({"base/", "include/"}, TEST_CASE_NAME); + TemporaryWorkingDirectory tempWorkDir(tempDir); + + string const preamble = + "// SPDX-License-Identifier: GPL-3.0\n" + "pragma solidity >=0.0;\n"; + string const mainContractSource = preamble + + // Ambiguous: both base/contract.sol and include/contract.sol match the import. + "import \"contract.sol\";"; + + createFilesWithParentDirs({"base/contract.sol", "include/contract.sol"}, preamble); + + boost::filesystem::path expectedWorkDir = "/" / boost::filesystem::canonical(tempDir).relative_path(); + + vector commandLine = { + "solc", + "--no-color", + "--base-path=base/", + "--include-path=include/", + "-", + }; + + string expectedMessage = + "Error: Source \"contract.sol\" not found: Ambiguous import. " + "Multiple matching files found inside base path and/or include paths: \"" + + (expectedWorkDir / "base/contract.sol").generic_string() + "\", \"" + + (expectedWorkDir / "include/contract.sol").generic_string() + "\".\n" + " --> :3:1:\n" + " |\n" + "3 | import \"contract.sol\";\n" + " | ^^^^^^^^^^^^^^^^^^^^^^\n\n"; + + OptionsReaderAndMessages result = parseCommandLineAndReadInputFiles( + commandLine, + mainContractSource, + true /* _processInput */ + ); + + BOOST_TEST(result.stderrContent == expectedMessage); + BOOST_REQUIRE(!result.success); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace solidity::frontend::test diff --git a/test/solc/CommandLineInterfaceAllowPaths.cpp b/test/solc/CommandLineInterfaceAllowPaths.cpp index f3401f967..215085699 100644 --- a/test/solc/CommandLineInterfaceAllowPaths.cpp +++ b/test/solc/CommandLineInterfaceAllowPaths.cpp @@ -76,6 +76,7 @@ ImportCheck checkImport( for (string const& option: _cliOptions) soltestAssert( boost::starts_with(option, "--base-path") || + boost::starts_with(option, "--include-path") || boost::starts_with(option, "--allow-paths") || !boost::starts_with(option, "--"), "" @@ -146,6 +147,7 @@ protected: m_codeDir / "X/bc/d.sol", m_codeDir / "x/y/z.sol", + m_codeDir / "1/2/3.sol", m_codeDir / "contract.sol", m_workDir / "a/b/c/d.sol", @@ -342,10 +344,14 @@ BOOST_FIXTURE_TEST_CASE(allow_path_should_work_with_various_import_forms, AllowP BOOST_TEST(checkImport("import 'a/../../code/a/../a/b/c.sol'", {"--allow-paths", "../code/a/b/c.sol"})); BOOST_TEST(checkImport("import 'a/../../code/a///b/c.sol'", {"--allow-paths", "../code/a/b/c.sol"})); - // UNC paths in imports +#if !defined(_WIN32) + // UNC paths in imports. + // Unfortunately can't test it on Windows without having an existing UNC path. On Linux we can + // at least rely on the fact that `//` works like `/`. string uncImportPath = "/" + m_portablePrefix + "/a/b/c.sol"; soltestAssert(FileReader::isUNCPath(uncImportPath), ""); BOOST_TEST(checkImport("import '" + uncImportPath + "'", {"--allow-paths", "../code/a/b/c.sol"}) == ImportCheck::PathDisallowed()); +#endif } BOOST_FIXTURE_TEST_CASE(allow_path_automatic_whitelisting_input_files, AllowPathsFixture) @@ -419,7 +425,7 @@ BOOST_FIXTURE_TEST_CASE(allow_path_automatic_whitelisting_remappings, AllowPaths BOOST_TEST(checkImport("import '/../../../work/a/b/c.sol'", {"x=contract.sol/", "--base-path=../code/a/b/"}) == ImportCheck::PathDisallowed()); // Adding a remapping with an empty target does not whitelist anything - BOOST_TEST(checkImport("import '" + m_portablePrefix + "/a/b/c.sol'", {m_portablePrefix + "="}) == ImportCheck::PathDisallowed()); + BOOST_TEST(checkImport("import '" + m_portablePrefix + m_portablePrefix + "/a/b/c.sol'", {m_portablePrefix + "="}) == ImportCheck::PathDisallowed()); BOOST_TEST(checkImport("import '" + m_portablePrefix + "/a/b/c.sol'", {"../code/="}) == ImportCheck::PathDisallowed()); BOOST_TEST(checkImport("import '/../work/a/b/c.sol'", {"../code/=", "--base-path", m_portablePrefix}) == ImportCheck::PathDisallowed()); @@ -508,6 +514,37 @@ BOOST_FIXTURE_TEST_CASE(allow_path_automatic_whitelisting_work_dir, AllowPathsFi BOOST_TEST(checkImport("import 'a/X/c.sol'", {"--base-path", ""})); } +BOOST_FIXTURE_TEST_CASE(allow_path_automatic_whitelisting_include_paths, AllowPathsFixture) +{ + // Relative include path whitelists its content + BOOST_TEST(checkImport("import 'b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/a"})); + BOOST_TEST(checkImport("import 'b/c/d.sol'", {"--base-path=a/b/c", "--include-path=../code/a"})); + BOOST_TEST(checkImport("import 'b/X.sol'", {"--base-path=a/b/c", "--include-path=../code/a"})); + BOOST_TEST(checkImport("import 'X/c.sol'", {"--base-path=a/b/c", "--include-path=../code/a"})); + + BOOST_TEST(checkImport("import 'b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/a/"})); + BOOST_TEST(checkImport("import 'b/c/d.sol'", {"--base-path=a/b/c", "--include-path=../code/a/"})); + BOOST_TEST(checkImport("import 'b/X.sol'", {"--base-path=a/b/c", "--include-path=../code/a/"})); + BOOST_TEST(checkImport("import 'X/c.sol'", {"--base-path=a/b/c", "--include-path=../code/a/"})); + + BOOST_TEST(checkImport("import 'a/b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/."})); + BOOST_TEST(checkImport("import 'a/b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/./"})); + BOOST_TEST(checkImport("import 'code/a/b/c.sol'", {"--base-path=a/b/c", "--include-path=.."})); + BOOST_TEST(checkImport("import 'code/a/b/c.sol'", {"--base-path=a/b/c", "--include-path=../"})); + + // Absolute include path whitelists its content + BOOST_TEST(checkImport("import 'b/c.sol'", {"--base-path=a/b/c", "--include-path", m_codeDir.string() + "/a"})); + BOOST_TEST(checkImport("import 'b/c/d.sol'", {"--base-path=a/b/c", "--include-path", m_codeDir.string() + "/a"})); + BOOST_TEST(checkImport("import 'b/X.sol'", {"--base-path=a/b/c", "--include-path", m_codeDir.string() + "/a"})); + BOOST_TEST(checkImport("import 'X/c.sol'", {"--base-path=a/b/c", "--include-path", m_codeDir.string() + "/a"})); + + // If there are multiple include paths, all of them get whitelisted + BOOST_TEST(checkImport("import 'b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/a", "--include-path=../code/1"})); + BOOST_TEST(checkImport("import '2/3.sol'", {"--base-path=a/b/c", "--include-path=../code/a", "--include-path=../code/1"})); + BOOST_TEST(checkImport("import 'b/c.sol'", {"--base-path=a/b/c", "--include-path=../code/1", "--include-path=../code/a"})); + BOOST_TEST(checkImport("import '2/3.sol'", {"--base-path=a/b/c", "--include-path=../code/1", "--include-path=../code/a"})); +} + BOOST_FIXTURE_TEST_CASE(allow_path_symlinks_within_whitelisted_dir, AllowPathsFixture) { BOOST_TEST(checkImport("import '" + m_portablePrefix + "/a/b_sym/c.sol'", {"--allow-paths=../code/a/b/"})); diff --git a/test/solc/CommandLineParser.cpp b/test/solc/CommandLineParser.cpp index 5644aaadd..7b1dd375b 100644 --- a/test/solc/CommandLineParser.cpp +++ b/test/solc/CommandLineParser.cpp @@ -117,6 +117,8 @@ BOOST_AUTO_TEST_CASE(cli_mode_options) "a:b=c/d", ":contract.sol=", "--base-path=/home/user/", + "--include-path=/usr/lib/include/", + "--include-path=/home/user/include", "--allow-paths=/tmp,/home,project,../contracts", "--ignore-missing", "--error-recovery", @@ -168,6 +170,8 @@ BOOST_AUTO_TEST_CASE(cli_mode_options) expectedOptions.input.addStdin = true; expectedOptions.input.basePath = "/home/user/"; + expectedOptions.input.includePaths = {"/usr/lib/include/", "/home/user/include"}; + expectedOptions.input.allowedDirectories = {"/tmp", "/home", "project", "../contracts", "c", "/usr/lib"}; expectedOptions.input.ignoreMissingFiles = true; expectedOptions.input.errorRecovery = (inputMode == InputMode::Compiler); @@ -257,6 +261,8 @@ BOOST_AUTO_TEST_CASE(assembly_mode_options) "a:b=c/d", ":contract.yul=", "--base-path=/home/user/", + "--include-path=/usr/lib/include/", + "--include-path=/home/user/include", "--allow-paths=/tmp,/home,project,../contracts", "--ignore-missing", "--error-recovery", // Ignored in assembly mode @@ -307,6 +313,7 @@ BOOST_AUTO_TEST_CASE(assembly_mode_options) }; expectedOptions.input.addStdin = true; expectedOptions.input.basePath = "/home/user/"; + expectedOptions.input.includePaths = {"/usr/lib/include/", "/home/user/include"}; expectedOptions.input.allowedDirectories = {"/tmp", "/home", "project", "../contracts", "c", "/usr/lib"}; expectedOptions.input.ignoreMissingFiles = true; expectedOptions.output.overwriteFiles = true; @@ -350,6 +357,8 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options) "input.json", "--standard-json", "--base-path=/home/user/", + "--include-path=/usr/lib/include/", + "--include-path=/home/user/include", "--allow-paths=/tmp,/home,project,../contracts", "--ignore-missing", "--error-recovery", // Ignored in Standard JSON mode @@ -390,6 +399,7 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options) expectedOptions.input.mode = InputMode::StandardJson; expectedOptions.input.paths = {"input.json"}; expectedOptions.input.basePath = "/home/user/"; + expectedOptions.input.includePaths = {"/usr/lib/include/", "/home/user/include"}; expectedOptions.input.allowedDirectories = {"/tmp", "/home", "project", "../contracts"}; expectedOptions.input.ignoreMissingFiles = true; expectedOptions.output.dir = "/tmp/out";