fix: reserve paths properly for unicode, windows

This updates the path reservation system such that it will properly await any paths that match based on unicode normalization. On windows, because 8.3 shortnames can collide in ways that are undetectable by any reasonable means, all unpack parallelization is simply disabled.
isaacs · Aug 12, 2021 · bb93ba2 · bb93ba2
1 parent 2f1bca0
commit bb93ba2
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 5 deletions.
diff --git a/lib/path-reservations.js b/lib/path-reservations.js
@@ -8,8 +8,12 @@
 
 const assert = require('assert')
 const normPath = require('./normalize-windows-path.js')
+const stripSlashes = require('./strip-trailing-slashes.js')
 const { join } = require('path')
 
+const platform = process.env.TESTING_TAR_FAKE_PLATFORM || process.platform
+const isWindows = platform === 'win32'
+
 module.exports = () => {
   // path => [function or Set]
   // A Set object means a directory reservation
@@ -20,10 +24,16 @@ module.exports = () => {
   const reservations = new Map()
 
   // return a set of parent dirs for a given path
-  const getDirs = path =>
-    path.split('/').slice(0, -1).reduce((set, path) =>
-      set.length ? set.concat(normPath(join(set[set.length - 1], path)))
-      : [path], [])
+  // '/a/b/c/d' -> ['/', '/a', '/a/b', '/a/b/c', '/a/b/c/d']
+  const getDirs = path => {
+    const dirs = path.split('/').slice(0, -1).reduce((set, path) => {
+      if (set.length)
+        path = normPath(join(set[set.length - 1], path))
+      set.push(path || '/')
+      return set
+    }, [])
+    return dirs
+  }
 
   // functions currently running
   const running = new Set()
@@ -99,7 +109,18 @@ module.exports = () => {
   }
 
   const reserve = (paths, fn) => {
-    paths = paths.map(p => normPath(join(p)).toLowerCase())
+    // collide on matches across case and unicode normalization
+    // On windows, thanks to the magic of 8.3 shortnames, it is fundamentally
+    // impossible to determine whether two paths refer to the same thing on
+    // disk, without asking the kernel for a shortname.
+    // So, we just pretend that every path matches every other path here,
+    // effectively removing all parallelization on windows.
+    paths = isWindows ? ['win32 parallelization disabled'] : paths.map(p => {
+      return stripSlashes(normPath(join(p)))
+        .normalize('NFKD')
+        .toLowerCase()
+    })
+
     const dirs = new Set(
       paths.map(path => getDirs(path)).reduce((a, b) => a.concat(b))
     )

diff --git a/test/path-reservations.js b/test/path-reservations.js
@@ -1,5 +1,14 @@
 const t = require('tap')
+const requireInject = require('require-inject')
+
+// load up the posix and windows versions of the reserver
+if (process.platform === 'win32')
+  process.env.TESTING_TAR_FAKE_PLATFORM = 'posix'
 const { reserve } = require('../lib/path-reservations.js')()
+delete process.env.TESTING_TAR_FAKE_PLATFORM
+if (process.platform !== 'win32')
+  process.env.TESTING_TAR_FAKE_PLATFORM = 'win32'
+const { reserve: winReserve } = requireInject('../lib/path-reservations.js')()
 
 t.test('basic race', t => {
   // simulate the race conditions we care about
@@ -54,3 +63,87 @@ t.test('basic race', t => {
   t.notOk(reserve(['a/b'], dir2), 'dir2 waits')
   t.notOk(reserve(['a/b/x'], dir3), 'dir3 waits')
 })
+
+t.test('unicode shenanigans', t => {
+  const e1 = Buffer.from([0xc3, 0xa9])
+  const e2 = Buffer.from([0x65, 0xcc, 0x81])
+  let didCafe1 = false
+  const cafe1 = done => {
+    t.equal(didCafe1, false, 'did cafe1 only once')
+    t.equal(didCafe2, false, 'did cafe1 before cafe2')
+    didCafe1 = true
+    setTimeout(done)
+  }
+  let didCafe2 = false
+  const cafe2 = done => {
+    t.equal(didCafe1, true, 'did cafe1 before cafe2')
+    t.equal(didCafe2, false, 'did cafe2 only once')
+    didCafe2 = true
+    done()
+    t.end()
+  }
+  const cafePath1 = `c/a/f/${e1}`
+  const cafePath2 = `c/a/f/${e2}`
+  t.ok(reserve([cafePath1], cafe1))
+  t.notOk(reserve([cafePath2], cafe2))
+})
+
+t.test('absolute paths and trailing slash', t => {
+  let calledA1 = false
+  let calledA2 = false
+  const a1 = done => {
+    t.equal(calledA1, false, 'called a1 only once')
+    t.equal(calledA2, false, 'called a1 before 2')
+    calledA1 = true
+    setTimeout(done)
+  }
+  const a2 = done => {
+    t.equal(calledA1, true, 'called a1 before 2')
+    t.equal(calledA2, false, 'called a2 only once')
+    calledA2 = true
+    done()
+    if (calledR2)
+      t.end()
+  }
+  let calledR1 = false
+  let calledR2 = false
+  const r1 = done => {
+    t.equal(calledR1, false, 'called r1 only once')
+    t.equal(calledR2, false, 'called r1 before 2')
+    calledR1 = true
+    setTimeout(done)
+  }
+  const r2 = done => {
+    t.equal(calledR1, true, 'called r1 before 2')
+    t.equal(calledR2, false, 'called r1 only once')
+    calledR2 = true
+    done()
+    if (calledA2)
+      t.end()
+  }
+  t.ok(reserve(['/p/a/t/h'], a1))
+  t.notOk(reserve(['/p/a/t/h/'], a2))
+  t.ok(reserve(['p/a/t/h'], r1))
+  t.notOk(reserve(['p/a/t/h/'], r2))
+})
+
+t.test('on windows, everything collides with everything', t => {
+  const reserve = winReserve
+  let called1 = false
+  let called2 = false
+  const f1 = done => {
+    t.equal(called1, false, 'only call 1 once')
+    t.equal(called2, false, 'call 1 before 2')
+    called1 = true
+    setTimeout(done)
+  }
+  const f2 = done => {
+    t.equal(called1, true, 'call 1 before 2')
+    t.equal(called2, false, 'only call 2 once')
+    called2 = true
+    done()
+    t.end()
+  }
+  t.equal(reserve(['some/path'], f1), true)
+  t.equal(reserve(['other/path'], f2), false)
+})