Skip to content

Commit bdf4f51

Browse files
committed
fix: properly prefix hard links
This moves all the prefix-handling logic into the WriteEntry classes, where it belongs. Fix: #284
1 parent 94b2a74 commit bdf4f51

File tree

4 files changed

+645
-22
lines changed

4 files changed

+645
-22
lines changed

lib/pack.js

+3-14
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,6 @@ const Pack = warner(class Pack extends MiniPass {
134134

135135
[ADDTARENTRY] (p) {
136136
const absolute = path.resolve(this.cwd, p.path)
137-
if (this.prefix)
138-
p.path = this.prefix + '/' + p.path.replace(/^\.(\/+|$)/, '')
139-
140137
// in this case, we don't have to wait for the stat
141138
if (!this.filter(p.path, p))
142139
p.resume()
@@ -153,9 +150,6 @@ const Pack = warner(class Pack extends MiniPass {
153150

154151
[ADDFSENTRY] (p) {
155152
const absolute = path.resolve(this.cwd, p)
156-
if (this.prefix)
157-
p = this.prefix + '/' + p.replace(/^\.(\/+|$)/, '')
158-
159153
this[QUEUE].push(new PackJob(p, absolute))
160154
this[PROCESS]()
161155
}
@@ -298,6 +292,7 @@ const Pack = warner(class Pack extends MiniPass {
298292
statCache: this.statCache,
299293
noMtime: this.noMtime,
300294
mtime: this.mtime,
295+
prefix: this.prefix,
301296
}
302297
}
303298

@@ -323,10 +318,7 @@ const Pack = warner(class Pack extends MiniPass {
323318

324319
if (job.readdir) {
325320
job.readdir.forEach(entry => {
326-
const p = this.prefix ?
327-
job.path.slice(this.prefix.length + 1) || './'
328-
: job.path
329-
321+
const p = job.path
330322
const base = p === './' ? '' : p.replace(/\/*$/, '/')
331323
this[ADDFSENTRY](base + entry)
332324
})
@@ -381,10 +373,7 @@ class PackSync extends Pack {
381373

382374
if (job.readdir) {
383375
job.readdir.forEach(entry => {
384-
const p = this.prefix ?
385-
job.path.slice(this.prefix.length + 1) || './'
386-
: job.path
387-
376+
const p = job.path
388377
const base = p === './' ? '' : p.replace(/\/*$/, '/')
389378
this[ADDFSENTRY](base + entry)
390379
})

lib/write-entry.js

+32-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ const Header = require('./header.js')
55
const fs = require('fs')
66
const path = require('path')
77

8+
const prefixPath = (path, prefix) => {
9+
if (!prefix)
10+
return path
11+
path = path.replace(/^\.([/\\]|$)/, '')
12+
return prefix + '/' + path
13+
}
14+
815
const maxReadSize = 16 * 1024 * 1024
916
const PROCESS = Symbol('process')
1017
const FILE = Symbol('file')
@@ -23,6 +30,7 @@ const CLOSE = Symbol('close')
2330
const MODE = Symbol('mode')
2431
const AWAITDRAIN = Symbol('awaitDrain')
2532
const ONDRAIN = Symbol('ondrain')
33+
const PREFIX = Symbol('prefix')
2634
const warner = require('./warn-mixin.js')
2735
const winchars = require('./winchars.js')
2836
const stripAbsolutePath = require('./strip-absolute-path.js')
@@ -50,6 +58,7 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
5058
this.noPax = !!opt.noPax
5159
this.noMtime = !!opt.noMtime
5260
this.mtime = opt.mtime || null
61+
this.prefix = opt.prefix || null
5362

5463
this.fd = null
5564
this.blockLen = null
@@ -128,13 +137,19 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
128137
return modeFix(mode, this.type === 'Directory', this.portable)
129138
}
130139

140+
[PREFIX] (path) {
141+
return prefixPath(path, this.prefix)
142+
}
143+
131144
[HEADER] () {
132145
if (this.type === 'Directory' && this.portable)
133146
this.noMtime = true
134147

135148
this.header = new Header({
136-
path: this.path,
137-
linkpath: this.linkpath,
149+
path: this[PREFIX](this.path),
150+
// only apply the prefix to hard links.
151+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
152+
: this.linkpath,
138153
// only the permissions and setuid/setgid/sticky bitflags
139154
// not the higher-order bits that specify file type
140155
mode: this[MODE](this.stat.mode),
@@ -155,8 +170,9 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
155170
ctime: this.portable ? null : this.header.ctime,
156171
gid: this.portable ? null : this.header.gid,
157172
mtime: this.noMtime ? null : this.mtime || this.header.mtime,
158-
path: this.path,
159-
linkpath: this.linkpath,
173+
path: this[PREFIX](this.path),
174+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
175+
: this.linkpath,
160176
size: this.header.size,
161177
uid: this.portable ? null : this.header.uid,
162178
uname: this.portable ? null : this.header.uname,
@@ -385,6 +401,8 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
385401
if (this.type === 'Directory' && this.portable)
386402
this.noMtime = true
387403

404+
this.prefix = opt.prefix || null
405+
388406
this.path = readEntry.path
389407
this.mode = this[MODE](readEntry.mode)
390408
this.uid = this.portable ? null : readEntry.uid
@@ -413,8 +431,9 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
413431
this.blockRemain = readEntry.startBlockSize
414432

415433
this.header = new Header({
416-
path: this.path,
417-
linkpath: this.linkpath,
434+
path: this[PREFIX](this.path),
435+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
436+
: this.linkpath,
418437
// only the permissions and setuid/setgid/sticky bitflags
419438
// not the higher-order bits that specify file type
420439
mode: this.mode,
@@ -441,8 +460,9 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
441460
ctime: this.portable ? null : this.ctime,
442461
gid: this.portable ? null : this.gid,
443462
mtime: this.noMtime ? null : this.mtime,
444-
path: this.path,
445-
linkpath: this.linkpath,
463+
path: this[PREFIX](this.path),
464+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
465+
: this.linkpath,
446466
size: this.size,
447467
uid: this.portable ? null : this.uid,
448468
uname: this.portable ? null : this.uname,
@@ -456,6 +476,10 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
456476
readEntry.pipe(this)
457477
}
458478

479+
[PREFIX] (path) {
480+
return prefixPath(path, this.prefix)
481+
}
482+
459483
[MODE] (mode) {
460484
return modeFix(mode, this.type === 'Directory', this.portable)
461485
}

test/pack.js

+94
Original file line numberDiff line numberDiff line change
@@ -1058,3 +1058,97 @@ t.test('prefix and subdirs', t => {
10581058
return t.test('./', t => runTest(t, './', Pack.Sync))
10591059
})
10601060
})
1061+
1062+
// https://github.com/npm/node-tar/issues/284
1063+
t.test('prefix and hard links', t => {
1064+
const dir = path.resolve(fixtures, 'pack-prefix-hardlinks')
1065+
t.teardown(_ => rimraf.sync(dir))
1066+
mkdirp.sync(dir + '/in/z/b/c')
1067+
fs.writeFileSync(dir + '/in/target', 'ddd')
1068+
fs.linkSync(dir + '/in/target', dir + '/in/z/b/c/d')
1069+
fs.linkSync(dir + '/in/target', dir + '/in/z/b/d')
1070+
fs.linkSync(dir + '/in/target', dir + '/in/z/d')
1071+
fs.linkSync(dir + '/in/target', dir + '/in/y')
1072+
1073+
const expect = [
1074+
'out/x/\0',
1075+
{
1076+
type: 'File',
1077+
size: 3,
1078+
path: 'out/x/target',
1079+
linkpath: '',
1080+
},
1081+
'ddd\0\0\0\0\0\0\0\0\0\0\0',
1082+
{
1083+
path: 'out/x/y',
1084+
type: 'Link',
1085+
linkpath: 'out/x/target',
1086+
},
1087+
'out/x/z/\0',
1088+
'out/x/z/b/\0',
1089+
{
1090+
path: 'out/x/z/d',
1091+
type: 'Link',
1092+
linkpath: 'out/x/target',
1093+
},
1094+
'out/x/z/b/c/\0',
1095+
{
1096+
path: 'out/x/z/b/d',
1097+
type: 'Link',
1098+
linkpath: 'out/x/target',
1099+
},
1100+
{
1101+
path: 'out/x/z/b/c/d',
1102+
type: 'Link',
1103+
linkpath: 'out/x/target',
1104+
},
1105+
'\0',
1106+
'\0',
1107+
]
1108+
1109+
const check = (out, t) => {
1110+
const data = Buffer.concat(out)
1111+
expect.forEach((e, i) => {
1112+
if (typeof e === 'string')
1113+
t.equal(data.slice(i * 512, i * 512 + e.length).toString(), e)
1114+
else
1115+
t.match(new Header(data.slice(i * 512, (i + 1) * 512)), e)
1116+
})
1117+
t.end()
1118+
}
1119+
1120+
const runTest = (t, path, Class) => {
1121+
const p = new Class({
1122+
cwd: dir + '/in',
1123+
prefix: 'out/x',
1124+
noDirRecurse: true,
1125+
})
1126+
const out = []
1127+
p.on('data', d => out.push(d))
1128+
p.on('end', () => check(out, t))
1129+
p.write(path)
1130+
if (path === '.')
1131+
path = './'
1132+
p.write(`${path}target`)
1133+
p.write(`${path}y`)
1134+
p.write(`${path}z`)
1135+
p.write(`${path}z/b`)
1136+
p.write(`${path}z/d`)
1137+
p.write(`${path}z/b/c`)
1138+
p.write(`${path}z/b/d`)
1139+
p.write(`${path}z/b/c/d`)
1140+
p.end()
1141+
}
1142+
1143+
t.test('async', t => {
1144+
t.test('.', t => runTest(t, '.', Pack))
1145+
return t.test('./', t => runTest(t, './', Pack))
1146+
})
1147+
1148+
t.test('sync', t => {
1149+
t.test('.', t => runTest(t, '.', Pack.Sync))
1150+
return t.test('./', t => runTest(t, './', Pack.Sync))
1151+
})
1152+
1153+
t.end()
1154+
})

0 commit comments

Comments
 (0)