From bb93ba243746f705092905da1955ac3b0509ba1e Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 11 Aug 2021 17:47:16 -0700 Subject: [PATCH] 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. --- lib/path-reservations.js | 31 ++++++++++--- test/path-reservations.js | 93 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/lib/path-reservations.js b/lib/path-reservations.js index 48c750e3..b7f6c916 100644 --- 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 index 9ce46b80..c495f310 100644 --- 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) +})