Skip to content

Commit

Permalink
fix: reserve paths properly for unicode, windows
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
isaacs committed Aug 12, 2021
1 parent 3aaf19b commit b6162c7
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 5 deletions.
31 changes: 26 additions & 5 deletions lib/path-reservations.js
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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))
)
Expand Down
92 changes: 92 additions & 0 deletions test/path-reservations.js
@@ -1,5 +1,13 @@
const t = require('tap')

// 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 } = t.mock('../lib/path-reservations.js')()

t.test('basic race', t => {
// simulate the race conditions we care about
Expand Down Expand Up @@ -54,3 +62,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)
})

0 comments on commit b6162c7

Please sign in to comment.