Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the noChmod option #270

Merged
merged 6 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test/fixtures/files/** text eol=lf
19 changes: 14 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ jobs:
shell: bash
- os: macos-latest
shell: bash
- os: windows-latest
shell: bash
- os: windows-latest
shell: powershell

# TODO: make tests pass on windows. it works, but the tests have
# quite a lot of unixisms wrt modes and paths. mostly trivial
# stuff to fix, but a lot of it.
#
# - os: windows-latest
# shell: bash
# - os: windows-latest
# shell: powershell

fail-fast: false

runs-on: ${{ matrix.platform.os }}
Expand All @@ -24,6 +30,10 @@ jobs:
shell: ${{ matrix.platform.shell }}

steps:
# there are files here that make windows unhappy by default
- name: Support longpaths
run: git config --global core.longpaths true

- name: Checkout Repository
uses: actions/checkout@v1.1.0

Expand All @@ -35,6 +45,5 @@ jobs:
- name: Install dependencies
run: npm install

# Run for all environments
- name: Run Tap Tests
run: npm test -- -c -t0
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ The following options are supported:
- `mtime` Set to a `Date` object to force a specific `mtime` for
everything added to the archive. Overridden by `noMtime`.


The following options are mostly internal, but can be modified in some
advanced use cases, such as re-using caches between runs.

Expand Down Expand Up @@ -396,6 +395,13 @@ The following options are supported:
the `filter` option described above.)
- `onentry` A function that gets called with `(entry)` for each entry
that passes the filter.
- `onwarn` A function that will get called with `(code, message, data)` for
any warnings encountered. (See "Warnings and Errors")
- `noChmod` Set to true to omit calling `fs.chmod()` to ensure that the
extracted file matches the entry mode. This also suppresses the call to
`process.umask()` to determine the default umask value, since tar will
extract with whatever mode is provided, and let the process `umask` apply
normally.

The following options are mostly internal, but can be modified in some
advanced use cases, such as re-using caches between runs.
Expand Down Expand Up @@ -451,6 +457,8 @@ The following options are supported:
the call to `onentry`. Set `noResume: true` to suppress this
behavior. Note that by opting into this, the stream will never
complete until the entry data is consumed.
- `onwarn` A function that will get called with `(code, message, data)` for
any warnings encountered. (See "Warnings and Errors")

### tar.u(options, fileList, callback) [alias: tar.update]

Expand Down Expand Up @@ -708,6 +716,11 @@ Most unpack errors will cause a `warn` event to be emitted. If the
that passes the filter.
- `onwarn` A function that will get called with `(code, message, data)` for
any warnings encountered. (See "Warnings and Errors")
- `noChmod` Set to true to omit calling `fs.chmod()` to ensure that the
extracted file matches the entry mode. This also suppresses the call to
`process.umask()` to determine the default umask value, since tar will
extract with whatever mode is provided, and let the process `umask` apply
normally.

### class tar.Unpack.Sync

Expand Down
10 changes: 7 additions & 3 deletions lib/unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,14 @@ class Unpack extends Parser {

this.cwd = path.resolve(opt.cwd || process.cwd())
this.strip = +opt.strip || 0
this.processUmask = process.umask()
// if we're not chmodding, then we don't need the process umask
this.processUmask = opt.noChmod ? 0 : process.umask()
this.umask = typeof opt.umask === 'number' ? opt.umask : this.processUmask

// default mode for dirs created as parents
this.dmode = opt.dmode || (0o0777 & (~this.umask))
this.fmode = opt.fmode || (0o0666 & (~this.umask))

this.on('entry', entry => this[ONENTRY](entry))
}

Expand Down Expand Up @@ -299,6 +302,7 @@ class Unpack extends Parser {
cache: this.dirCache,
cwd: this.cwd,
mode: mode,
noChmod: this.noChmod,
}, cb)
}

Expand Down Expand Up @@ -475,7 +479,7 @@ class Unpack extends Parser {

else if (st.isDirectory()) {
if (entry.type === 'Directory') {
if (!entry.mode || (st.mode & 0o7777) === entry.mode)
if (!this.noChmod && (!entry.mode || (st.mode & 0o7777) === entry.mode))
this[MAKEFS](null, entry, done)
else {
fs.chmod(entry.absolute, entry.mode,
Expand Down Expand Up @@ -538,7 +542,7 @@ class UnpackSync extends Unpack {
try {
if (st.isDirectory()) {
if (entry.type === 'Directory') {
if (entry.mode && (st.mode & 0o7777) !== entry.mode)
if (!this.noChmod && entry.mode && (st.mode & 0o7777) !== entry.mode)
fs.chmodSync(entry.absolute, entry.mode)
} else
fs.rmdirSync(entry.absolute)
Expand Down
51 changes: 48 additions & 3 deletions test/unpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,48 @@ t.test('use explicit chmod when required by umask', t => {
})
})

t.test('dont use explicit chmod if noChmod flag set', t => {
process.umask(0o022)
const { umask } = process
t.teardown(() => process.umask = umask)
process.umask = () => {
throw new Error('should not call process.umask()')
}

const basedir = path.resolve(unpackdir, 'umask-no-chmod')

const data = makeTar([
{
path: 'x/y/z',
mode: 0o775,
type: 'Directory',
},
'',
'',
])

const check = t => {
const st = fs.statSync(basedir + '/x/y/z')
t.equal(st.mode & 0o777, 0o755)
rimraf.sync(basedir)
t.end()
}

t.test('async', t => {
mkdirp.sync(basedir)
const unpack = new Unpack({ cwd: basedir, noChmod: true })
unpack.on('close', _ => check(t))
unpack.end(data)
})

return t.test('sync', t => {
mkdirp.sync(basedir)
const unpack = new Unpack.Sync({ cwd: basedir, noChmod: true})
unpack.end(data)
check(t)
})
})

t.test('chown implicit dirs and also the entries', t => {
const basedir = path.resolve(unpackdir, 'chownr')

Expand Down Expand Up @@ -2508,8 +2550,8 @@ t.test('do not reuse hardlinks, only nlink=1 files', t => {
t.end()
})

t.test('trying to unpack a javascript file should fail', t => {
const data = fs.readFileSync(__filename)
t.test('trying to unpack a non-zlib gzip file should fail', t => {
const data = Buffer.from('hello this is not gzip data')
const dataGzip = Buffer.concat([Buffer.from([0x1f, 0x8b]), data])
const basedir = path.resolve(unpackdir, 'bad-archive')
t.test('abort if gzip has an error', t => {
Expand All @@ -2529,7 +2571,10 @@ t.test('trying to unpack a javascript file should fail', t => {
new Unpack(opts)
.once('error', er => t.match(er, expect, 'async emits'))
.end(dataGzip)
t.throws(() => new UnpackSync(opts).end(dataGzip), expect, 'sync throws')
const skip = !/^v([0-9]|1[0-3])\./.test(process.version) ? false
: 'node prior to v14 did not raise sync zlib errors properly'
t.throws(() => new UnpackSync(opts).end(dataGzip),
expect, 'sync throws', {skip})
})

t.test('bad archive if no gzip', t => {
Expand Down