src: simplify legacy resolve functionality

anonrig · anonrig · commit d3cb3e174278 · 2023-07-03T22:24:44.000-04:00
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -844,13 +844,13 @@ void AfterOpenFileHandle(uv_fs_t* req) {
 
 // Reverse the logic applied by path.toNamespacedPath() to create a
 // namespace-prefixed path.
-void FromNamespacedPath(std::string* path) {
+void FromNamespacedPath(std::string& path) {  // NOLINT(runtime/references)
 #ifdef _WIN32
-  if (path->compare(0, 8, "\\\\?\\UNC\\", 8) == 0) {
-    *path = path->substr(8);
-    path->insert(0, "\\\\");
-  } else if (path->compare(0, 4, "\\\\?\\", 4) == 0) {
-    *path = path->substr(4);
+  if (path.compare(0, 8, "\\\\?\\UNC\\", 8) == 0) {
+    path = path.substr(8);
+    path.insert(0, "\\\\");
+  } else if (path.compare(0, 4, "\\\\?\\", 4) == 0) {
+    path = path.substr(4);
   }
 #endif
 }
@@ -864,7 +864,7 @@ void AfterMkdirp(uv_fs_t* req) {
     std::string first_path(req_wrap->continuation_data()->first_path());
     if (first_path.empty())
       return req_wrap->Resolve(Undefined(req_wrap->env()->isolate()));
-    FromNamespacedPath(&first_path);
+    FromNamespacedPath(first_path);
     Local<Value> path;
     Local<Value> error;
     if (!StringBytes::Encode(req_wrap->env()->isolate(), first_path.c_str(),
@@ -1804,7 +1804,7 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
           !req_wrap_sync.continuation_data()->first_path().empty()) {
         Local<Value> error;
         std::string first_path(req_wrap_sync.continuation_data()->first_path());
-        FromNamespacedPath(&first_path);
+        FromNamespacedPath(first_path);
         MaybeLocal<Value> path = StringBytes::Encode(env->isolate(),
                                                      first_path.c_str(),
                                                      UTF8, &error);
@@ -2732,67 +2732,52 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
 static bool FileURLToPath(
     Environment* env,
     const ada::url_aggregator& file_url,
-    /* The linter can't detect the assign for result_file_path
-       So we need to ignore since it suggest to put const */
-    // NOLINTNEXTLINE(runtime/references)
-    std::string& result_file_path) {
+    std::string& result_file_path) {  // NOLINT(runtime/references)
   if (file_url.type != ada::scheme::FILE) {
-    env->isolate()->ThrowException(ERR_INVALID_URL_SCHEME(env->isolate()));
-
+    THROW_ERR_INVALID_URL_SCHEME(env);
     return false;
   }
 
   std::string_view pathname = file_url.get_pathname();
 #ifdef _WIN32
-  size_t first_percent = std::string::npos;
   size_t pathname_size = pathname.size();
-  std::string pathname_escaped_slash;
+  std::string pathname_escaped_slash{};
+  pathname_escaped_slash.reserve(pathname_size);
 
   for (size_t i = 0; i < pathname_size; i++) {
     if (pathname[i] == '/') {
       pathname_escaped_slash += '\\';
-    } else {
-      pathname_escaped_slash += pathname[i];
+      continue;
     }
 
-    if (pathname[i] != '%') continue;
-
-    if (first_percent == std::string::npos) {
-      first_percent = i;
-    }
-
-    // just safe-guard against access the pathname
-    // outside the bounds
-    if ((i + 2) >= pathname_size) continue;
-
-    char third = pathname[i + 2] | 0x20;
-
-    bool is_slash = pathname[i + 1] == '2' && third == 102;
-    bool is_forward_slash = pathname[i + 1] == '5' && third == 99;
+    pathname_escaped_slash += pathname[i];
 
-    if (!is_slash && !is_forward_slash) continue;
+    if (pathname[i] == '%' && (i + 2) <= pathname_size) {
+      char third = pathname[i + 2] | 0x20;
+      bool is_slash = pathname[i + 1] == '2' && third == 102;
+      bool is_forward_slash = pathname[i + 1] == '5' && third == 99;
 
-    env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
-        env->isolate(),
-        "File URL path must not include encoded \\ or / characters"));
-
-    return false;
+      if (is_slash || is_forward_slash) {
+        THROW_ERR_INVALID_FILE_URL_PATH(
+            env, "File URL path must not include encoded \\ or / characters");
+        return false;
+      }
+    }
   }
 
-  std::string_view hostname = file_url.get_hostname();
   std::string decoded_pathname = ada::unicode::percent_decode(
-      std::string_view(pathname_escaped_slash), first_percent);
+      pathname_escaped_slash, pathname_escaped_slash.find('%'));
 
-  if (hostname.size() > 0) {
+  if (!file_url.has_empty_hostname()) {
     // If hostname is set, then we have a UNC path
     // Pass the hostname through domainToUnicode just in case
     // it is an IDN using punycode encoding. We do not need to worry
     // about percent encoding because the URL parser will have
     // already taken care of that for us. Note that this only
     // causes IDNs with an appropriate `xn--` prefix to be decoded.
-    result_file_path =
-        "\\\\" + ada::unicode::to_unicode(hostname) + decoded_pathname;
-
+    result_file_path = "\\\\" +
+                       ada::unicode::to_unicode(file_url.get_hostname()) +
+                       decoded_pathname;
     return true;
   }
 
@@ -2801,42 +2786,31 @@ static bool FileURLToPath(
 
   // a..z A..Z
   if (letter < 'a' || letter > 'z' || sep != ':') {
-    env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
-        env->isolate(), "File URL path must be absolute"));
-
+    THROW_ERR_INVALID_FILE_URL_PATH(env, "File URL path must be absolute");
     return false;
   }
 
   result_file_path = decoded_pathname.substr(1);
 
   return true;
 #else   // _WIN32
-  std::string_view hostname = file_url.get_hostname();
-
-  if (hostname.size() > 0) {
+  if (!file_url.has_empty_hostname()) {
     std::string error_message =
-        std::string("File URL host must be \"localhost\" or empty on ") +
+        "File URL host must be \"localhost\" or empty on " +
         std::string(per_process::metadata.platform);
-    env->isolate()->ThrowException(
-        ERR_INVALID_FILE_URL_HOST(env->isolate(), error_message.c_str()));
-
+    THROW_ERR_INVALID_FILE_URL_HOST(env, error_message.c_str());
     return false;
   }
 
-  size_t first_percent = std::string::npos;
-  for (size_t i = 0; (i + 2) < pathname.size(); i++) {
-    if (pathname[i] != '%') continue;
-
-    if (first_percent == std::string::npos) {
-      first_percent = i;
-    }
-
-    if (pathname[i + 1] == '2' && (pathname[i + 2] | 0x20) == 102) {
-      env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
-          env->isolate(),
-          "File URL path must not include encoded / characters"));
+  auto first_percent = pathname.find('%');
 
-      return false;
+  if (first_percent != std::string_view::npos) {
+    for (size_t i = first_percent; (i + 2) < pathname.size(); i++) {
+      if (pathname[i + 1] == '2' && (pathname[i + 2] | 0x20) == 102) {
+        THROW_ERR_INVALID_FILE_URL_PATH(
+            env, "File URL path must not include encoded / characters");
+        return false;
+      }
     }
   }
 
@@ -2847,16 +2821,16 @@ static bool FileURLToPath(
 }
 
 BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
-    Environment* env, const std::string& file_path) {
+    Environment* env, const std::string_view file_path) {
   THROW_IF_INSUFFICIENT_PERMISSIONS(
       env,
       permission::PermissionScope::kFileSystemRead,
       file_path,
-      BindingData::FilePathIsFileReturnType::kThrowInsufficientPermissions);
+      FilePathIsFileReturnType::kThrowInsufficientPermissions);
 
   uv_fs_t req;
 
-  int rc = uv_fs_stat(env->event_loop(), &req, file_path.c_str(), nullptr);
+  int rc = uv_fs_stat(env->event_loop(), &req, file_path.data(), nullptr);
 
   if (rc == 0) {
     const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
@@ -2871,22 +2845,6 @@ BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
   return BindingData::FilePathIsFileReturnType::kIsNotFile;
 }
 
-// the possible file extensions that should be tested
-// 0-6: when packageConfig.main is defined
-// 7-9: when packageConfig.main is NOT defined,
-//      or when the previous case didn't found the file
-const std::array<std::string, 10> BindingData::legacy_main_extensions = {
-    "",
-    ".js",
-    ".json",
-    ".node",
-    "/index.js",
-    "/index.json",
-    "/index.node",
-    ".js",
-    ".json",
-    ".node"};
-
 void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
   CHECK(args[0]->IsString());
@@ -2898,38 +2856,32 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
       ada::parse<ada::url_aggregator>(utf8_package_json_url.ToStringView());
 
   if (!package_json_url) {
-    env->isolate()->ThrowException(
-        ERR_INVALID_URL(env->isolate(), "Invalid URL"));
-
+    THROW_ERR_INVALID_URL(env, "Invalid URL");
     return;
   }
 
   ada::result<ada::url_aggregator> file_path_url;
   std::string initial_file_path;
-  std::string file_path;
 
-  if (args.Length() >= 2 && !args[1]->IsNullOrUndefined() &&
-      args[1]->IsString()) {
+  if (args.Length() >= 2 && args[1]->IsString()) {
     std::string package_config_main =
         Utf8Value(env->isolate(), args[1].As<String>()).ToString();
 
-    file_path_url = ada::parse<ada::url_aggregator>(
-        std::string("./") + package_config_main, &package_json_url.value());
+    file_path_url = ada::parse<ada::url_aggregator>("./" + package_config_main,
+                                                    &package_json_url.value());
 
     if (!file_path_url) {
-      env->isolate()->ThrowException(
-          ERR_INVALID_URL(env->isolate(), "Invalid URL"));
-
+      THROW_ERR_INVALID_URL(env, "Invalid URL");
       return;
     }
 
-    if (!FileURLToPath(env, file_path_url.value(), initial_file_path)) return;
+    if (!FileURLToPath(env, *file_path_url, initial_file_path)) return;
 
-    FromNamespacedPath(&initial_file_path);
+    FromNamespacedPath(initial_file_path);
 
-    for (int i = 0; i < BindingData::legacy_main_extensions_with_main_end;
-         i++) {
-      file_path = initial_file_path + BindingData::legacy_main_extensions[i];
+    for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
+      auto file_path = initial_file_path +
+                       std::string(BindingData::legacy_main_extensions[i]);
 
       switch (FilePathIsFile(env, file_path)) {
         case BindingData::FilePathIsFileReturnType::kIsFile:
@@ -2952,20 +2904,19 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
       ada::parse<ada::url_aggregator>("./index", &package_json_url.value());
 
   if (!file_path_url) {
-    env->isolate()->ThrowException(
-        ERR_INVALID_URL(env->isolate(), "Invalid URL"));
-
+    THROW_ERR_INVALID_URL(env, "Invalid URL");
     return;
   }
 
-  if (!FileURLToPath(env, file_path_url.value(), initial_file_path)) return;
+  if (!FileURLToPath(env, *file_path_url, initial_file_path)) return;
 
-  FromNamespacedPath(&initial_file_path);
+  FromNamespacedPath(initial_file_path);
 
-  for (int i = BindingData::legacy_main_extensions_with_main_end;
-       i < BindingData::legacy_main_extensions_package_fallback_end;
+  for (int i = legacy_main_extensions_with_main_end;
+       i < legacy_main_extensions_package_fallback_end;
        i++) {
-    file_path = initial_file_path + BindingData::legacy_main_extensions[i];
+    auto file_path =
+        initial_file_path + std::string(BindingData::legacy_main_extensions[i]);
 
     switch (FilePathIsFile(env, file_path)) {
       case BindingData::FilePathIsFileReturnType::kIsFile:
@@ -2987,32 +2938,27 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
 
   if (!FileURLToPath(env, package_json_url.value(), module_path)) return;
 
-  if (args.Length() >= 3 && !args[2]->IsNullOrUndefined() &&
-      args[2]->IsString()) {
+  if (args.Length() >= 3 && args[2]->IsString()) {
     Utf8Value utf8_base_path(env->isolate(), args[2].As<String>());
     auto base_url =
         ada::parse<ada::url_aggregator>(utf8_base_path.ToStringView());
 
     if (!base_url) {
-      env->isolate()->ThrowException(
-          ERR_INVALID_URL(env->isolate(), "Invalid URL"));
-
+      THROW_ERR_INVALID_URL(env->isolate(), "Invalid URL");
       return;
     }
 
     if (!FileURLToPath(env, base_url.value(), module_base)) return;
-  } else {
-    std::string err_arg_message =
-        "The \"base\" argument must be of type string or an instance of URL.";
-    env->isolate()->ThrowException(
-        ERR_INVALID_ARG_TYPE(env->isolate(), err_arg_message.c_str()));
+
+    std::string err_module_message = "Cannot find package '" + module_path +
+                                     "' imported from " + module_base;
+    THROW_ERR_MODULE_NOT_FOUND(env, err_module_message.c_str());
     return;
   }
 
-  std::string err_module_message =
-      "Cannot find package '" + module_path + "' imported from " + module_base;
-  env->isolate()->ThrowException(
-      ERR_MODULE_NOT_FOUND(env->isolate(), err_module_message.c_str()));
+  THROW_ERR_INVALID_ARG_TYPE(
+      env,
+      "The \"base\" argument must be of type string or an instance of URL.");
 }
 
 void BindingData::MemoryInfo(MemoryTracker* tracker) const {
diff --git a/src/node_file.h b/src/node_file.h
@@ -100,10 +100,24 @@ class BindingData : public SnapshotableObject {
  private:
   InternalFieldInfo* internal_field_info_ = nullptr;
 
-  static FilePathIsFileReturnType FilePathIsFile(Environment* env,
-                                                 const std::string& file_path);
-
-  static const std::array<std::string, 10> legacy_main_extensions;
+  static FilePathIsFileReturnType FilePathIsFile(
+      Environment* env, const std::string_view file_path);
+
+  // the possible file extensions that should be tested
+  // 0-6: when packageConfig.main is defined
+  // 7-9: when packageConfig.main is NOT defined,
+  //      or when the previous case didn't found the file
+  static constexpr std::array<std::string_view, 10> legacy_main_extensions = {
+      "",
+      ".js",
+      ".json",
+      ".node",
+      "/index.js",
+      "/index.json",
+      "/index.node",
+      ".js",
+      ".json",
+      ".node"};
   // define the final index of the algorithm resolution
   // when packageConfig.main is defined.
   static const uint8_t legacy_main_extensions_with_main_end = 7;
