Skip to content

Commit e29a665

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

File tree

4 files changed

+659
-25
lines changed

4 files changed

+659
-25
lines changed

‎lib/pack.js

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,6 @@ const Pack = warner(class Pack extends MiniPass {
133133

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

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

@@ -324,10 +319,7 @@ const Pack = warner(class Pack extends MiniPass {
324319

325320
if (job.readdir)
326321
job.readdir.forEach(entry => {
327-
const p = this.prefix ?
328-
job.path.slice(this.prefix.length + 1) || './'
329-
: job.path
330-
322+
const p = job.path
331323
const base = p === './' ? '' : p.replace(/\/*$/, '/')
332324
this[ADDFSENTRY](base + entry)
333325
})
@@ -380,10 +372,7 @@ class PackSync extends Pack {
380372

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

‎lib/write-entry.js

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ const ReadEntry = require('./read-entry.js')
77
const fs = require('fs')
88
const path = require('path')
99

10-
const types = require('./types.js')
10+
const prefixPath = (path, prefix) => {
11+
if (!prefix)
12+
return path
13+
path = path.replace(/^\.([/\\]|$)/, '')
14+
return prefix + '/' + path
15+
}
16+
1117
const maxReadSize = 16 * 1024 * 1024
1218
const PROCESS = Symbol('process')
1319
const FILE = Symbol('file')
@@ -26,6 +32,7 @@ const CLOSE = Symbol('close')
2632
const MODE = Symbol('mode')
2733
const AWAITDRAIN = Symbol('awaitDrain')
2834
const ONDRAIN = Symbol('ondrain')
35+
const PREFIX = Symbol('prefix')
2936
const warner = require('./warn-mixin.js')
3037
const winchars = require('./winchars.js')
3138
const stripAbsolutePath = require('./strip-absolute-path.js')
@@ -53,6 +60,7 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
5360
this.noPax = !!opt.noPax
5461
this.noMtime = !!opt.noMtime
5562
this.mtime = opt.mtime || null
63+
this.prefix = opt.prefix || null
5664

5765
this.fd = null
5866
this.blockLen = null
@@ -123,13 +131,19 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
123131
return modeFix(mode, this.type === 'Directory')
124132
}
125133

134+
[PREFIX] (path) {
135+
return prefixPath(path, this.prefix)
136+
}
137+
126138
[HEADER] () {
127139
if (this.type === 'Directory' && this.portable)
128140
this.noMtime = true
129141

130142
this.header = new Header({
131-
path: this.path,
132-
linkpath: this.linkpath,
143+
path: this[PREFIX](this.path),
144+
// only apply the prefix to hard links.
145+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
146+
: this.linkpath,
133147
// only the permissions and setuid/setgid/sticky bitflags
134148
// not the higher-order bits that specify file type
135149
mode: this[MODE](this.stat.mode),
@@ -150,8 +164,9 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
150164
ctime: this.portable ? null : this.header.ctime,
151165
gid: this.portable ? null : this.header.gid,
152166
mtime: this.noMtime ? null : this.mtime || this.header.mtime,
153-
path: this.path,
154-
linkpath: this.linkpath,
167+
path: this[PREFIX](this.path),
168+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
169+
: this.linkpath,
155170
size: this.header.size,
156171
uid: this.portable ? null : this.header.uid,
157172
uname: this.portable ? null : this.header.uname,
@@ -385,6 +400,8 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
385400
if (this.type === 'Directory' && this.portable)
386401
this.noMtime = true
387402

403+
this.prefix = opt.prefix || null
404+
388405
this.path = readEntry.path
389406
this.mode = this[MODE](readEntry.mode)
390407
this.uid = this.portable ? null : readEntry.uid
@@ -415,8 +432,9 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
415432
this.blockRemain = readEntry.startBlockSize
416433

417434
this.header = new Header({
418-
path: this.path,
419-
linkpath: this.linkpath,
435+
path: this[PREFIX](this.path),
436+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
437+
: this.linkpath,
420438
// only the permissions and setuid/setgid/sticky bitflags
421439
// not the higher-order bits that specify file type
422440
mode: this.mode,
@@ -436,8 +454,9 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
436454
ctime: this.portable ? null : this.ctime,
437455
gid: this.portable ? null : this.gid,
438456
mtime: this.noMtime ? null : this.mtime,
439-
path: this.path,
440-
linkpath: this.linkpath,
457+
path: this[PREFIX](this.path),
458+
linkpath: this.type === 'Link' ? this[PREFIX](this.linkpath)
459+
: this.linkpath,
441460
size: this.size,
442461
uid: this.portable ? null : this.uid,
443462
uname: this.portable ? null : this.uname,
@@ -450,6 +469,10 @@ const WriteEntryTar = warner(class WriteEntryTar extends MiniPass {
450469
readEntry.pipe(this)
451470
}
452471

472+
[PREFIX] (path) {
473+
return prefixPath(path, this.prefix)
474+
}
475+
453476
[MODE] (mode) {
454477
return modeFix(mode, this.type === 'Directory')
455478
}

‎test/pack.js

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

0 commit comments

Comments
 (0)