path: fix path.normalize not correctly normalizing relative paths

After slicing, the `lastSegmentLength` should be calculated again, instead of assigning value `j`. Fixes: #17928
nodejs · Jan 4, 2018 · b1aba4a · b1aba4a
1 parent f51067a
commit b1aba4a
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 20 deletions.
diff --git a/lib/path.js b/lib/path.js
@@ -29,6 +29,17 @@ function assertPath(path) {
   }
 }
 
+function findLastSlashIndex(path, isWin32) {
+  var i = path.length - 1;
+  for (; i >= 0; i--) {
+    const charCode = path.charCodeAt(i);
+    if ((!isWin32 && charCode === 47/*/*/) ||
+        (isWin32 && charCode === 92/*\*/))
+      break;
+  }
+  return i;
+}
+
 // Resolves . and .. elements in a path with directory names
 function normalizeStringWin32(path, allowAboveRoot) {
   var res = '';
@@ -51,19 +62,15 @@ function normalizeStringWin32(path, allowAboveRoot) {
             res.charCodeAt(res.length - 1) !== 46/*.*/ ||
             res.charCodeAt(res.length - 2) !== 46/*.*/) {
           if (res.length > 2) {
-            const start = res.length - 1;
-            var j = start;
-            for (; j >= 0; --j) {
-              if (res.charCodeAt(j) === 92/*\*/)
-                break;
-            }
-            if (j !== start) {
-              if (j === -1) {
+            const lastSlashIndex = findLastSlashIndex(res, true);
+            if (lastSlashIndex !== res.length - 1) {
+              if (lastSlashIndex === -1) {
                 res = '';
                 lastSegmentLength = 0;
               } else {
-                res = res.slice(0, j);
-                lastSegmentLength = j;
+                res = res.slice(0, lastSlashIndex);
+                lastSegmentLength = res.length - 1 -
+                                    findLastSlashIndex(res, true);
               }
               lastSlash = i;
               dots = 0;
@@ -124,19 +131,14 @@ function normalizeStringPosix(path, allowAboveRoot) {
             res.charCodeAt(res.length - 1) !== 46/*.*/ ||
             res.charCodeAt(res.length - 2) !== 46/*.*/) {
           if (res.length > 2) {
-            const start = res.length - 1;
-            var j = start;
-            for (; j >= 0; --j) {
-              if (res.charCodeAt(j) === 47/*/*/)
-                break;
-            }
-            if (j !== start) {
-              if (j === -1) {
+            const lastSlashIndex = findLastSlashIndex(res);
+            if (lastSlashIndex !== res.length - 1) {
+              if (lastSlashIndex === -1) {
                 res = '';
                 lastSegmentLength = 0;
               } else {
-                res = res.slice(0, j);
-                lastSegmentLength = j;
+                res = res.slice(0, lastSlashIndex);
+                lastSegmentLength = res.length - 1 - findLastSlashIndex(res);
               }
               lastSlash = i;
               dots = 0;

diff --git a/test/parallel/test-path-normalize.js b/test/parallel/test-path-normalize.js
@@ -27,6 +27,18 @@ assert.strictEqual(path.win32.normalize('..\\foo..\\..\\..\\bar'),
                    '..\\..\\bar');
 assert.strictEqual(path.win32.normalize('..\\...\\..\\.\\...\\..\\..\\bar'),
                    '..\\..\\bar');
+assert.strictEqual(path.win32.normalize('../../../foo/../../../bar'),
+                   '..\\..\\..\\..\\..\\bar');
+assert.strictEqual(path.win32.normalize('../../../foo/../../../bar/../../'),
+                   '..\\..\\..\\..\\..\\..\\');
+assert.strictEqual(
+  path.win32.normalize('../foobar/barfoo/foo/../../../bar/../../'),
+  '..\\..\\'
+);
+assert.strictEqual(
+  path.win32.normalize('../.../../foobar/../../../bar/../../baz'),
+  '..\\..\\..\\..\\baz'
+);
 
 assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'),
                    'fixtures/b/c.js');
@@ -44,3 +56,15 @@ assert.strictEqual(path.posix.normalize('bar/foo..'), 'bar/foo..');
 assert.strictEqual(path.posix.normalize('../foo../../../bar'), '../../bar');
 assert.strictEqual(path.posix.normalize('../.../.././.../../../bar'),
                    '../../bar');
+assert.strictEqual(path.posix.normalize('../../../foo/../../../bar'),
+                   '../../../../../bar');
+assert.strictEqual(path.posix.normalize('../../../foo/../../../bar/../../'),
+                   '../../../../../../');
+assert.strictEqual(
+  path.posix.normalize('../foobar/barfoo/foo/../../../bar/../../'),
+  '../../'
+);
+assert.strictEqual(
+  path.posix.normalize('../.../../foobar/../../../bar/../../baz'),
+  '../../../../baz'
+);