Skip to content

Commit 7d4cc17

Browse files
committed
fix race puting a Link ahead of its target File
This avoids a situation in the async Pack class where a detected Link and File pair can end up with the Link ahead of the File in the archive, meaning that it'll throw an error when it tries to unpack, because the hardlink target will not exist yet. Fix: #448
1 parent 26ab904 commit 7d4cc17

File tree

4 files changed

+39
-16
lines changed

4 files changed

+39
-16
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
- Only read from ustar block if not specified in Pax
88
- Fix sync tar.list when file size reduces while reading
99
- Sanitize absolute linkpaths properly
10+
- Prevent writing hardlink entries to the archive ahead of their
11+
file target
1012

1113
## 7.4
1214

src/pack.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export class Pack
7070
extends Minipass<Buffer, ReadEntry | string, WarnEvent<Buffer>>
7171
implements Warner
7272
{
73+
sync: boolean = false
7374
opt: TarOptions
7475
cwd: string
7576
maxReadSize?: number
@@ -286,6 +287,18 @@ export class Pack
286287
// now we have the stat, we can filter it.
287288
if (!this.filter(job.path, stat)) {
288289
job.ignore = true
290+
} else if (
291+
stat.isFile() &&
292+
stat.nlink > 1 &&
293+
job === this[CURRENT] &&
294+
!this.linkCache.get(`${stat.dev}:${stat.ino}`) &&
295+
!this.sync
296+
) {
297+
// if it's not filtered, and it's a new File entry,
298+
// jump the queue in case any pending Link entries are about
299+
// to try to link to it. This prevents a hardlink from coming ahead
300+
// of its target in the archive.
301+
this[PROCESSJOB](job)
289302
}
290303

291304
this[PROCESS]()

test/create.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ t.test('create', t => {
167167
const p = c(['README.md'])
168168
//@ts-expect-error
169169
p.then
170-
//@ts-expect-error
171170
p.sync
172171
t.type(c(['README.md']), Pack)
173172

test/pack.js

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import t from 'tap'
22
import { Pack, PackSync } from '../dist/esm/pack.js'
33
import fs from 'fs'
4-
import path from 'path'
4+
import path, { resolve } from 'path'
55
import { fileURLToPath } from 'url'
66

77
import { Header } from '../dist/esm/header.js'
@@ -92,9 +92,10 @@ t.test('pack a file', t => {
9292
throw new Error('no data!')
9393
}
9494

95+
t.equal(sync.subarray(512).length, data.subarray(512).length)
9596
t.equal(
96-
sync.subarray(512).toString(),
97-
data.subarray(512).toString(),
97+
(sync.subarray(512).toString()),
98+
(data.subarray(512).toString()),
9899
)
99100
const hs = new Header(sync)
100101
t.match(hs, expect)
@@ -1816,14 +1817,22 @@ t.test('prefix and subdirs', t => {
18161817

18171818
// https://github.com/npm/node-tar/issues/284
18181819
t.test('prefix and hard links', t => {
1819-
const dir = path.resolve(fixtures, 'pack-prefix-hardlinks')
1820-
t.teardown(_ => rimraf.sync(dir))
1821-
mkdirp.sync(dir + '/in/z/b/c')
1822-
fs.writeFileSync(dir + '/in/target', 'ddd')
1823-
fs.linkSync(dir + '/in/target', dir + '/in/z/b/c/d')
1824-
fs.linkSync(dir + '/in/target', dir + '/in/z/b/d')
1825-
fs.linkSync(dir + '/in/target', dir + '/in/z/d')
1826-
fs.linkSync(dir + '/in/target', dir + '/in/y')
1820+
const target = resolve(t.testdirName, 'in', 'target')
1821+
const dir = t.testdir({
1822+
in: {
1823+
target: 'ddd',
1824+
z: {
1825+
b: {
1826+
c: {
1827+
d: t.fixture('link', target),
1828+
},
1829+
d: t.fixture('link', target),
1830+
},
1831+
d: t.fixture('link', target),
1832+
},
1833+
y: t.fixture('link', target),
1834+
},
1835+
})
18271836

18281837
const expect = [
18291838
'out/x/\0',
@@ -1901,14 +1910,14 @@ t.test('prefix and hard links', t => {
19011910
p.end()
19021911
}
19031912

1904-
t.test('async', t => {
1913+
t.test('async', async t => {
19051914
t.test('.', t => runTest(t, '.', Pack))
1906-
return t.test('./', t => runTest(t, './', Pack))
1915+
t.test('./', t => runTest(t, './', Pack))
19071916
})
19081917

1909-
t.test('sync', t => {
1918+
t.test('sync', async t => {
19101919
t.test('.', t => runTest(t, '.', PackSync))
1911-
return t.test('./', t => runTest(t, './', PackSync))
1920+
t.test('./', t => runTest(t, './', PackSync))
19121921
})
19131922

19141923
t.end()

0 commit comments

Comments
 (0)