src: optimize NormalizeString to cut allocations and copies

src/path.cc

@@ -21,8 +21,15 @@ constexpr bool IsPathSeparator(const char c) noexcept {
 std::string NormalizeString(const std::string_view path,
                             bool allowAboveRoot,
                             const std::string_view separator) {
+  const char separator_char = separator[0];
   std::string res;
-  int lastSegmentLength = 0;
+  res.reserve(path.size());
+  // `dotdot` is the floor in `res` below which a `..` segment cannot
+  // backtrack: it sits just past any leading `..` run that could not be
+  // resolved. This lets `..` rewind the write position to the preceding
+  // separator without rescanning or reallocating the whole prefix. Same
+  // approach as Go's path/filepath.Clean.
+  int dotdot = 0;
   int lastSlash = -1;
   int dots = 0;
   char code = 0;
@@ -37,45 +44,35 @@ std::string NormalizeString(const std::string_view path,
 
     if (IsPathSeparator(code)) {
       if (lastSlash == static_cast<int>(i - 1) || dots == 1) {
-        // NOOP
+        // NOOP: empty segment (e.g. `//`) or a `.` segment.
       } else if (dots == 2) {
-        int len = res.length();
-        if (len < 2 || lastSegmentLength != 2 || res[len - 1] != '.' ||
-            res[len - 2] != '.') {
-          if (len > 2) {
-            auto lastSlashIndex = res.find_last_of(separator);
-            if (lastSlashIndex == std::string::npos) {
-              res = "";
-              lastSegmentLength = 0;
-            } else {
-              res = res.substr(0, lastSlashIndex);
-              len = res.length();
-              lastSegmentLength = len - 1 - res.find_last_of(separator);
-            }
-            lastSlash = i;
-            dots = 0;
-            continue;
-          } else if (len != 0) {
-            res = "";
-            lastSegmentLength = 0;
-            lastSlash = i;
-            dots = 0;
-            continue;
+        int w = static_cast<int>(res.length());
+        if (w > dotdot) {
+          // Drop the previous segment by rewinding the write position to the
+          // separator that precedes it.
+          w--;
+          while (w > dotdot && res[w] != separator_char) {
+            w--;
           }
-        }
-
-        if (allowAboveRoot) {
-          res += res.length() > 0 ? std::string(separator) + ".." : "..";
-          lastSegmentLength = 2;
+          res.resize(static_cast<size_t>(w));
+          lastSlash = i;
+          dots = 0;
+          continue;
+        } else if (allowAboveRoot) {
+          // Cannot backtrack past the floor; keep the `..` and raise the floor.
+          if (!res.empty()) {
+            res.push_back(separator_char);
+          }
+          res.append("..", 2);
+          dotdot = static_cast<int>(res.length());
         }
       } else {
         if (!res.empty()) {
-          res += std::string(separator) +
-                 std::string(path.substr(lastSlash + 1, i - (lastSlash + 1)));
+          res.push_back(separator_char);
+          res.append(path.data() + lastSlash + 1, i - (lastSlash + 1));
         } else {
-          res = path.substr(lastSlash + 1, i - (lastSlash + 1));
+          res.assign(path.data() + lastSlash + 1, i - (lastSlash + 1));
         }
-        lastSegmentLength = i - lastSlash - 1;
       }
       lastSlash = i;
       dots = 0;

test/cctest/test_path.cc

@@ -8,6 +8,7 @@
 #include "v8.h"
 
 using node::BufferValue;
+using node::NormalizeString;
 using node::PathResolve;
 using node::ToNamespacedPath;
 
@@ -43,6 +44,13 @@ TEST_F(PathTest, PathResolve) {
             "\\\\.\\PHYSICALDRIVE0");
   EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
             "\\\\?\\PHYSICALDRIVE0");
+  // Backtracking past the drive root stays clamped at the drive root.
+  EXPECT_EQ(PathResolve(*env, {"c:/a/b/c", "..\\..\\..\\.."}), "c:\\");
+  // UNC root is preserved when backtracking past it. The UNC share
+  // \\server\share is the root, so "..","..","x" cannot escape it and the
+  // remaining segment "x" is appended to the share root.
+  EXPECT_EQ(PathResolve(*env, {"//server/share", "..", "..", "x"}),
+            "\\\\server\\share\\x");
 #else
   EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
   EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file");
@@ -51,9 +59,42 @@ TEST_F(PathTest, PathResolve) {
   EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute");
   EXPECT_EQ(PathResolve(*env, {"/foo/tmp.3/", "../tmp.3/cycles/root.js"}),
             "/foo/tmp.3/cycles/root.js");
+  // Backtracking past the root stays clamped at the root.
+  EXPECT_EQ(PathResolve(*env, {"/a/b/c/d/e", "../../../../.."}), "/");
+  EXPECT_EQ(PathResolve(*env, {"/a/b/c", "../../../../.."}), "/");
+  // Mixed current-dir and parent-dir segments.
+  EXPECT_EQ(PathResolve(*env, {"/a/./b/../c/./d"}), "/a/c/d");
+  // Collapsing of repeated separators.
+  EXPECT_EQ(PathResolve(*env, {"/a//b///c"}), "/a/b/c");
+  // Single parent-dir traversal.
+  EXPECT_EQ(PathResolve(*env, {"/a/../b"}), "/b");
+  EXPECT_EQ(PathResolve(*env, {"/a/b/../../c"}), "/c");
+  // Trailing separator is stripped.
+  EXPECT_EQ(PathResolve(*env, {"/a/b/c/"}), "/a/b/c");
+  // Single absolute segment.
+  EXPECT_EQ(PathResolve(*env, {"/single"}), "/single");
 #endif
 }
 
+TEST_F(PathTest, NormalizeString) {
+  // allowAboveRoot = false (absolute context): ".." that cannot be resolved is
+  // dropped, "." segments and repeated/trailing separators are collapsed.
+  EXPECT_EQ(NormalizeString("a/b/../../../c", false, "/"), "c");
+  EXPECT_EQ(NormalizeString("a/b/c/d/e/../../../../..", false, "/"), "");
+  EXPECT_EQ(NormalizeString("a/./b//c/", false, "/"), "a/b/c");
+  EXPECT_EQ(NormalizeString("./foo/./bar/", false, "/"), "foo/bar");
+  // allowAboveRoot = true (relative context): leading ".." is preserved.
+  EXPECT_EQ(NormalizeString("a/b/../../../c", true, "/"), "../c");
+  EXPECT_EQ(NormalizeString("../../a", true, "/"), "../../a");
+  EXPECT_EQ(NormalizeString("foo/..", true, "/"), "");
+  EXPECT_EQ(NormalizeString("foo/../..", true, "/"), "..");
+#ifdef _WIN32
+  // The Windows separator is handled the same way.
+  EXPECT_EQ(NormalizeString("a\\b\\..\\..\\..\\c", false, "\\"), "c");
+  EXPECT_EQ(NormalizeString("..\\..\\a", true, "\\"), "..\\..\\a");
+#endif  // _WIN32
+}
+
 TEST_F(PathTest, ToNamespacedPath) {
   const v8::HandleScope handle_scope(isolate_);
   Argv argv;