fix: clang-analyzer errors in script loader

This changes script loader paths to use owned string instead of raw
pointers to fix potential memory leaks reported by clang. The tradeof is
that now we call c_str in few places, but the impact on performance
should be negligible since we call this only once.

Author: andreiltd

runtime/script_loader.cpp

@@ -2,22 +2,27 @@
 #include "encode.h"
 
 #include <cstdio>
-#include <iostream>
 #include <js/CompilationAndEvaluation.h>
 #include <js/MapAndSet.h>
 #include <js/Value.h>
 #include <jsfriendapi.h>
 #include <sys/stat.h>
 
+namespace {
+
 static api::Engine* ENGINE;
 static ScriptLoader* SCRIPT_LOADER;
+static bool MODULE_MODE = true;
+static std::string BASE_PATH;
+
 JS::PersistentRootedObject moduleRegistry;
 JS::PersistentRootedObject builtinModules;
-static bool MODULE_MODE = true;
-static char* BASE_PATH = nullptr;
-mozilla::Maybe<std::string> PATH_PREFIX = mozilla::Nothing();
 JS::CompileOptions *COMPILE_OPTS;
 
+mozilla::Maybe<std::string> PATH_PREFIX = mozilla::Nothing();
+
+} // namespace
+
 using host_api::HostString;
 
 namespace ScriptLoaderErrors {
@@ -40,107 +45,92 @@ class AutoCloseFile {
       success = !fclose(file);
     }
     file = nullptr;
+    // Clang analyzer complains about the stream leaking when stream is stdin, stdout, or stderr.
+    // NOLINTNEXTLINE(clang-analyzer-unix.Stream)
     return success;
   }
 };
 
 // strip off the given prefix when possible for nicer debugging stacks
-static const char* strip_prefix(const char* resolved_path,
+static std::string strip_prefix(std::string_view resolved_path,
                                 mozilla::Maybe<std::string> path_prefix) {
   if (!path_prefix) {
-    return strdup(resolved_path);
+    return std::string(resolved_path);
   }
-  auto& base = *path_prefix;
-  if (strncmp(resolved_path, base.data(), base.length()) != 0) {
-    return strdup(resolved_path);
+
+  const auto& base = *path_prefix;
+  if (resolved_path.rfind(base, 0) != 0) {
+    return std::string(resolved_path);
   }
-  size_t path_len = strlen(resolved_path);
-  char* buf(js_pod_malloc<char>(path_len - base.length() + 1));
-  strncpy(buf, &resolved_path[base.length()], path_len - base.length() + 1);
-  return buf;
+
+  return std::string(resolved_path.substr(base.size()));
 }
 
 struct stat s;
 
-static const char* resolve_extension(const char* resolved_path) {
-  if (stat(resolved_path, &s) == 0) {
+
+static std::string resolve_extension(std::string resolved_path) {
+  if (stat(resolved_path.c_str(), &s) == 0) {
     return resolved_path;
   }
-  size_t len = strlen(resolved_path);
-  if (strncmp(resolved_path + len - 3, ".js", 3) == 0) {
+
+  if (resolved_path.size() >= 3 &&
+      resolved_path.compare(resolved_path.size() - 3, 3, ".js") == 0) {
     return resolved_path;
   }
-  char* resolved_path_ext = new char[len + 4];
-  strncpy(resolved_path_ext, resolved_path, len);
-  strncpy(resolved_path_ext + len, ".js", 4);
-  MOZ_ASSERT(strlen(resolved_path_ext) == len + 3);
-  if (stat(resolved_path_ext, &s) == 0) {
-    delete[] resolved_path;
-    return resolved_path_ext;
+
+  std::string with_ext = resolved_path + ".js";
+  if (stat(with_ext.c_str(), &s) == 0) {
+    return with_ext;
   }
-  delete[] resolved_path_ext;
   return resolved_path;
 }
 
-static const char* resolve_path(const char* path, const char* base, size_t base_len) {
-  MOZ_ASSERT(base);
-  while (base_len > 0 && base[base_len - 1] != '/') {
-    base_len--;
+static std::string resolve_path(std::string_view path,
+                                std::string_view base) {
+  // Prepare initial resolved (with trailing '/')
+  std::string resolved;
+  if (!path.empty() && path[0] == '/') {
+    // absolute path
+  } else {
+    resolved.assign(base);
+    auto slash = resolved.find_last_of('/');
+    if (slash != std::string::npos) {
+      resolved.resize(slash + 1);
+    }
   }
-  size_t path_len = strlen(path);
-
-  // create the maximum buffer size as a working buffer
-  char* resolved_path = new char[base_len + path_len + 1];
 
-  // copy the base in if used
-  size_t resolved_len = base_len;
-  if (path[0] == '/') {
-    resolved_len = 0;
-  } else {
-    strncpy(resolved_path, base, base_len);
+  // Split into segments
+  std::vector<std::string_view> parts;
+  size_t start = 0;
+  while (start <= path.size()) {
+    auto end = path.find('/', start);
+    if (end == std::string_view::npos) end = path.size();
+    parts.emplace_back(path.substr(start, end - start));
+    start = end + 1;
   }
 
   // Iterate through each segment of the path, copying each segment into the resolved path,
   // while handling backtracking for .. segments and skipping . segments appropriately.
-  size_t path_from_idx = 0;
-  size_t path_cur_idx = 0;
-  while (path_cur_idx < path_len) {
-    // read until the end or the next / to get the segment position
-    // as the substring between path_from_idx and path_cur_idx
-    while (path_cur_idx < path_len && path[path_cur_idx] != '/')
-      path_cur_idx++;
-    if (path_cur_idx == path_from_idx)
-      break;
-    // . segment to skip
-    if (path_cur_idx - path_from_idx == 1 && path[path_from_idx] == '.') {
-      path_cur_idx++;
-      path_from_idx = path_cur_idx;
+  std::string result = resolved;
+  for (auto seg : parts) {
+    if (seg.empty() || seg == ".") {
       continue;
     }
-    // .. segment backtracking
-    if (path_cur_idx - path_from_idx == 2 && path[path_from_idx] == '.' && path[path_from_idx + 1] == '.') {
-      path_cur_idx++;
-      path_from_idx = path_cur_idx;
-      if (resolved_len > 0 && resolved_path[resolved_len - 1] == '/') {
-        resolved_len --;
+    if (seg == "..") {
+      auto pos = result.find_last_of('/');
+      if (pos != std::string::npos) {
+        result.resize(pos);
       }
-      while (resolved_len > 0 && resolved_path[resolved_len - 1] != '/') {
-        resolved_len--;
+    } else {
+      if (!result.empty() && result.back() != '/') {
+        result.push_back('/');
       }
-      continue;
+      result.append(seg);
     }
-    // normal segment to copy (with the trailing / if not the last segment)
-    if (path[path_cur_idx] == '/')
-      path_cur_idx++;
-    strncpy(resolved_path + resolved_len, path + path_from_idx, path_cur_idx - path_from_idx);
-    resolved_len += path_cur_idx - path_from_idx;
-    path_from_idx = path_cur_idx;
-  }
-
-  // finalize the buffer
-  resolved_path[resolved_len] = '\0';
-  MOZ_ASSERT(strlen(resolved_path) == resolved_len);
-  return resolve_extension(resolved_path);
+  }
+
+  return resolve_extension(result);
 }
 
 static JSObject* get_module(JSContext* cx, JS::SourceText<mozilla::Utf8Unit> &source,
@@ -178,11 +168,12 @@ static JSObject* get_module(JSContext* cx, JS::SourceText<mozilla::Utf8Unit> &so
 static JSObject* get_module(JSContext* cx, const char* specifier, const char* resolved_path,
                             const JS::CompileOptions &opts) {
   RootedString resolved_path_str(cx, JS_NewStringCopyZ(cx, resolved_path));
+
   if (!resolved_path_str) {
     return nullptr;
   }
-  RootedValue resolved_path_val(cx, StringValue(resolved_path_str));
 
+  RootedValue resolved_path_val(cx, StringValue(resolved_path_str));
   RootedValue module_val(cx);
   if (!JS::MapGet(cx, moduleRegistry, resolved_path_val, &module_val)) {
     return nullptr;
@@ -327,11 +318,12 @@ JSObject* module_resolve_hook(JSContext* cx, HandleValue referencingPrivate,
   }
 
   HostString str = core::encode(cx, parent_path_val);
-  const char* resolved_path = resolve_path(path.get(), str.ptr.get(), str.len);
+  auto resolved_path = resolve_path(path.get(), str.ptr.get());
 
   JS::CompileOptions opts(cx, *COMPILE_OPTS);
-  opts.setFileAndLine(strip_prefix(resolved_path, PATH_PREFIX), 1);
-  return get_module(cx, path.get(), resolved_path, opts);
+  auto stripped = strip_prefix(resolved_path, PATH_PREFIX);
+  opts.setFileAndLine(stripped.c_str(), 1);
+  return get_module(cx, path.get(), resolved_path.c_str(), opts);
 }
 
 bool module_metadata_hook(JSContext* cx, HandleValue referencingPrivate, HandleObject metaObject) {
@@ -432,34 +424,31 @@ bool ScriptLoader::load_resolved_script(JSContext *cx, const char *specifier,
 
 bool ScriptLoader::load_script(JSContext *cx, const char *script_path,
                                JS::SourceText<mozilla::Utf8Unit> &script) {
-  const char *resolved_path;
-  if (!BASE_PATH) {
-    auto last_slash = strrchr(script_path, '/');
-    size_t base_len;
-    if (last_slash) {
-      last_slash++;
-      base_len = last_slash - script_path;
-      BASE_PATH = new char[base_len + 1];
-      MOZ_ASSERT(BASE_PATH);
-      strncpy(BASE_PATH, script_path, base_len);
-      BASE_PATH[base_len] = '\0';
+
+  std::string path(script_path);
+  std::string resolved;
+  if (BASE_PATH.empty()) {
+    auto pos = path.find_last_of('/');
+    if (pos != std::string::npos) {
+      BASE_PATH = path.substr(0, pos + 1);
     } else {
-      BASE_PATH = strdup("./");
+      BASE_PATH = "./";
     }
-    resolved_path = script_path;
+    resolved = path;
   } else {
-    resolved_path = resolve_path(script_path, BASE_PATH, strlen(BASE_PATH));
+    resolved = resolve_path(path, BASE_PATH);
   }
-
-  return load_resolved_script(cx, script_path, resolved_path, script);
+  return load_resolved_script(cx, script_path, resolved.c_str(), script);
 }
 
 bool ScriptLoader::eval_top_level_script(const char *path, JS::SourceText<mozilla::Utf8Unit> &source,
                                          MutableHandleValue result, MutableHandleValue tla_promise) {
   JSContext *cx = ENGINE->cx();
 
   JS::CompileOptions opts(cx, *COMPILE_OPTS);
-  opts.setFileAndLine(strip_prefix(path, PATH_PREFIX), 1);
+  auto stripped = strip_prefix(path, PATH_PREFIX);
+  opts.setFileAndLine(stripped.c_str(), 1);
+
   JS::RootedScript script(cx);
   RootedObject module(cx);
   if (MODULE_MODE) {