[torque-ls] Properly decode file URIs sent by the client

This CL changes the language server to store file paths as URIs and
decodes them on-demand during compilation. For now, this will
eliminate the need for an URI encoding function.

R=tebbi@chromium.org

Bug: v8:8880
Change-Id: If79f635cb60035f58712c1458ecca3bfa23a6e47
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1502992
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60025}

src/torque/ls/message-handler.cc

@@ -25,9 +25,6 @@ namespace ls {
 static const char kContentLength[] = "Content-Length: ";
 static const size_t kContentLengthSize = sizeof(kContentLength) - 1;
 
-static const char kFileUriPrefix[] = "file://";
-static const int kFileUriPrefixLength = sizeof(kFileUriPrefix) - 1;
-
 JsonValue ReadMessage() {
   std::string line;
   std::getline(std::cin, line);
@@ -115,15 +112,17 @@ void HandleTorqueFileListNotification(TorqueFileListNotification notification) {
 
   std::vector<std::string>& files = TorqueFileList::Get();
   Logger::Log("[info] Initial file list:\n");
-  for (const auto& fileJson :
+  for (const auto& file_json :
        notification.params().object()["files"].ToArray()) {
-    CHECK(fileJson.IsString());
-    // We only consider file URIs (there shouldn't be anything else).
-    if (fileJson.ToString().rfind(kFileUriPrefix) != 0) continue;
+    CHECK(file_json.IsString());
 
-    std::string file = fileJson.ToString().substr(kFileUriPrefixLength);
-    files.push_back(file);
-    Logger::Log("    ", file, "\n");
+    // We only consider file URIs (there shouldn't be anything else).
+    // Internally we store the URI instead of the path, eliminating the need
+    // to encode it again.
+    if (auto maybe_path = FileUriDecode(file_json.ToString())) {
+      files.push_back(file_json.ToString());
+      Logger::Log("    ", *maybe_path, "\n");
+    }
   }
 
   // The Torque compiler expects to see some files first,
@@ -147,12 +146,11 @@ void HandleGotoDefinitionRequest(GotoDefinitionRequest request,
   GotoDefinitionResponse response;
   response.set_id(request.id());
 
-  std::string file = request.params().textDocument().uri();
-  CHECK_EQ(file.rfind(kFileUriPrefix), 0);
-  SourceId id = SourceFileMap::GetSourceId(file.substr(kFileUriPrefixLength));
+  SourceId id =
+      SourceFileMap::GetSourceId(request.params().textDocument().uri());
 
-  // If we do not know about the source file, send back an empty response,
-  // i.e. we did not find anything.
+  // Unknown source files cause an empty response which corresponds with
+  // the definition not beeing found.
   if (!id.IsValid()) {
     response.SetNull("result");
     writer(response.GetJsonValue());
@@ -166,7 +164,7 @@ void HandleGotoDefinitionRequest(GotoDefinitionRequest request,
     SourcePosition definition = *maybe_definition;
 
     std::string definition_file = SourceFileMap::GetSource(definition.source);
-    response.result().set_uri(kFileUriPrefix + definition_file);
+    response.result().set_uri(definition_file);
 
     Range range = response.result().range();
     range.start().set_line(definition.start.line);

src/torque/torque-compiler.cc

@@ -18,13 +18,31 @@ namespace torque {
 
 namespace {
 
+base::Optional<std::string> ReadFile(const std::string& path) {
+  std::ifstream file_stream(path);
+  if (!file_stream.good()) return base::nullopt;
+
+  return std::string{std::istreambuf_iterator<char>(file_stream),
+                     std::istreambuf_iterator<char>()};
+}
+
 void ReadAndParseTorqueFile(const std::string& path) {
   SourceId source_id = SourceFileMap::AddSource(path);
   CurrentSourceFile::Scope source_id_scope(source_id);
-  std::ifstream file_stream(path);
-  std::string file_content = {std::istreambuf_iterator<char>(file_stream),
-                              std::istreambuf_iterator<char>()};
-  ParseTorque(file_content);
+
+  // path might be either a normal file path or an encoded URI.
+  auto maybe_content = ReadFile(path);
+  if (!maybe_content) {
+    if (auto maybe_path = FileUriDecode(path)) {
+      maybe_content = ReadFile(*maybe_path);
+    }
+  }
+
+  if (!maybe_content) {
+    ReportErrorWithoutPosition("Cannot open file path/uri: ", path);
+  }
+
+  ParseTorque(*maybe_content);
 }
 
 }  // namespace

src/torque/utils.cc

@@ -73,14 +73,57 @@ std::string StringLiteralQuote(const std::string& s) {
   return result.str();
 }
 
+static const char kFileUriPrefix[] = "file://";
+static const int kFileUriPrefixLength = sizeof(kFileUriPrefix) - 1;
+
+static int HexCharToInt(unsigned char c) {
+  if (isdigit(c)) return c - '0';
+  if (isupper(c)) return c - 'A' + 10;
+  DCHECK(islower(c));
+  return c - 'a' + 10;
+}
+
+base::Optional<std::string> FileUriDecode(const std::string& uri) {
+  // Abort decoding of URIs that don't start with "file://".
+  if (uri.rfind(kFileUriPrefix) != 0) return base::nullopt;
+
+  const std::string path = uri.substr(kFileUriPrefixLength);
+  std::ostringstream decoded;
+
+  for (auto iter = path.begin(), end = path.end(); iter != end; ++iter) {
+    std::string::value_type c = (*iter);
+
+    // Normal characters are appended.
+    if (c != '%') {
+      decoded << c;
+      continue;
+    }
+
+    // If '%' is not followed by at least two hex digits, we abort.
+    if (std::distance(iter, end) <= 2) return base::nullopt;
+
+    unsigned char first = (*++iter);
+    unsigned char second = (*++iter);
+    if (!isxdigit(first) || !isxdigit(second)) return base::nullopt;
+
+    // An escaped hex value needs converting.
+    unsigned char value = HexCharToInt(first) * 16 + HexCharToInt(second);
+    decoded << value;
+  }
+
+  return decoded.str();
+}
+
 std::string CurrentPositionAsString() {
   return PositionAsString(CurrentSourcePosition::Get());
 }
 
 DEFINE_CONTEXTUAL_VARIABLE(LintErrorStatus)
 
-[[noreturn]] void ReportErrorString(const std::string& error) {
-  std::cerr << CurrentPositionAsString() << ": Torque error: " << error << "\n";
+[[noreturn]] void ReportErrorString(const std::string& error,
+                                    bool print_position) {
+  if (print_position) std::cerr << CurrentPositionAsString() << ": ";
+  std::cerr << ": Torque error: " << error << "\n";
   v8::base::OS::Abort();
 }
 

src/torque/utils.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "src/base/functional.h"
+#include "src/base/optional.h"
 #include "src/torque/contextual.h"
 
 namespace v8 {
@@ -21,6 +22,11 @@ namespace torque {
 std::string StringLiteralUnquote(const std::string& s);
 std::string StringLiteralQuote(const std::string& s);
 
+// Decodes "file://" URIs into file paths which can then be used
+// with the standard stream API.
+V8_EXPORT_PRIVATE base::Optional<std::string> FileUriDecode(
+    const std::string& s);
+
 class LintErrorStatus : public ContextualClass<LintErrorStatus> {
  public:
   LintErrorStatus() : has_lint_errors_(false) {}
@@ -45,12 +51,19 @@ bool IsSnakeCase(const std::string& s);
 bool IsValidNamespaceConstName(const std::string& s);
 bool IsValidTypeName(const std::string& s);
 
-[[noreturn]] void ReportErrorString(const std::string& error);
+[[noreturn]] void ReportErrorString(const std::string& error,
+                                    bool print_position);
 template <class... Args>
 [[noreturn]] void ReportError(Args&&... args) {
   std::stringstream s;
   USE((s << std::forward<Args>(args))...);
-  ReportErrorString(s.str());
+  ReportErrorString(s.str(), true);
+}
+template <class... Args>
+[[noreturn]] void ReportErrorWithoutPosition(Args&&... args) {
+  std::stringstream s;
+  USE((s << std::forward<Args>(args))...);
+  ReportErrorString(s.str(), false);
 }
 
 std::string CapifyStringWithUnderscores(const std::string& camellified_string);

test/unittests/BUILD.gn

@@ -201,6 +201,7 @@ v8_source_set("unittests_sources") {
     "torque/ls-json-unittest.cc",
     "torque/ls-message-unittest.cc",
     "torque/torque-unittest.cc",
+    "torque/torque-utils-unittest.cc",
     "unicode-unittest.cc",
     "utils-unittest.cc",
     "value-serializer-unittest.cc",

test/unittests/torque/ls-message-unittest.cc

@@ -68,8 +68,8 @@ TEST(LanguageServerMessage, GotoDefinitionUnkownFile) {
 
 TEST(LanguageServerMessage, GotoDefinition) {
   SourceFileMap::Scope source_file_map_scope;
-  SourceId test_id = SourceFileMap::AddSource("test.tq");
-  SourceId definition_id = SourceFileMap::AddSource("base.tq");
+  SourceId test_id = SourceFileMap::AddSource("file://test.tq");
+  SourceId definition_id = SourceFileMap::AddSource("file://base.tq");
 
   LanguageServerData::Scope server_data_scope;
   LanguageServerData::AddDefinition({test_id, {1, 0}, {1, 10}},

test/unittests/torque/torque-utils-unittest.cc

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/torque/utils.h"
+#include "test/unittests/test-utils.h"
+
+namespace v8 {
+namespace internal {
+namespace torque {
+
+TEST(TorqueUtils, FileUriDecodeIllegal) {
+  EXPECT_EQ(FileUriDecode("http://wrong.scheme"), base::nullopt);
+  EXPECT_EQ(FileUriDecode("file://wrong-escape%"), base::nullopt);
+  EXPECT_EQ(FileUriDecode("file://another-wrong-escape%a"), base::nullopt);
+  EXPECT_EQ(FileUriDecode("file://no-hex-escape%0g"), base::nullopt);
+}
+
+TEST(TorqueUtils, FileUriDecode) {
+  EXPECT_EQ(FileUriDecode("file:///some/src/file.tq").value(),
+            "/some/src/file.tq");
+  EXPECT_EQ(FileUriDecode("file:///c%3A/torque/base.tq").value(),
+            "/c:/torque/base.tq");
+  EXPECT_EQ(FileUriDecode("file:///d%3a/lower/hex.txt").value(),
+            "/d:/lower/hex.txt");
+}
+
+}  // namespace torque
+}  // namespace internal
+}  // namespace v8