LSP: Fixes URL decoding incoming file names.

Changelog.md

@@ -9,6 +9,7 @@ Compiler Features:
 * Standard JSON: Add a boolean field `settings.metadata.appendCBOR` that skips CBOR metadata from getting appended at the end of the bytecode.
 * Yul Optimizer: Allow replacing the previously hard-coded cleanup sequence by specifying custom steps after a colon delimiter (``:``) in the sequence string.
 * Language Server: Add basic document hover support.
+* Language Server: Fixes URL decoding of incoming file names.
 
 
 Bugfixes:
@@ -23,6 +24,7 @@ Important Bugfixes:
 
 Compiler Features:
  * Code Generator: More efficient overflow checks for multiplication.
+ * Yul Optimizer: Simplify the starting offset of zero-length operations to zero.
  * Language Server: Analyze all files in a project by default (can be customized by setting ``'file-load-strategy'`` to ``'directly-opened-and-on-import'`` in LSP settings object).
  * Yul Optimizer: Simplify the starting offset of zero-length operations to zero.
 

libsolidity/lsp/FileRepository.cpp

@@ -73,23 +73,23 @@ string FileRepository::sourceUnitNameToUri(string const& _sourceUnitName) const
 	else if (_sourceUnitName.find("file://") == 0)
 		return _sourceUnitName;
 	else if (regex_search(_sourceUnitName, windowsDriveLetterPath))
-		return "file:///" + _sourceUnitName;
+		return "file:///" + util::encodeURI(_sourceUnitName);
 	else if (
 		auto const resolvedPath = tryResolvePath(_sourceUnitName);
 		resolvedPath.message().empty()
 	)
-		return "file://" + ensurePathIsUnixLike(resolvedPath.get().generic_string());
+		return "file://" + util::encodeURI(ensurePathIsUnixLike(resolvedPath.get().generic_string()));
 	else if (m_basePath.generic_string() != "/")
-		return "file://" + m_basePath.generic_string() + "/" + _sourceUnitName;
+		return "file://" + util::encodeURI(m_basePath.generic_string() + "/" + _sourceUnitName);
 	else
 		// Avoid double-/ in case base-path itself is simply a UNIX root filesystem root.
-		return "file:///" + _sourceUnitName;
+		return "file:///" + util::encodeURI(_sourceUnitName);
 }
 
 string FileRepository::uriToSourceUnitName(string const& _path) const
 {
 	lspRequire(boost::algorithm::starts_with(_path, "file://"), ErrorCode::InternalError, "URI must start with file://");
-	return stripFileUriSchemePrefix(_path);
+	return stripFileUriSchemePrefix(util::decodeURI(_path));
 }
 
 void FileRepository::setSourceByUri(string const& _uri, string _source)

libsolidity/lsp/LanguageServer.cpp

@@ -396,7 +396,7 @@ void LanguageServer::handleInitialize(MessageID _id, Json::Value const& _args)
 			ErrorCode::InvalidParams,
 			"rootUri only supports file URI scheme."
 		);
-		rootPath = stripFileUriSchemePrefix(rootPath);
+		rootPath = stripFileUriSchemePrefix(util::decodeURI(rootPath));
 	}
 	else if (Json::Value rootPath = _args["rootPath"])
 		rootPath = rootPath.asString();
@@ -432,11 +432,11 @@ void LanguageServer::handleInitialized(MessageID, Json::Value const&)
 
 void LanguageServer::semanticTokensFull(MessageID _id, Json::Value const& _args)
 {
-	auto uri = _args["textDocument"]["uri"];
+	auto uri = _args["textDocument"]["uri"].asString();
 
 	compile();
 
-	auto const sourceName = m_fileRepository.uriToSourceUnitName(uri.as<string>());
+	auto const sourceName = m_fileRepository.uriToSourceUnitName(uri);
 	SourceUnit const& ast = m_compilerStack.ast(sourceName);
 	m_compilerStack.charStream(sourceName);
 	Json::Value data = SemanticTokensBuilder().build(ast, m_compilerStack.charStream(sourceName));

libsolutil/CMakeLists.txt

@@ -43,7 +43,7 @@ set(sources
 )
 
 add_library(solutil ${sources})
-target_link_libraries(solutil PUBLIC jsoncpp Boost::boost Boost::filesystem Boost::system range-v3)
+target_link_libraries(solutil PUBLIC jsoncpp Boost::boost Boost::filesystem Boost::system range-v3 fmt::fmt-header-only)
 target_include_directories(solutil PUBLIC "${CMAKE_SOURCE_DIR}")
 add_dependencies(solutil solidity_BuildInfo.h)
 

libsolutil/StringUtils.cpp

@@ -25,8 +25,10 @@
 #include <libsolutil/StringUtils.h>
 #include <string>
 #include <vector>
+#include <fmt/format.h>
 
 using namespace std;
+using namespace std::string_view_literals;
 using namespace solidity::util;
 
 bool solidity::util::stringWithinDistance(string const& _str1, string const& _str2, size_t _maxDistance, size_t _lenThreshold)
@@ -113,3 +115,50 @@ string solidity::util::suffixedVariableNameList(string const& _baseName, size_t
 	}
 	return result;
 }
+
+string solidity::util::decodeURI(string const& _uri)
+{
+	string decodedString;
+	for (size_t i = 0; i < _uri.size(); ++i)
+	{
+		char const ch = _uri[i];
+		if (ch != '%')
+			decodedString.push_back(ch);
+		else if (i + 2 < _uri.size())
+		{
+			char const buf[3] = { _uri[i + 1], _uri[i + 2], '\0' };
+			char* end = nullptr;
+			auto const numericValue = std::strtol(buf, &end, 16);
+			if (end == buf + 2)
+			{
+				decodedString.push_back(static_cast<char>(numericValue));
+				i += 2;
+			}
+		}
+	}
+	return decodedString;
+}
+
+string solidity::util::encodeURI(string const& _uri)
+{
+	string encodedString;
+	for (char const ch: _uri)
+	{
+		// see https://en.wikipedia.org/wiki/Percent-encoding#Types_of_URI_characters
+		static auto constexpr allowedCharacters = string_view(
+			// unreserved characters
+			"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+			"abcdefghijklmnopqrstuvwxyz"
+			"0123456789"
+			"-_.~"
+			// reserved characters (special meaning depending on URI component)
+			"/:"
+		);
+		if (allowedCharacters.find(ch) != string_view::npos)
+			encodedString += ch;
+		else
+			encodedString += fmt::format("%{:02X}", static_cast<unsigned>(ch));
+	}
+	return encodedString;
+}
+

libsolutil/StringUtils.h

@@ -50,6 +50,13 @@ std::string quotedAlternativesList(std::vector<std::string> const& suggestions);
 /// If @a _startSuffix == @a _endSuffix, the empty string is returned.
 std::string suffixedVariableNameList(std::string const& _baseName, size_t _startSuffix, size_t _endSuffix);
 
+/// Decodes a URI with respect to %XX notation.
+/// No URI-validity verification is performed but simply the URI decoded into non-escaping characters.
+std::string decodeURI(std::string const& _uri);
+
+/// Encodes a string into a URI conform notation.
+std::string encodeURI(std::string const& _uri);
+
 /// Joins collection of strings into one string with separators between, last separator can be different.
 /// @param _list collection of strings to join
 /// @param _separator defaults to ", "

test/libsolutil/StringUtils.cpp

@@ -210,6 +210,38 @@ BOOST_AUTO_TEST_CASE(test_format_number_readable_signed)
 	);
 }
 
+BOOST_AUTO_TEST_CASE(test_encodeURI)
+{
+	BOOST_CHECK_EQUAL(util::encodeURI(""), "");
+	BOOST_CHECK_EQUAL(util::encodeURI(" "), "%20");
+	BOOST_CHECK_EQUAL(util::encodeURI("%"), "%25");
+	BOOST_CHECK_EQUAL(util::encodeURI("Hello World."), "Hello%20World.");
+	BOOST_CHECK_EQUAL(util::encodeURI("\x01\x02\x7F"), "%01%02%7F");
+	BOOST_CHECK_EQUAL(util::encodeURI("C:\\readme.md"), "C:%5Creadme.md");
+	BOOST_CHECK_EQUAL(util::encodeURI("C:/readme.md"), "C:/readme.md");
+	BOOST_CHECK_EQUAL(util::encodeURI("/var/log"), "/var/log");
+}
+
+BOOST_AUTO_TEST_CASE(test_decodeURI)
+{
+	BOOST_CHECK_EQUAL(util::decodeURI(""), "");
+	BOOST_CHECK_EQUAL(util::decodeURI(""), "");
+	BOOST_CHECK_EQUAL(util::decodeURI(" ")," ");
+	BOOST_CHECK_EQUAL(util::decodeURI("%25"), "%");
+	BOOST_CHECK_EQUAL(util::decodeURI("Hello%20World."), "Hello World.");
+	BOOST_CHECK_EQUAL(util::decodeURI("Hello World."), "Hello World.");
+	BOOST_CHECK_EQUAL(util::decodeURI("%01%02%7F"), "\x01\x02\x7F");
+	BOOST_CHECK_EQUAL(util::decodeURI("C%3A%5Creadme.md"), "C:\\readme.md");
+	BOOST_CHECK_EQUAL(util::decodeURI("C:/readme.md"), "C:/readme.md");
+
+	// Decoding failure cases (there's not really a standard for that).
+	BOOST_CHECK_EQUAL(util::decodeURI("%"), "");
+	BOOST_CHECK_EQUAL(util::decodeURI("%ZZ"), "ZZ");
+	BOOST_CHECK_EQUAL(util::decodeURI("%7Ge"), "7Ge");
+	BOOST_CHECK_EQUAL(util::decodeURI("%2F%2%2F"), "/2/");
+	BOOST_CHECK_EQUAL(util::decodeURI("%1G/%7G/%FG"), "1G/7G/FG");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }

test/lsp.py

@@ -11,6 +11,7 @@ import re
 import subprocess
 import sys
 import traceback
+import urllib.parse
 from collections import namedtuple
 from copy import deepcopy
 from enum import Enum, auto
@@ -358,6 +359,10 @@ def extendEnd(marker, amount=1):
     newMarker["end"]["character"] += amount
     return newMarker
 
+# Convenience method to intuitively decode a URI.
+def decodeURI(text: str) -> str:
+    return urllib.parse.unquote(text)
+
 class TestParserException(Exception):
     def __init__(self, incompleteResult, msg: str):
         self.result = incompleteResult
@@ -590,11 +595,11 @@ class FileTestRunner:
                 self.suite.open_file_and_wait_for_diagnostics(self.solc, self.test_name, self.sub_dir)
 
             for diagnostics in published_diagnostics:
-                if not diagnostics["uri"].startswith(self.suite.project_root_uri + "/"):
+                if not decodeURI(decodeURI(diagnostics["uri"])).startswith(self.suite.project_root_uri + "/"):
                     raise Exception(
                         f"'{self.test_name}.sol' imported file outside of test directory: '{diagnostics['uri']}'"
                     )
-                self.open_tests.append(self.suite.normalizeUri(diagnostics["uri"]))
+                self.open_tests.append(self.suite.normalizeUri(decodeURI(diagnostics["uri"])))
 
             self.suite.expect_equal(
                 len(published_diagnostics),
@@ -747,7 +752,9 @@ class FileTestRunner:
             if isinstance(actualResponseJson["result"], list):
                 for result in actualResponseJson["result"]:
                     if "uri" in result:
-                        result["uri"] = result["uri"].replace(self.suite.project_root_uri + "/" + self.sub_dir + "/", "")
+                        result["uri"] = decodeURI(result["uri"]).replace(
+                            self.suite.project_root_uri + "/" + self.sub_dir + "/", ""
+                        )
 
             elif isinstance(actualResponseJson["result"], dict):
                 if "changes" in actualResponseJson["result"]:
@@ -813,7 +820,7 @@ class FileTestRunner:
             # Needs to be done before the loop or it might be called only after
             # we found "range" or "position"
             if "uri" in data:
-                markers = self.suite.get_test_tags(data["uri"][:-len(".sol")], self.sub_dir)
+                markers = self.suite.get_test_tags(decodeURI(data["uri"])[:-len(".sol")], self.sub_dir)
 
             for key, val in data.items():
                 if key == "range":
@@ -1020,7 +1027,7 @@ class SolidityLSPTestSuite: # {{{
         published_diagnostics = self.open_file_and_wait_for_diagnostics(solc, test, sub_dir)
 
         for file_diagnostics in published_diagnostics:
-            testname, local_sub_dir = split_path(self.normalizeUri(file_diagnostics["uri"]))
+            testname, local_sub_dir = split_path(self.normalizeUri(decodeURI(file_diagnostics["uri"])))
 
             # Skip empty diagnostics within the same file
             if len(file_diagnostics["diagnostics"]) == 0 and testname == test:
@@ -1242,7 +1249,7 @@ class SolidityLSPTestSuite: # {{{
         for item in recursive_iter(content):
             if "uri" in item and "range" in item:
                 try:
-                    markers = self.get_test_tags(item["uri"][:-len(".sol")], sub_dir)
+                    markers = self.get_test_tags(decodeURI(item["uri"])[:-len(".sol")], sub_dir)
                     replace_range(item, markers)
                 except FileNotFoundError:
                     # Skip over errors as this is user provided input that can