Skip to content

Commit

Permalink
unpack: only reuse file fs entries if nlink = 1
Browse files Browse the repository at this point in the history
This matches behavior in bsdtar to prevent overwriting a hardlink with
contents from a File archive entry.
  • Loading branch information
isaacs committed Apr 30, 2018
1 parent 58a8d43 commit b0c5843
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lib/unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const wc = require('./winchars.js')

const ONENTRY = Symbol('onEntry')
const CHECKFS = Symbol('checkFs')
const ISREUSABLE = Symbol('isReusable')
const MAKEFS = Symbol('makeFs')
const FILE = Symbol('file')
const DIRECTORY = Symbol('directory')
Expand Down Expand Up @@ -351,6 +352,17 @@ class Unpack extends Parser {
entry.resume()
}

// Check if we can reuse an existing filesystem entry safely and
// overwrite it, rather than unlinking and recreating
// Windows doesn't report a useful nlink, so we just never reuse entries
[ISREUSABLE] (entry, st) {
return entry.type === 'File' &&
!this.unlink &&
st.isFile() &&
st.nlink <= 1 &&
process.platform !== 'win32'
}

// check if a thing is there, and if so, try to clobber it
[CHECKFS] (entry) {
this[PEND]()
Expand All @@ -360,7 +372,7 @@ class Unpack extends Parser {
fs.lstat(entry.absolute, (er, st) => {
if (st && (this.keep || this.newer && st.mtime > entry.mtime))
this[SKIP](entry)
else if (er || (entry.type === 'File' && !this.unlink && st.isFile()))
else if (er || this[ISREUSABLE](entry, st))
this[MAKEFS](null, entry)
else if (st.isDirectory()) {
if (entry.type === 'Directory') {
Expand Down Expand Up @@ -422,7 +434,7 @@ class UnpackSync extends Unpack {
const st = fs.lstatSync(entry.absolute)
if (this.keep || this.newer && st.mtime > entry.mtime)
return this[SKIP](entry)
else if (entry.type === 'File' && !this.unlink && st.isFile())
else if (this[ISREUSABLE](entry, st))
return this[MAKEFS](null, entry)
else {
try {
Expand Down
63 changes: 63 additions & 0 deletions test/unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -2286,3 +2286,66 @@ t.test('onentry option is preserved', t => {

t.end()
})

t.test('do not reuse hardlinks, only nlink=1 files', t => {
const basedir = path.resolve(unpackdir, 'hardlink-reuse')
mkdirp.sync(basedir)
t.teardown(() => rimraf.sync(basedir))

const now = new Date('2018-04-30T18:30:39.025Z')

const data = makeTar([
{
path: 'overwriteme',
type: 'File',
size: 4,
mode: 0o644,
mtime: now
},
'foo\n',
{
path: 'link',
linkpath: 'overwriteme',
type: 'Link',
mode: 0o644,
mtime: now
},
{
path: 'link',
type: 'File',
size: 4,
mode: 0o644,
mtime: now
},
'bar\n',
'',
''
])

const checks = {
'link': 'bar\n',
'overwriteme': 'foo\n'
}

const check = t => {
for (let f in checks) {
t.equal(fs.readFileSync(basedir + '/' + f, 'utf8'), checks[f], f)
t.equal(fs.statSync(basedir + '/' + f).nlink, 1, f)
}
t.end()
}

t.test('async', t => {
const u = new Unpack({ cwd: basedir })
u.on('close', () => check(t))
u.end(data)
})

t.test('sync', t => {
const u = new UnpackSync({ cwd: basedir })
u.end(data)
check(t)
})

t.end()
})

0 comments on commit b0c5843

Please sign in to comment.