From 465764e91b99e3adb8d51b937378d0a1c15eb13f Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 9 Sep 2024 17:32:08 +0200 Subject: [PATCH 01/16] test: Add tests for permissions --- src/__tests__/util.ts | 12 +++++ src/__tests__/volume/ReadStream.test.ts | 24 +++++++++ src/__tests__/volume/WriteStream.test.ts | 36 +++++++++++++ src/__tests__/volume/appendFile.test.ts | 38 +++++++++++++- src/__tests__/volume/appendFileSync.test.ts | 22 ++++++++ src/__tests__/volume/copyFile.test.ts | 55 +++++++++++++++++++- src/__tests__/volume/copyFileSync.test.ts | 35 +++++++++++++ src/__tests__/volume/exists.test.ts | 15 ++++++ src/__tests__/volume/existsSync.test.ts | 8 +++ src/__tests__/volume/mkdirSync.test.ts | 15 ++++++ src/__tests__/volume/openSync.test.ts | 57 ++++++++++++++++++++- src/__tests__/volume/readFile.test.ts | 12 +++++ src/__tests__/volume/readSync.test.ts | 5 ++ src/__tests__/volume/readdirSync.test.ts | 17 ++++-- src/__tests__/volume/realpathSync.test.ts | 7 +++ src/__tests__/volume/rename.test.ts | 47 ++++++++++++++++- src/__tests__/volume/renameSync.test.ts | 29 +++++++++++ src/__tests__/volume/rmPromise.test.ts | 13 +++++ src/__tests__/volume/rmSync.test.ts | 15 ++++++ src/__tests__/volume/statSync.test.ts | 29 +++++++++++ src/__tests__/volume/write.test.ts | 5 ++ src/__tests__/volume/writeFileSync.test.ts | 30 +++++++++++ src/__tests__/volume/writeSync.test.ts | 5 ++ 23 files changed, 523 insertions(+), 8 deletions(-) diff --git a/src/__tests__/util.ts b/src/__tests__/util.ts index 1107578fb..df6c47446 100644 --- a/src/__tests__/util.ts +++ b/src/__tests__/util.ts @@ -1,6 +1,18 @@ import { createFsFromVolume, Volume } from '..'; import { Link, Node } from '../node'; +// Turn the done callback into an incremental one that will only fire after being called +// `times` times, failing with the first reported error if such exists. +// Useful for testing callback-style functions with several different fixtures without +// having to clutter the test suite with a multitude of individual tests (like it.each would). +export const multitest = (_done: (err?: Error) => void, times: number) => { + let err; + return function done(_err?: Error) { + err ??= _err; + if (!--times) _done(_err); + } +} + export const create = (json: { [s: string]: string } = { '/foo': 'bar' }) => { const vol = Volume.fromJSON(json); return vol; diff --git a/src/__tests__/volume/ReadStream.test.ts b/src/__tests__/volume/ReadStream.test.ts index 3ca588aef..9fa2a9ff7 100644 --- a/src/__tests__/volume/ReadStream.test.ts +++ b/src/__tests__/volume/ReadStream.test.ts @@ -18,4 +18,28 @@ describe('ReadStream', () => { done(); }); }); + + it('should emit EACCES error when file has insufficient permissions', done => { + const fs = createFs({ '/test': 'test' }); + fs.chmodSync('/test', 0o333); // wx + new fs.ReadStream('/test').on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }).on('open', () => { + done(new Error('Expected ReadStream to emit EACCES but it didn\'t')); + });; + }); + + it('should emit EACCES error when containing directory has insufficient permissions', done => { + const fs = createFs({ '/foo/test': 'test' }); + fs.chmodSync('/foo', 0o666); // rw + new fs.ReadStream('/foo/test').on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }).on('open', () => { + done(new Error('Expected ReadStream to emit EACCES but it didn\'t')); + }); + }); }); diff --git a/src/__tests__/volume/WriteStream.test.ts b/src/__tests__/volume/WriteStream.test.ts index 9e85a0ba4..b77bc55f1 100644 --- a/src/__tests__/volume/WriteStream.test.ts +++ b/src/__tests__/volume/WriteStream.test.ts @@ -19,4 +19,40 @@ describe('WriteStream', () => { done(); }); }); + + it('should emit EACCES error when file has insufficient permissions', done => { + const fs = createFs({ '/test': 'test' }); + fs.chmodSync('/test', 0o555); // rx + new fs.WriteStream('/test').on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }).on('open', () => { + done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); + });; + }); + + it('should emit EACCES error for an existing file when containing directory has insufficient permissions', done => { + const fs = createFs({ '/foo/test': 'test' }); + fs.chmodSync('/foo', 0o666); // rw + new fs.WriteStream('/foo/test').on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }).on('open', () => { + done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); + }); + }); + + it('should emit EACCES error for an non-existant file when containing directory has insufficient permissions', done => { + const fs = createFs({}); + fs.mkdirSync('/foo', { mode: 0o555 }); // rx + new fs.WriteStream('/foo/test').on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }).on('open', () => { + done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); + }); + }); }); diff --git a/src/__tests__/volume/appendFile.test.ts b/src/__tests__/volume/appendFile.test.ts index bd8ee4c05..168b1510c 100644 --- a/src/__tests__/volume/appendFile.test.ts +++ b/src/__tests__/volume/appendFile.test.ts @@ -1,4 +1,4 @@ -import { create } from '../util'; +import { create, multitest } from '../util'; describe('appendFile(file, data[, options], callback)', () => { it('Simple write to non-existing file', done => { @@ -15,4 +15,40 @@ describe('appendFile(file, data[, options], callback)', () => { done(); }); }); + + it('Appending gives EACCES without sufficient permissions', done => { + const vol = create({ '/foo': 'foo' }); + vol.chmodSync('/foo', 0o555); // rx across the board + vol.appendFile('/foo', 'bar', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch(x) { + done(x); + } + }); + }); + + it('Appending gives EACCES if file does not exist and containing directory has insufficient permissions', _done => { + const perms = [ + 0o555, // rx across the board + 0o666 // rw across the board + ]; + const done = multitest(_done, perms.length); + + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: perm }); + vol.appendFile('/foo/test', 'bar', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch(x) { + done(x); + } + }); + }) + }); }); diff --git a/src/__tests__/volume/appendFileSync.test.ts b/src/__tests__/volume/appendFileSync.test.ts index 8fbdaad8b..515f829f5 100644 --- a/src/__tests__/volume/appendFileSync.test.ts +++ b/src/__tests__/volume/appendFileSync.test.ts @@ -11,4 +11,26 @@ describe('appendFileSync(file, data, options)', () => { vol.appendFileSync('/a', 'c'); expect(vol.readFileSync('/a', 'utf8')).toEqual('bc'); }); + it('Appending throws EACCES without sufficient permissions', () => { + const vol = create({ '/foo': 'foo' }); + vol.chmodSync('/foo', 0o555); // rx across the board + expect(() => { + vol.appendFileSync('/foo', 'bar'); + }).toThrowError(/EACCES/); + }); + it('Appending throws EACCES if file does not exist and containing directory has insufficient permissions', () => { + const perms = [ + 0o555, // rx across the board + // 0o666, // rw across the board + // 0o111, // x + // 0o222 // w + ]; + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo', perm); + expect(() => { + vol.appendFileSync('/foo/test', 'bar'); + }).toThrowError(/EACCES/); + }); + }); }); diff --git a/src/__tests__/volume/copyFile.test.ts b/src/__tests__/volume/copyFile.test.ts index 2210f90fa..3f3cb7062 100644 --- a/src/__tests__/volume/copyFile.test.ts +++ b/src/__tests__/volume/copyFile.test.ts @@ -1,4 +1,4 @@ -import { create } from '../util'; +import { create, multitest } from '../util'; import { constants } from '../../constants'; describe('copyFile(src, dest[, flags], callback)', () => { @@ -44,4 +44,57 @@ describe('copyFile(src, dest[, flags], callback)', () => { done(); }); }); + + describe('permissions', () => { + it('copying gives EACCES with insufficient permissions on the source file', done => { + const vol = create({ '/foo': 'foo' }); + vol.chmodSync('/foo', 0o333); // wx across the board + vol.copyFile('/foo', '/bar', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + } finally { + done(); + } + }); + }); + + it('copying gives EACCES with insufficient permissions on the source directory', done => { + const vol = create({ '/foo/bar': 'foo' }); + vol.chmodSync('/foo', 0o666); // rw across the board + vol.copyFile('/foo/bar', '/bar', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + } finally { + done(); + } + }); + }); + + it('copying yields EACCES with insufficient permissions on the destination directory', _done => { + const perms = [ + 0o555, // rx + 0o666, // rw + 0o111, // x + 0o222 // w + ]; + const done = multitest(_done, perms.length); + + perms.forEach(perm => { + const vol = create({ '/foo': 'foo' }); + vol.mkdirSync('/bar'); + vol.chmodSync('/bar', perm); + vol.copyFile('/foo', '/bar/foo', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch(err) { + done(err); + } + }); + }); + }); + }); }); diff --git a/src/__tests__/volume/copyFileSync.test.ts b/src/__tests__/volume/copyFileSync.test.ts index cba2aecdc..0ab968ef3 100644 --- a/src/__tests__/volume/copyFileSync.test.ts +++ b/src/__tests__/volume/copyFileSync.test.ts @@ -91,4 +91,39 @@ describe('copyFileSync(src, dest[, flags])', () => { expect(vol.readFileSync('/foo', 'utf8')).toBe('hello world'); }); }); + + describe('permissions', () => { + it('copying throws EACCES with insufficient permissions on the source file', () => { + const vol = create({ '/foo': 'foo' }); + vol.chmodSync('/foo', 0o333); // wx across the board + expect(() => { + vol.copyFileSync('/foo', '/bar'); + }).toThrowError(/EACCES/); + }); + + it('copying throws EACCES with insufficient permissions on the source directory', () => { + const vol = create({ '/foo/bar': 'foo' }); + vol.chmodSync('/foo', 0o666); // rw across the board + expect(() => { + vol.copyFileSync('/foo/bar', '/bar'); + }).toThrowError(/EACCES/); + }); + + it('copying throws EACCES with insufficient permissions on the destination directory', () => { + const perms = [ + 0o555, // rx + 0o666, // rw + 0o111, // x + 0o222 // w + ]; + perms.forEach(perm => { + const vol = create({ '/foo': 'foo' }); + vol.mkdirSync('/bar'); + vol.chmodSync('/bar', perm); + expect(() => { + vol.copyFileSync('/foo', '/bar/foo'); + }).toThrowError(/EACCES/); + }); + }); + }); }); diff --git a/src/__tests__/volume/exists.test.ts b/src/__tests__/volume/exists.test.ts index b8004b13f..58a8a820e 100644 --- a/src/__tests__/volume/exists.test.ts +++ b/src/__tests__/volume/exists.test.ts @@ -31,4 +31,19 @@ describe('exists(path, callback)', () => { expect(err.message !== 'not_this').toEqual(true); } }); + it('gives false if permissions on containing directory are insufficient', done => { + // Experimentally determined: fs.exists treats missing permissions as "file does not exist", + // presumably because due to the non-standard callback signature there is no way to signal + // that permissions were insufficient + const vol = create({ '/foo/bar': 'test' }); + vol.chmodSync('/foo', 0o666); // rw across the board + vol.exists('/foo/bar', exists => { + try { + expect(exists).toEqual(false); + done(); + } catch(err) { + done(err); + } + }); + }); }); diff --git a/src/__tests__/volume/existsSync.test.ts b/src/__tests__/volume/existsSync.test.ts index 3d8fa94ad..cc65da4bc 100644 --- a/src/__tests__/volume/existsSync.test.ts +++ b/src/__tests__/volume/existsSync.test.ts @@ -13,4 +13,12 @@ describe('existsSync(path)', () => { it('invalid path type should not throw', () => { expect(vol.existsSync(123 as any)).toEqual(false); }); + it('returns false if permissions are insufficient on containing directory', () => { + // Experimentally determined: fs.existsSync treats missing permissions as "file does not exist", + // even though it could throw EACCES instead. + // This is presumably to achieve unity of behavior with fs.exists. + const vol = create({ '/foo/bar': 'test' }); + vol.chmodSync('/foo', 0o666); // rw across the board + expect(vol.existsSync('/foo/bar')).toEqual(false); + }); }); diff --git a/src/__tests__/volume/mkdirSync.test.ts b/src/__tests__/volume/mkdirSync.test.ts index 8230a0572..6729f96f3 100644 --- a/src/__tests__/volume/mkdirSync.test.ts +++ b/src/__tests__/volume/mkdirSync.test.ts @@ -63,4 +63,19 @@ describe('mkdirSync', () => { expect(vol.statSync('/__proto__').isDirectory()).toBe(true); }); + + it('throws EACCES with insufficient permissions on containing directory', () => { + const perms = [ + 0o666, // rw across the board + 0o555 // rx across the bord + ] + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chmodSync('/foo', perm); + expect(() => { + vol.mkdirSync(`/foo/bar`); + }).toThrowError(/EACCES/) + }); + }); }); diff --git a/src/__tests__/volume/openSync.test.ts b/src/__tests__/volume/openSync.test.ts index 94f8675c9..48b1c7eeb 100644 --- a/src/__tests__/volume/openSync.test.ts +++ b/src/__tests__/volume/openSync.test.ts @@ -1,8 +1,63 @@ -import { fs } from '../..'; +import { createFs } from '../util'; describe('openSync(path, mode[, flags])', () => { it('should return a file descriptor', () => { + const fs = createFs(); const fd = fs.openSync('/foo', 'w'); expect(typeof fd).toEqual('number'); }); + + it('throws ENOTDIR when trying to create a non-existent file inside another file', () => { + const fs = createFs(); + + expect(() => { + fs.openSync('/foo/baz', 'a'); + }).toThrow(/ENOTDIR/); + }); + + describe('permissions', () => { + it('opening for writing throws EACCES without sufficient permissions', () => { + const flags = [ 'a', 'w', 'r+' ]; // append, write, read+write + flags.forEach(intent => { + const fs = createFs(); + fs.chmodSync('/foo', 0o555); // rx across the board + expect(() => { + fs.openSync('/foo', intent); + }).toThrowError(/EACCES/) + }); + }); + + it('opening for reading throws EACCES without sufficient permissions', () => { + const flags = [ 'a+', 'r', 'w+' ]; // append+read, read, write+read + flags.forEach(intent => { + const fs = createFs(); + fs.chmodSync('/foo', 0o333); // wx across the board + expect(() => { + fs.openSync('/foo', intent); + }).toThrowError(/EACCES/); + }); + }); + + it('opening for anything throws EACCES without sufficient permissions on the containing directory of an existing file', () => { + const flags = [ 'a+', 'r', 'w' ]; // append+read, read, write + flags.forEach(intent => { + const fs = createFs({ '/foo/bar': 'test' }); + fs.chmodSync('/foo', 0o666); // wr across the board + expect(() => { + fs.openSync('/foo/bar', intent); + }).toThrowError(/EACCES/); + }); + }); + + it('opening for anything throws EACCES without sufficient permissions on the containing directory of an non-existent file', () => { + const flags = [ 'a+', 'r', 'w' ]; // append+read, read, write + flags.forEach(intent => { + const fs = createFs({}); + fs.mkdirSync('/foo', { mode: 0o666 }); // wr + expect(() => { + fs.openSync('/foo/bar', intent); + }).toThrowError(/EACCES/); + }); + }); + }); }); diff --git a/src/__tests__/volume/readFile.test.ts b/src/__tests__/volume/readFile.test.ts index 45d4a3902..6c63f5c2d 100644 --- a/src/__tests__/volume/readFile.test.ts +++ b/src/__tests__/volume/readFile.test.ts @@ -14,4 +14,16 @@ describe('.readFile()', () => { expect(err).toBeInstanceOf(Error); expect((err).code).toBe('ENOENT'); }); + + it('throws EACCES if file has insufficient permissions', async () => { + const { fs } = memfs({ '/foo': 'test' }); + fs.chmodSync('/foo', 0o333); // wx + return expect(fs.promises.readFile('/foo')).rejects.toThrow(/EACCES/); + }); + + it('throws EACCES if containing directory has insufficient permissions', async () => { + const { fs } = memfs({ '/foo/bar': 'test' }); + fs.chmodSync('/foo', 0o666); // rw + return expect(fs.promises.readFile('/foo/bar')).rejects.toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/readSync.test.ts b/src/__tests__/volume/readSync.test.ts index 2cacc5b4b..7c3133689 100644 --- a/src/__tests__/volume/readSync.test.ts +++ b/src/__tests__/volume/readSync.test.ts @@ -39,4 +39,9 @@ describe('.readSync(fd, buffer, offset, length, position)', () => { expect(buf.equals(Buffer.from('675'))).toBe(true); }); xit('Negative tests', () => {}); + + /* + * No need for permissions tests, because readSync requires a file descriptor, which can only be + * obtained from open or openSync. + */ }); diff --git a/src/__tests__/volume/readdirSync.test.ts b/src/__tests__/volume/readdirSync.test.ts index adf03fd29..1e38079e6 100644 --- a/src/__tests__/volume/readdirSync.test.ts +++ b/src/__tests__/volume/readdirSync.test.ts @@ -36,10 +36,9 @@ describe('readdirSync()', () => { '/a/aa': 'aa', '/b/b': 'b', }); + vol.symlinkSync('/a', '/lnk'); - vol.symlinkSync('/a', '/b/b/b'); - - const dirs = vol.readdirSync('/b/b/b'); + const dirs = vol.readdirSync('/lnk'); (dirs as any).sort(); @@ -117,4 +116,16 @@ describe('readdirSync()', () => { { mode: 33206, name: 'cf2', path: '/z/c/c' }, ]); }); + + it('throws EACCES when trying to readdirSync a directory with insufficient permissions', () => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: 0o333 }); // wx across the board + expect(() => { + vol.readdirSync('/foo'); + }).toThrowError(/EACCES/); + // Check that it also throws with one of the subdirs of a recursive scan + expect(() => { + vol.readdirSync('/', { recursive: true }); + }).toThrowError(/EACCES/); + }); }); diff --git a/src/__tests__/volume/realpathSync.test.ts b/src/__tests__/volume/realpathSync.test.ts index 95465c15f..661795109 100644 --- a/src/__tests__/volume/realpathSync.test.ts +++ b/src/__tests__/volume/realpathSync.test.ts @@ -15,4 +15,11 @@ describe('.realpathSync(...)', () => { const vol = create({ './a': 'a' }); expect(vol.realpathSync('/')).toBe('/'); }); + it('throws EACCES when the containing directory does not have sufficient permissions', () => { + const vol = create({ '/foo/bar': 'bar' }); + vol.chmodSync('/foo', 0o666); // rw + expect(() => { + vol.realpathSync('/foo/bar'); + }).toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/rename.test.ts b/src/__tests__/volume/rename.test.ts index 305927c4e..f7c3d8a87 100644 --- a/src/__tests__/volume/rename.test.ts +++ b/src/__tests__/volume/rename.test.ts @@ -1,4 +1,4 @@ -import { create } from '../util'; +import { create, multitest } from '../util'; describe('renameSync(fromPath, toPath)', () => { it('Renames a simple case', done => { @@ -8,4 +8,47 @@ describe('renameSync(fromPath, toPath)', () => { done(); }); }); -}); + + it('gives EACCES when source directory has insufficient permissions', _done => { + const perms = [ + 0o666, // rw + 0o555 // rx - insufficient because the file will be removed from this directory during renaming + ]; + const done = multitest(_done, perms.length); + perms.forEach(perm => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest'); + vol.chmodSync('/src', perm); + vol.rename('/src/test', '/dest/fail', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch(x) { + done(x); + } + }); + }); + }); + + it('gives EACCES when destination directory has insufficient permissions', _done => { + const perms = [ + 0o666, // rw + 0o555 // rx + ]; + const done = multitest(_done, perms.length); + perms.forEach(perm => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest', { mode: perm }); + vol.rename('/src/test', '/dest/fail', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch(x) { + done(x); + } + }); + }); + }); +}); \ No newline at end of file diff --git a/src/__tests__/volume/renameSync.test.ts b/src/__tests__/volume/renameSync.test.ts index 2354a03ea..5346cc2d3 100644 --- a/src/__tests__/volume/renameSync.test.ts +++ b/src/__tests__/volume/renameSync.test.ts @@ -71,4 +71,33 @@ describe('renameSync(fromPath, toPath)', () => { (vol as any).renameSync('/foo', 123); }).toThrowErrorMatchingSnapshot(); }); + + it('throws EACCES when source directory has insufficient permissions', () => { + const perms = [ + 0o666, // rw + 0o555 // rx - insufficient because the file will be removed from this directory during renaming + ]; + perms.forEach(perm => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest'); + vol.chmodSync('/src', perm); + expect(() => { + vol.renameSync('/src/test', '/dest/fail'); + }).toThrowError(/EACCES/); + }); + }); + + it('throws EACCES when destination directory has insufficient permissions', () => { + const perms = [ + 0o666, // rw + 0o555 // rx + ]; + perms.forEach(perm => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest', { mode: perm }); + expect(() => { + vol.renameSync('/src/test', '/dest/fail'); + }).toThrowError(/EACCES/); + }); + }); }); diff --git a/src/__tests__/volume/rmPromise.test.ts b/src/__tests__/volume/rmPromise.test.ts index d9ba5c842..aa8065433 100644 --- a/src/__tests__/volume/rmPromise.test.ts +++ b/src/__tests__/volume/rmPromise.test.ts @@ -136,4 +136,17 @@ describe('rmSync', () => { expect(vol.toJSON()).toEqual({}); }); }); + + it('throws EACCES when containing directory has insufficient permissions', async () => { + const perms = [ + 0o666, // rw + 0o555, // rx + 0o111 // x + ]; + return Promise.all(perms.map(perm => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', perm); + return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); + })); + }); }); diff --git a/src/__tests__/volume/rmSync.test.ts b/src/__tests__/volume/rmSync.test.ts index 49b1c811f..57edab3ef 100644 --- a/src/__tests__/volume/rmSync.test.ts +++ b/src/__tests__/volume/rmSync.test.ts @@ -114,4 +114,19 @@ describe('rmSync', () => { expect(vol.toJSON()).toEqual({}); }); }); + + it('throws EACCES when containing directory has insufficient permissions', () => { + const perms = [ + 0o666, // rw + 0o555, // rx + 0o111 // x + ]; + perms.forEach(perm => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', perm); + expect(() => { + vol.rmSync('/foo/test'); + }).toThrowError(/EACCES/); + }); + }); }); diff --git a/src/__tests__/volume/statSync.test.ts b/src/__tests__/volume/statSync.test.ts index 50e4d9568..928182619 100644 --- a/src/__tests__/volume/statSync.test.ts +++ b/src/__tests__/volume/statSync.test.ts @@ -11,4 +11,33 @@ describe('.statSync(...)', () => { const stats = vol.statSync('/a/b/index.js'); expect(stats.size).toBe(11); }); + + it('returns undefined for non-existent targets with the throwIfNoEntry option set to false', () => { + const vol = create({}); + + const stats = vol.statSync('/non-existent', { throwIfNoEntry: false }); + expect(stats).toBeUndefined(); + }); + + it('throws EACCES when for a non-existent file when containing directory does not have sufficient permissions even if throwIfNoEntry option is false', () => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: 0o666 }); // rw + expect(() => { + vol.statSync('/foo/non-existent', { throwIfNoEntry: false }); + }).toThrowError(/EACCES/); + }); + + it('throws EACCES when containing directory does not have sufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o666); // rw + + expect(() => { + vol.statSync('/foo/test'); + }).toThrowError(/EACCES/); + + // Make sure permissions win out against throwIfNoEntry option: + expect(() => { + vol.statSync('/foo/test', { throwIfNoEntry: false }); + }).toThrowError(/EACCES/); + }); }); diff --git a/src/__tests__/volume/write.test.ts b/src/__tests__/volume/write.test.ts index 86a7434b7..fc31a6f1c 100644 --- a/src/__tests__/volume/write.test.ts +++ b/src/__tests__/volume/write.test.ts @@ -17,4 +17,9 @@ describe('write(fs, str, position, encoding, callback)', () => { done(); }); }); + + /* + * No need for permissions tests, because write requires a file descriptor, which can only be + * obtained from open or openSync. + */ }); diff --git a/src/__tests__/volume/writeFileSync.test.ts b/src/__tests__/volume/writeFileSync.test.ts index fc089668d..0341d4af6 100644 --- a/src/__tests__/volume/writeFileSync.test.ts +++ b/src/__tests__/volume/writeFileSync.test.ts @@ -44,4 +44,34 @@ describe('writeFileSync(path, data[, options])', () => { expect(err.code).toBe('ENOENT'); } }); + + it('Write throws EACCES without sufficient permissions on containing directory', () => { + const perms = [ + 0o666, // rw + 0o555 // rx, only when target file does not exist yet + ] + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chmodSync('/foo', perm); + expect(() => { + vol.writeFileSync('/foo/test', 'test'); + }).toThrowError(/EACCES/); + }); + + // If the target file exists, it should not care about the write permission on containing dir + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o555); // rx, should be enough + expect(() => { + vol.writeFileSync('/foo/test', 'test'); + }).not.toThrowError(); + }); + + it('Write throws EACCES if file exists but has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo/test', 0o555); // rx + expect(() => { + vol.writeFileSync('/foo/test', 'test'); + }).toThrowError(/EACCES/); + }); }); diff --git a/src/__tests__/volume/writeSync.test.ts b/src/__tests__/volume/writeSync.test.ts index 371597edf..99d5defc0 100644 --- a/src/__tests__/volume/writeSync.test.ts +++ b/src/__tests__/volume/writeSync.test.ts @@ -25,4 +25,9 @@ describe('.writeSync(fd, buffer, offset, length, position)', () => { fs.writeSync(fd, 'x', 1); expect(fs.readFileSync('/foo', 'utf8')).toBe('1x3'); }); + + /* + * No need for permissions tests, because write requires a file descriptor, which can only be + * obtained from open or openSync. + */ }); From d25e21a9cbe233df171da77dd10d5c4bd52cbdf6 Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 9 Sep 2024 17:32:51 +0200 Subject: [PATCH 02/16] feat: Add permissions implementations to Volume --- src/node.ts | 38 +++-- src/volume.ts | 434 +++++++++++++++++++++++++++++++++++--------------- src/walker.ts | 81 ++++++++++ 3 files changed, 413 insertions(+), 140 deletions(-) create mode 100644 src/walker.ts diff --git a/src/node.ts b/src/node.ts index 9df2ead26..c5c74119a 100644 --- a/src/node.ts +++ b/src/node.ts @@ -279,6 +279,26 @@ export class Node extends EventEmitter { return false; } + canExecute(uid: number = getuid(), gid: number = getgid()): boolean { + if (this.perm & S.IXOTH) { + return true; + } + + if (gid === this.gid) { + if (this.perm & S.IXGRP) { + return true; + } + } + + if (uid === this.uid) { + if (this.perm & S.IXUSR) { + return true; + } + } + + return false; + } + del() { this.emit('delete', this); } @@ -434,15 +454,15 @@ export class Link extends EventEmitter { * * @return {Link|null} */ - walk(steps: string[], stop: number = steps.length, i: number = 0): Link | null { - if (i >= steps.length) return this; - if (i >= stop) return this; - - const step = steps[i]; - const link = this.getChild(step); - if (!link) return null; - return link.walk(steps, stop, i + 1); - } + // walk(steps: string[], stop: number = steps.length, i: number = 0): Link | null { + // if (i >= steps.length) return this; + // if (i >= stop) return this; + + // const step = steps[i]; + // const link = this.getChild(step); + // if (!link) return null; + // return link.walk(steps, stop, i + 1); + // } toJSON() { return { diff --git a/src/volume.ts b/src/volume.ts index 43f31406f..1d8d965f7 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -58,6 +58,7 @@ import { getWriteSyncArgs, unixify, } from './node/util'; +import Walker from './walker'; import type { PathLike, symlink } from 'fs'; import type { FsPromisesApi, FsSynchronousApi } from './node/types'; import { fsSynchronousApiList } from './node/lists/fsSynchronousApiList'; @@ -388,6 +389,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } createNode(isDirectory: boolean = false, perm?: number): Node { + perm ??= isDirectory ? 0o777 : 0o666; const node = new this.props.Node(this.newInoNumber(), perm); if (isDirectory) node.setIsDirectory(); this.inodes[node.ino] = node; @@ -400,49 +402,127 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.releasedInos.push(node.ino); } + // private walk( + // stepsOrFilenameOrLink: string[] | string | Link, + // resolveSymlinks: boolean = false, + // checkExistence: boolean = false, + // checkAccess: boolean = false, + // funcName?: string): Link | null { + + // let steps: string[]; + // if (stepsOrFilenameOrLink instanceof Link) + // steps = stepsOrFilenameOrLink.steps; + // else if (typeof stepsOrFilenameOrLink === 'string') + // steps = filenameToSteps(stepsOrFilenameOrLink) + // else + // steps = stepsOrFilenameOrLink; + + // // Preserve the original filename for errors. + // // This is necessary because if we need to resolve symlinks, + // // steps array will be changed + // const originalFilename = steps.join(sep); + + // let curr: Link | null = this.root; + // let i = 0; + // while (i < steps.length) { + // curr = steps[i] === '' ? this.root : (curr.getChild(steps[i]) ?? null); + + // // Check existence of current link + // if (!curr) + // if (checkExistence) + // throw createError(ENOENT, funcName, originalFilename); + // else + // return null; + + // const node = curr.getNode(); + + // // Check access permissions if current link is a directory + // if (checkAccess && node.isDirectory() && !node.canExecute()) + // throw createError(EACCES, funcName, originalFilename); + + // // Resolve symlink + // if (resolveSymlinks && node.isSymlink()) { + // steps = node.symlink.concat(steps.slice(i + 1)); + // if (steps[0] !== '') steps.unshift(''); + // i = 0; + // continue; + // } + + // i++; + // } + + // return curr; + // } + // Returns a `Link` (hard link) referenced by path "split" into steps. getLink(steps: string[]): Link | null { - return this.root.walk(steps); + return new Walker(Walker.step).walk(this.root, steps); + // return this.walk(steps, false, false, false); + // return this.root.walk(steps); } // Just link `getLink`, but throws a correct user error, if link to found. + // getLinkOrThrow(filename: string, funcName?: string): Link { - const steps = filenameToSteps(filename); - const link = this.getLink(steps); - if (!link) throw createError(ENOENT, funcName, filename); - return link; + const steps: string[] = filenameToSteps(filename); + const walker = new Walker( + Walker.step, + (link: Link | null): Link => Walker.checkExistence(link, filename, funcName), + (link: Link | null): Link => Walker.checkAccess(link!, filename, funcName)); + + // The result is known to be defined, because otherwise, checkExistence would have thrown + return walker.walk(this.root, steps)!; + // return this.walk(filename, false, true, true, funcName)!; + // const steps = filenameToSteps(filename); + // const link = this.getLink(steps); + // if (!link) throw createError(ENOENT, funcName, filename); + // return link; } // Just like `getLink`, but also dereference/resolves symbolic links. getResolvedLink(filenameOrSteps: string | string[]): Link | null { - let steps: string[] = typeof filenameOrSteps === 'string' ? filenameToSteps(filenameOrSteps) : filenameOrSteps; - - let link: Link | undefined = this.root; - let i = 0; - while (i < steps.length) { - const step = steps[i]; - link = link.getChild(step); - if (!link) return null; - - const node = link.getNode(); - if (node.isSymlink()) { - steps = node.symlink.concat(steps.slice(i + 1)); - link = this.root; - i = 0; - continue; - } - - i++; - } + const steps: string[] = filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps); + const walker = new Walker(Walker.step, Walker.resolveSymlink); + return walker.walk(this.root, steps); + // return this.walk(filenameOrSteps, true, false, false); + // let steps: string[] = typeof filenameOrSteps === 'string' ? filenameToSteps(filenameOrSteps) : filenameOrSteps; + + // let link: Link | undefined = this.root; + // let i = 0; + // while (i < steps.length) { + // const step = steps[i]; + // link = link.getChild(step); + // if (!link) return null; + + // const node = link.getNode(); + // if (node.isSymlink()) { + // steps = node.symlink.concat(steps.slice(i + 1)); + // link = this.root; + // i = 0; + // continue; + // } + + // i++; + // } - return link; + // return link; } // Just like `getLinkOrThrow`, but also dereference/resolves symbolic links. getResolvedLinkOrThrow(filename: string, funcName?: string): Link { - const link = this.getResolvedLink(filename); - if (!link) throw createError(ENOENT, funcName, filename); - return link; + const steps: string[] = filenameToSteps(filename); + const walker = new Walker( + Walker.step, + (link: Link | null): Link => Walker.checkExistence(link, filename, funcName), + (link: Link | null): Link => Walker.checkAccess(link!, filename, funcName), + Walker.resolveSymlink); + + // The result is known to be defined, because otherwise, checkExistence would have thrown + return walker.walk(this.root, steps)!; + // return this.walk(filename, true, true, true, funcName)!; + // const link = this.getResolvedLink(filename); + // if (!link) throw createError(ENOENT, funcName, filename); + // return link; } resolveSymlinks(link: Link): Link | null { @@ -458,21 +538,27 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Just like `getLinkOrThrow`, but also verifies that the link is a directory. private getLinkAsDirOrThrow(filename: string, funcName?: string): Link { - const link = this.getLinkOrThrow(filename, funcName); + const link = this.getLinkOrThrow(filename, funcName)!; if (!link.getNode().isDirectory()) throw createError(ENOTDIR, funcName, filename); return link; } // Get the immediate parent directory of the link. private getLinkParent(steps: string[]): Link | null { - return this.root.walk(steps, steps.length - 1); + return this.getLink(steps.slice(0, -1)); + // return this.walk(steps.slice(0, steps.length - 1), false, false, false); + // return this.root.walk(steps, steps.length - 1); } private getLinkParentAsDirOrThrow(filenameOrSteps: string | string[], funcName?: string): Link { - const steps = filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps); - const link = this.getLinkParent(steps); - if (!link) throw createError(ENOENT, funcName, sep + steps.join(sep)); - if (!link.getNode().isDirectory()) throw createError(ENOTDIR, funcName, sep + steps.join(sep)); + const steps: string[] = (filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps)).slice(0, -1); + // const link = this.getLinkParent(steps); + // if (!link) throw createError(ENOENT, funcName, sep + steps.join(sep)); + + // const link = this.walk(steps, false, true, true, funcName)!; + const filename: string = sep + steps.join(sep); + const link = this.getLinkOrThrow(filename, funcName); + if (!link.getNode().isDirectory()) throw createError(ENOTDIR, funcName, filename); return link; } @@ -642,9 +728,11 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } // Resolve symlinks. + // + // @TODO: This should be superfluous. This method is only ever called by openFile(), which does its own symlink resolution + // prior to calling. let realLink: Link | null = link; - if (resolveSymlinks) realLink = this.resolveSymlinks(link); - if (!realLink) throw createError(ENOENT, 'open', link.getPath()); + if (resolveSymlinks) realLink = this.getResolvedLinkOrThrow(link.getPath(), 'open'); const node = realLink.getNode(); @@ -661,7 +749,10 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { throw createError(EACCES, 'open', link.getPath()); } } - if (flagsNum & O_RDWR) { + if (!(flagsNum & O_RDONLY)) { + if (!node.canWrite()) { + throw createError(EACCES, 'open', link.getPath()); + } } const file = new this.props.File(link, node, flagsNum, this.newFdNumber()); @@ -680,22 +771,32 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { resolveSymlinks: boolean = true, ): File { const steps = filenameToSteps(filename); - let link: Link | null = resolveSymlinks ? this.getResolvedLink(steps) : this.getLink(steps); - - if (link && flagsNum & O_EXCL) throw createError(EEXIST, 'open', filename); - - // Try creating a new file, if it does not exist. - if (!link && flagsNum & O_CREAT) { - // const dirLink: Link = this.getLinkParent(steps); - const dirLink: Link | null = this.getResolvedLink(steps.slice(0, steps.length - 1)); - // if(!dirLink) throw createError(ENOENT, 'open', filename); - if (!dirLink) throw createError(ENOENT, 'open', sep + steps.join(sep)); - - if (flagsNum & O_CREAT && typeof modeNum === 'number') { + let link: Link | null; + try { + link = resolveSymlinks ? this.getResolvedLinkOrThrow(filename, 'open') : this.getLinkOrThrow(filename, 'open'); + } catch(err) { + // Try creating a new file, if it does not exist and O_CREAT flag is set. + // Note that this will still throw if the ENOENT came from one of the + // intermediate directories instead of the file itself. + if (err.code === ENOENT && flagsNum & O_CREAT) { + const dirname: string = pathModule.dirname(filename); + const dirLink: Link = this.getResolvedLinkOrThrow(dirname); + const dirNode = dirLink.getNode(); + + // Check that the place we create the new file is actually a directory and that we are allowed to do so: + if (!dirNode.isDirectory()) throw createError(ENOTDIR, 'open', filename); + if (!dirNode.canExecute() || !dirNode.canWrite()) throw createError(EACCES, 'open', filename); + + // This is a difference to the original implementation, which would simply not create a file unless modeNum was specified. + // However, current Node versions will default to 0o666. + modeNum ??= 0o666; link = this.createLink(dirLink, steps[steps.length - 1], false, modeNum); - } + } else + throw err; } + if (link && flagsNum & O_EXCL) throw createError(EEXIST, 'open', filename); + if (link) return this.openLink(link, flagsNum, resolveSymlinks); throw createError(ENOENT, 'open', filename); } @@ -886,13 +987,10 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { if (userOwnsFd) fd = id as number; else { const filename = pathToFilename(id as PathLike); - const steps = filenameToSteps(filename); - const link: Link | null = this.getResolvedLink(steps); + const link: Link = this.getResolvedLinkOrThrow(filename, 'open'); - if (link) { - const node = link.getNode(); - if (node.isDirectory()) throw createError(EISDIR, 'open', link.getPath()); - } + const node = link.getNode(); + if (node.isDirectory()) throw createError(EISDIR, 'open', link.getPath()); fd = this.openSync(id as PathLike, flagsNum); } @@ -1085,17 +1183,26 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private linkBase(filename1: string, filename2: string) { - const steps1 = filenameToSteps(filename1); - const link1 = this.getLink(steps1); - if (!link1) throw createError(ENOENT, 'link', filename1, filename2); - - const steps2 = filenameToSteps(filename2); - - // Check new link directory exists. - const dir2 = this.getLinkParent(steps2); - if (!dir2) throw createError(ENOENT, 'link', filename1, filename2); + let link1: Link; + try { + link1 = this.getLinkOrThrow(filename1, 'link'); + } catch(err) { + // Augment error with filename2 + if (err.code) err = createError(err.code, 'link', filename1, filename2); + throw err; + } + + const dirname2 = pathModule.dirname(filename2); + let dir2: Link; + try { + dir2 = this.getLinkOrThrow(dirname2, 'link'); + } catch(err) { + // Augment error with filename1 + if (err.code) err = createError(err.code, 'link', filename1, filename2); + throw err; + } - const name = steps2[steps2.length - 1]; + const name = pathModule.basename(filename2); // Check if new file already exists. if (dir2.getChild(name)) throw createError(EEXIST, 'link', filename1, filename2); @@ -1163,9 +1270,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private unlinkBase(filename: string) { - const steps = filenameToSteps(filename); - const link = this.getLink(steps); - if (!link) throw createError(ENOENT, 'unlink', filename); + const link: Link = this.getLinkOrThrow(filename, 'unlink'); // TODO: Check if it is file, dir, other... @@ -1196,14 +1301,27 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { const pathSteps = filenameToSteps(pathFilename); // Check if directory exists, where we about to create a symlink. - const dirLink = this.getLinkParent(pathSteps); - if (!dirLink) throw createError(ENOENT, 'symlink', targetFilename, pathFilename); + let dirLink; + try { + dirLink = this.getLinkParentAsDirOrThrow(pathSteps); + } catch(err) { + // Catch error to populate with the correct fields - getLinkParentAsDirOrThrow won't be aware of the second path + if (err.code) + err = createError(err.code, 'symlink', targetFilename, pathFilename); + throw err; + } const name = pathSteps[pathSteps.length - 1]; // Check if new file already exists. if (dirLink.getChild(name)) throw createError(EEXIST, 'symlink', targetFilename, pathFilename); + // Check permissions on the path where we are creating the symlink. + // Note we're not checking permissions on the target path: It is not an error to create a symlink to a + // non-existent or inaccessible target + const node = dirLink.getNode(); + if (!node.canExecute() || !node.canWrite()) throw createError(EACCES, 'symlink', targetFilename, pathFilename); + // Create symlink. const symlink: Link = dirLink.createChild(name); symlink.getNode().makeSymlink(filenameToSteps(targetFilename)); @@ -1227,9 +1345,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private realpathBase(filename: string, encoding: TEncodingExtended | undefined): TDataOut { - const steps = filenameToSteps(filename); - const realLink = this.getResolvedLink(steps); - if (!realLink) throw createError(ENOENT, 'realpath', filename); + const realLink = this.getResolvedLinkOrThrow(filename, 'realpath'); return strToEncoding(realLink.getPath() || '/', encoding); } @@ -1251,15 +1367,15 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { private lstatBase(filename: string, bigint: true, throwIfNoEntry: false): Stats | undefined; private lstatBase(filename: string, bigint: false, throwIfNoEntry: false): Stats | undefined; private lstatBase(filename: string, bigint = false, throwIfNoEntry = false): Stats | undefined { - const link = this.getLink(filenameToSteps(filename)); - - if (link) { - return Stats.build(link.getNode(), bigint); - } else if (!throwIfNoEntry) { - return undefined; - } else { - throw createError(ENOENT, 'lstat', filename); + let link: Link; + try { + link = this.getLinkOrThrow(filename, 'lstat'); + } catch(err) { + if (err.code === ENOENT && !throwIfNoEntry) return undefined; + else throw err; } + + return Stats.build(link.getNode(), bigint); } lstatSync(path: PathLike): Stats; @@ -1287,14 +1403,14 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { private statBase(filename: string, bigint: true, throwIfNoEntry: false): Stats | undefined; private statBase(filename: string, bigint: false, throwIfNoEntry: false): Stats | undefined; private statBase(filename: string, bigint = false, throwIfNoEntry = true): Stats | undefined { - const link = this.getResolvedLink(filenameToSteps(filename)); - if (link) { - return Stats.build(link.getNode(), bigint); - } else if (!throwIfNoEntry) { - return undefined; - } else { - throw createError(ENOENT, 'stat', filename); + let link: Link; + try { + link = this.getResolvedLinkOrThrow(filename, 'stat'); + } catch(err) { + if (err.code === ENOENT && !throwIfNoEntry) return undefined; + else throw err; } + return Stats.build(link.getNode(), bigint); } statSync(path: PathLike): Stats; @@ -1342,28 +1458,44 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private renameBase(oldPathFilename: string, newPathFilename: string) { - const link = this.getLink(filenameToSteps(oldPathFilename)); - if (!link) throw createError(ENOENT, 'rename', oldPathFilename, newPathFilename); + let link: Link; + try { + link = this.getLinkOrThrow(oldPathFilename); + } catch(err) { + // Augment err with newPathFilename + if (err.code) err = createError(err.code, 'rename', oldPathFilename, newPathFilename); + throw err; + } // TODO: Check if it is directory, if non-empty, we cannot move it, right? - const newPathSteps = filenameToSteps(newPathFilename); - // Check directory exists for the new location. - const newPathDirLink = this.getLinkParent(newPathSteps); - if (!newPathDirLink) throw createError(ENOENT, 'rename', oldPathFilename, newPathFilename); + let newPathDirLink: Link; + try { + newPathDirLink = this.getLinkParentAsDirOrThrow(newPathFilename); + } catch(err) { + // Augment error with oldPathFilename + if (err.code) err = createError(err.code, 'rename', oldPathFilename, newPathFilename); + throw err; + } // TODO: Also treat cases with directories and symbolic links. // TODO: See: http://man7.org/linux/man-pages/man2/rename.2.html // Remove hard link from old folder. const oldLinkParent = link.parent; + + // Check we have write permissions in both places + if (!oldLinkParent.getNode().canWrite() || !newPathDirLink.getNode().canWrite()) { + throw createError(EACCES, 'rename', oldPathFilename, newPathFilename); + } + if (oldLinkParent) { oldLinkParent.deleteChild(link); } // Rename should overwrite the new path, if that exists. - const name = newPathSteps[newPathSteps.length - 1]; + const name = pathModule.basename(newPathFilename); link.name = name; link.steps = [...newPathDirLink.steps, name]; newPathDirLink.setChild(link.getName(), link); @@ -1409,8 +1541,6 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { private accessBase(filename: string, mode: number) { const link = this.getLinkOrThrow(filename, 'access'); - - // TODO: Verify permissions } accessSync(path: PathLike, mode: number = F_OK) { @@ -1459,12 +1589,14 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { private readdirBase(filename: string, options: opts.IReaddirOptions): TDataOut[] | Dirent[] { const steps = filenameToSteps(filename); - const link: Link | null = this.getResolvedLink(steps); - if (!link) throw createError(ENOENT, 'readdir', filename); + const link: Link = this.getResolvedLinkOrThrow(filename, 'scandir'); const node = link.getNode(); if (!node.isDirectory()) throw createError(ENOTDIR, 'scandir', filename); + // Check we have permissions + if (!node.canRead()) throw createError(EACCES, 'scandir', filename); + const list: Dirent[] = []; // output list for (const name of link.children.keys()) { @@ -1665,6 +1797,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { const name = steps[steps.length - 1]; if (dir.getChild(name)) throw createError(EEXIST, 'mkdir', filename); + const node = dir.getNode(); + if (!node.canWrite() || !node.canExecute()) throw createError(EACCES, 'mkdir', filename); + dir.createChild(name, this.createNode(true, modeNum)); } @@ -1674,26 +1809,49 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { * @param modeNum */ private mkdirpBase(filename: string, modeNum: number) { - const fullPath = resolve(filename); - const fullPathSansSlash = fullPath.substring(1); - const steps = !fullPathSansSlash ? [] : fullPathSansSlash.split(sep); - let link = this.root; let created = false; - for (let i = 0; i < steps.length; i++) { - const step = steps[i]; - - if (!link.getNode().isDirectory()) throw createError(ENOTDIR, 'mkdir', link.getPath()); + const walker = new Walker( + (parent: Link, step: string): Link => { + let link = step === '' ? this.root : parent.getChild(step); + + if (link) { + const node = link.getNode(); + // Check we have permissions for traversing + if (!node.canExecute()) throw createError(EACCES, 'mkdir', filename); + // Check it is, in fact, a directory + if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', link.getPath()); + } else { + const parentNode = parent.getNode(); + if (!parentNode.canWrite()) throw createError(EACCES, 'mkdir', filename); - const child = link.getChild(step); - if (child) { - if (child.getNode().isDirectory()) link = child; - else throw createError(ENOTDIR, 'mkdir', child.getPath()); - } else { - link = link.createChild(step, this.createNode(true, modeNum)); - created = true; - } - } - return created ? fullPath : undefined; + created = true; + link = parent.createChild(step, this.createNode(true, modeNum)); + } + return link; + }); + walker.walk(this.root, filenameToSteps(filename)); + return created ? filename : undefined; + + // const fullPath = resolve(filename); + // const fullPathSansSlash = fullPath.substring(1); + // const steps = !fullPathSansSlash ? [] : fullPathSansSlash.split(sep); + // let link = this.root; + // let created = false; + // for (let i = 0; i < steps.length; i++) { + // const step = steps[i]; + + // if (!link.getNode().isDirectory()) throw createError(ENOTDIR, 'mkdir', link.getPath()); + + // const child = link.getChild(step); + // if (child) { + // if (child.getNode().isDirectory()) link = child; + // else throw createError(ENOTDIR, 'mkdir', child.getPath()); + // } else { + // link = link.createChild(step, this.createNode(true, modeNum)); + // created = true; + // } + // } + // return created ? fullPath : undefined; } mkdirSync(path: PathLike, options: opts.IMkdirOptions & { recursive: true }): string | undefined; @@ -1778,18 +1936,34 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private rmBase(filename: string, options: opts.IRmOptions = {}): void { - const link = this.getResolvedLink(filename); - if (!link) { - // "stat" is used to match Node's native error message. - if (!options.force) throw createError(ENOENT, 'stat', filename); - return; - } - if (link.getNode().isDirectory()) { - if (!options.recursive) { - throw createError(ERR_FS_EISDIR, 'rm', filename); - } + // "stat" is used to match Node's native error message. + let link: Link; + try { + link = this.getResolvedLinkOrThrow(filename, 'stat'); + } catch(err) { + // Silently ignore missing paths if force option is true + if (err.code === ENOENT && options.force) return; + else throw err; } + + if (link.getNode().isDirectory() && !options.recursive) throw createError(ERR_FS_EISDIR, 'rm', filename); + + // Check permissions + if (!link.parent.getNode().canWrite()) throw createError(EACCES, 'rm', filename); + this.deleteLink(link); + + // if (!link) { + // // "stat" is used to match Node's native error message. + // if (!options.force) throw createError(ENOENT, 'stat', filename); + // return; + // } + // if (link.getNode().isDirectory()) { + // if (!options.recursive) { + // throw createError(ERR_FS_EISDIR, 'rm', filename); + // } + // } + // this.deleteLink(link); } public rmSync(path: PathLike, options?: opts.IRmOptions): void { @@ -2020,9 +2194,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { public openAsBlob: FsCallbackApi['openAsBlob'] = notImplemented; private opendirBase(filename: string, options: opts.IOpendirOptions): Dir { - const steps = filenameToSteps(filename); - const link: Link | null = this.getResolvedLink(steps); - if (!link) throw createError(ENOENT, 'opendir', filename); + const link: Link = this.getResolvedLinkOrThrow(filename, 'scandir'); const node = link.getNode(); if (!node.isDirectory()) throw createError(ENOTDIR, 'scandir', filename); diff --git a/src/walker.ts b/src/walker.ts new file mode 100644 index 000000000..b4ae5fe86 --- /dev/null +++ b/src/walker.ts @@ -0,0 +1,81 @@ +import type { Node, Link, File } from './node'; +import { createError } from './node/util'; + +const ENOENT = 'ENOENT'; +const EACCES = 'EACCES'; + +export interface IVisitor { + (link: Link | null, step: string): Link | null +} + + +/** + * Helper class that models different ways to walk a directory (sub-)tree, visiting each link and subjecting + * it to a sequence of functions. + */ +export default class Walker { + private fns: IVisitor[]; + /** + * Construct a `Walker` instance that subjects each visited link to the given functions when walking. + * Functions will be bound to the walker instance when called. + */ + constructor(...fns: IVisitor[]) { + this.fns = fns.map((fn: IVisitor): IVisitor => fn.bind(this)); + } + + /** + * Select the child given in `step` from the `link`. + * If `step` is the empty string, or `link` is not defined, return `link` itself. + * Returns `null` if there is no such child for `link`. + */ + static step(link: Link | null, step: string): Link | null { + if (step === '' || !link) return link; + return link.getChild(step) ?? null; + } + + /** + * Check that `link` exists. Throws `ENOENT` if it doesn't. + * Returns `link`. + */ + static checkExistence(link: Link | null, filename: string, funcName?: string): Link { + if (!link) throw createError(ENOENT, funcName, filename); + return link; + } + + /** + * If `link` is a directory, checks that access permissions are granted. Throws `EACCES` otherwise. + * Returns `link`. + */ + static checkAccess(link: Link, filename: string, funcName?: string): Link { + const node = link.getNode(); + if (node.isDirectory() && !node.canExecute()) + throw createError(EACCES, funcName, filename); + return link; + } + + /** + * If `link` is a symlink, resolves it and returns the symlink target. + * Otherwise returns `link`. + */ + static resolveSymlink(link: Link | null): Link | null { + const node = link?.getNode(); + if (node && node.isSymlink()) { + return (this as unknown as Walker).walk(link!.vol.root, node.symlink); + } + return link; + } + + /** + * Walk from `root` the path given by `steps`, subjecting each visited link to this walker's functions. + */ + walk(root: Link, steps: string[]): Link | null { + let link: Link | null = root; + + for (let step of steps) { + for (let fn of this.fns) { + link = fn(link, step); + } + } + return link; + } +} \ No newline at end of file From d94228227a0e1396a20620a4b0c57aa55328c813 Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 9 Sep 2024 18:54:55 +0200 Subject: [PATCH 03/16] fix: Side effects test pollution in chmod() test --- src/__tests__/promises.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/__tests__/promises.test.ts b/src/__tests__/promises.test.ts index ffe82ccb2..d3fcb2f01 100644 --- a/src/__tests__/promises.test.ts +++ b/src/__tests__/promises.test.ts @@ -38,18 +38,22 @@ describe('Promises API', () => { }); }); describe('chmod(mode)', () => { - const vol = new Volume(); - const { promises } = vol; - vol.fromJSON({ - '/foo': 'bar', - }); + let vol; + beforeEach(() => { + vol = new Volume(); + vol.fromJSON({ + '/foo': 'bar', + }); + }); it('Change mode of existing file', async () => { + const { promises } = vol; const fileHandle = await promises.open('/foo', 'a'); await fileHandle.chmod(0o444); expect(vol.statSync('/foo').mode & 0o777).toEqual(0o444); await fileHandle.close(); }); it('Reject when the file handle was closed', async () => { + const { promises } = vol; const fileHandle = await promises.open('/foo', 'a'); await fileHandle.close(); return expect(fileHandle.chmod(0o666)).rejects.toBeInstanceOf(Error); From 6daebcd184db5a65054d10284f3e0cc60cef6cab Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 9 Sep 2024 19:21:51 +0200 Subject: [PATCH 04/16] fix: Improper handling of O_EXCL in openFile() --- src/volume.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/volume.ts b/src/volume.ts index 1d8d965f7..f05bb6f52 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -774,6 +774,11 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link: Link | null; try { link = resolveSymlinks ? this.getResolvedLinkOrThrow(filename, 'open') : this.getLinkOrThrow(filename, 'open'); + + // Check if file already existed when trying to create it exclusively (O_CREAT and O_EXCL flags are set). + // This is an error, see https://pubs.opengroup.org/onlinepubs/009695399/functions/open.html: + // "If O_CREAT and O_EXCL are set, open() shall fail if the file exists." + if (link && flagsNum & O_CREAT && flagsNum & O_EXCL) throw createError(EEXIST, 'open', filename); } catch(err) { // Try creating a new file, if it does not exist and O_CREAT flag is set. // Note that this will still throw if the ENOENT came from one of the @@ -790,12 +795,12 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // This is a difference to the original implementation, which would simply not create a file unless modeNum was specified. // However, current Node versions will default to 0o666. modeNum ??= 0o666; + + link = this.createLink(dirLink, steps[steps.length - 1], false, modeNum); } else throw err; - } - - if (link && flagsNum & O_EXCL) throw createError(EEXIST, 'open', filename); + } if (link) return this.openLink(link, flagsNum, resolveSymlinks); throw createError(ENOENT, 'open', filename); From 4e96b8156e6580e2bf56e8f338883356b414d0a4 Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 9 Sep 2024 19:23:03 +0200 Subject: [PATCH 05/16] fix: No longer check execute permission on files in mkdirp While files are not a permitted place to create subdirectories in, the proper error should be ENOTDIR, not EACCES --- src/volume.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/volume.ts b/src/volume.ts index f05bb6f52..9346a80cd 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -1822,9 +1822,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { if (link) { const node = link.getNode(); // Check we have permissions for traversing - if (!node.canExecute()) throw createError(EACCES, 'mkdir', filename); + if (node.isDirectory() && !node.canExecute()) throw createError(EACCES, 'mkdir', filename); // Check it is, in fact, a directory - if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', link.getPath()); + if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', filename); } else { const parentNode = parent.getNode(); if (!parentNode.canWrite()) throw createError(EACCES, 'mkdir', filename); From 06ee20113a53fa8aa077aa6214bd387335aa48f5 Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 13 Sep 2024 14:40:39 +0200 Subject: [PATCH 06/16] test: Add tests for recursive mkdir --- src/__tests__/volume/mkdirSync.test.ts | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/__tests__/volume/mkdirSync.test.ts b/src/__tests__/volume/mkdirSync.test.ts index 6729f96f3..fd02da2f2 100644 --- a/src/__tests__/volume/mkdirSync.test.ts +++ b/src/__tests__/volume/mkdirSync.test.ts @@ -78,4 +78,38 @@ describe('mkdirSync', () => { }).toThrowError(/EACCES/) }); }); + + describe('recursive', () => { + it('can create nested directories', () => { + const vol = create({}); + const steps = [ 'a','b','c' ]; + vol.mkdirSync('/a/b/c', { recursive: true }); + expect(() => { + vol.statSync('/a/b/c'); + }).not.toThrow(); + }); + + it('throws ENOTDIR if trying to create under something that is not a directory', () => { + const vol = create({ '/a': 'I am a file' }); + expect(() => {debugger + vol.mkdirSync('/a/b/c', { recursive: true }); + }).toThrow(/ENOTDIR/); + }); + + it('throws EACCES with insufficient permissions on containing directory', () => { + const perms = [ + 0o666, // rw + 0o555, // rx + 0o111, // x + 0o222 // w + ] + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/a', { mode: perm }); + expect(() => { + vol.mkdirSync('/a/b/c', { recursive: true }); + }).toThrow(/EACCES/); + }); + }); + }); }); From 68836a58709bb3158cd0de22612e806dc6c9cbd9 Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 13 Sep 2024 14:40:54 +0200 Subject: [PATCH 07/16] test: Add tests for chmodSync --- src/__tests__/volume/chmodSync.test.ts | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 src/__tests__/volume/chmodSync.test.ts diff --git a/src/__tests__/volume/chmodSync.test.ts b/src/__tests__/volume/chmodSync.test.ts new file mode 100644 index 000000000..e8b06b1b7 --- /dev/null +++ b/src/__tests__/volume/chmodSync.test.ts @@ -0,0 +1,60 @@ +import { create } from '../util'; + +describe('chmodSync', () => { + it('should be able to chmod files and directories owned by the UID regardless of their permissions', () => { + const perms = [ + 0o777, // rwx + 0o666, // rw + 0o555, // rx + 0o444, // r + 0o333, // wx + 0o222, // w + 0o111, // x + 0o000 // none + ] + // Check for directories + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: perm }); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).not.toThrow(); + }); + // Check for files + perms.forEach(perm => { + const vol = create({ '/foo': 'foo' }); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).not.toThrow(); + }); + }); + + it.skip('should throw EPERM when trying to chmod targets not owned by the uid', () => { + const uid = process.getuid() + 1; + // Check for directories + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chownSync('/foo', uid, process.getgid()); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).toThrow(/PERM/); + }); + + it('should throw ENOENT when target doesn\'t exist', () => { + const vol = create({}); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).toThrow(/ENOENT/); + }); + + it('should chmod the target of a symlink, not the symlink itself', () => { + const vol = create({ '/target': 'contents' }); + vol.symlinkSync('/target', '/link'); + const expectedLink = vol.lstatSync('/link').mode; + const expectedTarget = vol.statSync('/target').mode & ~(0o777) + vol.chmodSync('/link', 0); + + expect(vol.lstatSync('/link').mode).toEqual(expectedLink); + expect(vol.statSync('/target').mode).toEqual(expectedTarget); + }); +}); \ No newline at end of file From ed3e6d148b4a189d4490ec745a03c2a404900660 Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 13 Sep 2024 14:45:50 +0200 Subject: [PATCH 08/16] fix: No longer require permissions to chmod --- src/volume.ts | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/volume.ts b/src/volume.ts index 9346a80cd..4d8af4f5a 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -1995,19 +1995,16 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.wrapAsync(this.fchmodBase, [fd, modeToNumber(mode)], callback); } - private chmodBase(filename: string, modeNum: number) { - const fd = this.openSync(filename, 'r'); - try { - this.fchmodBase(fd, modeNum); - } finally { - this.closeSync(fd); - } + private chmodBase(filename: string, modeNum: number, followSymlinks: boolean = true) { + const link = followSymlinks ? this.getResolvedLinkOrThrow(filename, 'chmod') : this.getLinkOrThrow(filename, 'chmod'); + const node = link.getNode(); + node.chmod(modeNum); } chmodSync(path: PathLike, mode: TMode) { const modeNum = modeToNumber(mode); const filename = pathToFilename(path); - this.chmodBase(filename, modeNum); + this.chmodBase(filename, modeNum, true); } chmod(path: PathLike, mode: TMode, callback: TCallback) { @@ -2017,12 +2014,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private lchmodBase(filename: string, modeNum: number) { - const fd = this.openBase(filename, O_RDWR, 0, false); - try { - this.fchmodBase(fd, modeNum); - } finally { - this.closeSync(fd); - } + this.chmodBase(filename, modeNum, false); } lchmodSync(path: PathLike, mode: TMode) { From b5ac3fb79a481474eb49b4d103ad394817bdca3c Mon Sep 17 00:00:00 2001 From: chris Date: Sat, 14 Sep 2024 07:58:27 +0200 Subject: [PATCH 09/16] test: Add more extensive tests for mkdir recursive mode --- src/__tests__/volume/mkdirSync.test.ts | 35 +++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/__tests__/volume/mkdirSync.test.ts b/src/__tests__/volume/mkdirSync.test.ts index fd02da2f2..d0ec67e7c 100644 --- a/src/__tests__/volume/mkdirSync.test.ts +++ b/src/__tests__/volume/mkdirSync.test.ts @@ -80,16 +80,45 @@ describe('mkdirSync', () => { }); describe('recursive', () => { - it('can create nested directories', () => { + it('can create nested directories when none exist', () => { const vol = create({}); - const steps = [ 'a','b','c' ]; vol.mkdirSync('/a/b/c', { recursive: true }); expect(() => { vol.statSync('/a/b/c'); }).not.toThrow(); }); - it('throws ENOTDIR if trying to create under something that is not a directory', () => { + it('can create nested directories when some exist', () => { + const vol = create({}); + vol.mkdirSync('/a'); + vol.mkdirSync('/a/b/c', { recursive: true }); + expect(() => { + vol.statSync('/a/b/c'); + }).not.toThrow(); + }); + + it('can create nested directories when all exist', () => { + const vol = create({}); + vol.mkdirSync('/a'); + vol.mkdirSync('/a/b'); + vol.mkdirSync('/a/b/c'); + vol.mkdirSync('/a/b/c', { recursive: true }); + expect(() => { + vol.statSync('/a/b/c'); + }).not.toThrow(); + }); + + it('can create directories under symlinks', () => { + const vol = create({}); + vol.mkdirSync('/target'); + vol.symlinkSync('/target', '/a'); + vol.mkdirSync('/a/b/c', { recursive: true }); + expect(() => { + vol.statSync('/a/b/c'); + }).not.toThrow(); + }); + + it('throws ENOTDIR when trying to create under something that is not a directory', () => { const vol = create({ '/a': 'I am a file' }); expect(() => {debugger vol.mkdirSync('/a/b/c', { recursive: true }); From 137ef35ae5590f3f64df7b3692872affb1cd952a Mon Sep 17 00:00:00 2001 From: chris Date: Sat, 14 Sep 2024 08:00:12 +0200 Subject: [PATCH 10/16] refactor: Inline tree walking --- src/volume.ts | 234 ++++++++++++++++++++++++++++---------------------- src/walker.ts | 81 ----------------- 2 files changed, 131 insertions(+), 184 deletions(-) delete mode 100644 src/walker.ts diff --git a/src/volume.ts b/src/volume.ts index 4d8af4f5a..b409c0e8e 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -58,7 +58,6 @@ import { getWriteSyncArgs, unixify, } from './node/util'; -import Walker from './walker'; import type { PathLike, symlink } from 'fs'; import type { FsPromisesApi, FsSynchronousApi } from './node/types'; import { fsSynchronousApiList } from './node/lists/fsSynchronousApiList'; @@ -402,77 +401,84 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.releasedInos.push(node.ino); } - // private walk( - // stepsOrFilenameOrLink: string[] | string | Link, - // resolveSymlinks: boolean = false, - // checkExistence: boolean = false, - // checkAccess: boolean = false, - // funcName?: string): Link | null { + private walk(steps: string[], resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null + private walk(filename: string, resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null + private walk(link: Link, resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null + private walk( + stepsOrFilenameOrLink: string[] | string | Link, + resolveSymlinks: boolean, + checkExistence: boolean, + checkAccess: boolean, + funcName?: string): Link | null; + private walk( + stepsOrFilenameOrLink: string[] | string | Link, + resolveSymlinks: boolean = false, + checkExistence: boolean = false, + checkAccess: boolean = false, + funcName?: string): Link | null { - // let steps: string[]; - // if (stepsOrFilenameOrLink instanceof Link) - // steps = stepsOrFilenameOrLink.steps; - // else if (typeof stepsOrFilenameOrLink === 'string') - // steps = filenameToSteps(stepsOrFilenameOrLink) - // else - // steps = stepsOrFilenameOrLink; - - // // Preserve the original filename for errors. - // // This is necessary because if we need to resolve symlinks, - // // steps array will be changed - // const originalFilename = steps.join(sep); - - // let curr: Link | null = this.root; - // let i = 0; - // while (i < steps.length) { - // curr = steps[i] === '' ? this.root : (curr.getChild(steps[i]) ?? null); - - // // Check existence of current link - // if (!curr) - // if (checkExistence) - // throw createError(ENOENT, funcName, originalFilename); - // else - // return null; - - // const node = curr.getNode(); - - // // Check access permissions if current link is a directory - // if (checkAccess && node.isDirectory() && !node.canExecute()) - // throw createError(EACCES, funcName, originalFilename); - - // // Resolve symlink - // if (resolveSymlinks && node.isSymlink()) { - // steps = node.symlink.concat(steps.slice(i + 1)); - // if (steps[0] !== '') steps.unshift(''); - // i = 0; - // continue; - // } - - // i++; - // } - - // return curr; - // } + let steps: string[]; + let filename: string; + if (stepsOrFilenameOrLink instanceof Link) { + steps = stepsOrFilenameOrLink.steps; + filename = sep + steps.join(sep); + } + else if (typeof stepsOrFilenameOrLink === 'string') { + steps = filenameToSteps(stepsOrFilenameOrLink) + filename = stepsOrFilenameOrLink; + } + else { + steps = stepsOrFilenameOrLink; + filename = sep + steps.join(sep); + } + + let curr: Link | null = this.root; + let i = 0; + while (i < steps.length) { + let node: Node = curr.getNode(); + // Check access permissions if current link is a directory + if (node.isDirectory()) { + if (checkAccess && !node.canExecute()) { + throw createError(EACCES, funcName, filename); + } + } else { + if (i < steps.length - 1) throw createError(ENOTDIR, funcName, filename); + } + + curr = curr.getChild(steps[i]) ?? null; + + // Check existence of current link + if (!curr) + if (checkExistence) + throw createError(ENOENT, funcName, filename); + else + return null; + + node = curr?.getNode(); + // Resolve symlink + if (resolveSymlinks && node.isSymlink()) { + steps = node.symlink.concat(steps.slice(i + 1)); + curr = this.root; + i = 0; + continue; + } + + i++; + } + + return curr; + } // Returns a `Link` (hard link) referenced by path "split" into steps. getLink(steps: string[]): Link | null { - return new Walker(Walker.step).walk(this.root, steps); - // return this.walk(steps, false, false, false); + return this.walk(steps, false, false, false); // return this.root.walk(steps); } // Just link `getLink`, but throws a correct user error, if link to found. // getLinkOrThrow(filename: string, funcName?: string): Link { - const steps: string[] = filenameToSteps(filename); - const walker = new Walker( - Walker.step, - (link: Link | null): Link => Walker.checkExistence(link, filename, funcName), - (link: Link | null): Link => Walker.checkAccess(link!, filename, funcName)); - - // The result is known to be defined, because otherwise, checkExistence would have thrown - return walker.walk(this.root, steps)!; - // return this.walk(filename, false, true, true, funcName)!; + return this.walk(filename, false, true, true, funcName)!; // const steps = filenameToSteps(filename); // const link = this.getLink(steps); // if (!link) throw createError(ENOENT, funcName, filename); @@ -481,10 +487,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Just like `getLink`, but also dereference/resolves symbolic links. getResolvedLink(filenameOrSteps: string | string[]): Link | null { - const steps: string[] = filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps); - const walker = new Walker(Walker.step, Walker.resolveSymlink); - return walker.walk(this.root, steps); - // return this.walk(filenameOrSteps, true, false, false); + return this.walk(filenameOrSteps, true, false, false); // let steps: string[] = typeof filenameOrSteps === 'string' ? filenameToSteps(filenameOrSteps) : filenameOrSteps; // let link: Link | undefined = this.root; @@ -510,16 +513,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Just like `getLinkOrThrow`, but also dereference/resolves symbolic links. getResolvedLinkOrThrow(filename: string, funcName?: string): Link { - const steps: string[] = filenameToSteps(filename); - const walker = new Walker( - Walker.step, - (link: Link | null): Link => Walker.checkExistence(link, filename, funcName), - (link: Link | null): Link => Walker.checkAccess(link!, filename, funcName), - Walker.resolveSymlink); - - // The result is known to be defined, because otherwise, checkExistence would have thrown - return walker.walk(this.root, steps)!; - // return this.walk(filename, true, true, true, funcName)!; + return this.walk(filename, true, true, true, funcName)!; // const link = this.getResolvedLink(filename); // if (!link) throw createError(ENOENT, funcName, filename); // return link; @@ -1349,7 +1343,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.wrapAsync(this.symlinkBase, [targetFilename, pathFilename], callback); } - private realpathBase(filename: string, encoding: TEncodingExtended | undefined): TDataOut { + private realpathBase(filename: string, encoding: TEncodingExtended | undefined): TDataOut {debugger const realLink = this.getResolvedLinkOrThrow(filename, 'realpath'); return strToEncoding(realLink.getPath() || '/', encoding); @@ -1465,7 +1459,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { private renameBase(oldPathFilename: string, newPathFilename: string) { let link: Link; try { - link = this.getLinkOrThrow(oldPathFilename); + link = this.getResolvedLinkOrThrow(oldPathFilename); } catch(err) { // Augment err with newPathFilename if (err.code) err = createError(err.code, 'rename', oldPathFilename, newPathFilename); @@ -1490,14 +1484,14 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Remove hard link from old folder. const oldLinkParent = link.parent; - // Check we have write permissions in both places - if (!oldLinkParent.getNode().canWrite() || !newPathDirLink.getNode().canWrite()) { - throw createError(EACCES, 'rename', oldPathFilename, newPathFilename); + // Check we have access and write permissions in both places + const oldParentNode: Node = oldLinkParent.getNode(); + const newPathDirNode: Node = newPathDirLink.getNode(); + if (!oldParentNode.canExecute() || !oldParentNode.canWrite() || !newPathDirNode.canExecute() || !newPathDirNode.canWrite()) { + throw createError(EACCES, 'rename', oldPathFilename, newPathFilename); } - if (oldLinkParent) { - oldLinkParent.deleteChild(link); - } + oldLinkParent.deleteChild(link); // Rename should overwrite the new path, if that exists. const name = pathModule.basename(newPathFilename); @@ -1815,28 +1809,62 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { */ private mkdirpBase(filename: string, modeNum: number) { let created = false; - const walker = new Walker( - (parent: Link, step: string): Link => { - let link = step === '' ? this.root : parent.getChild(step); - - if (link) { - const node = link.getNode(); - // Check we have permissions for traversing - if (node.isDirectory() && !node.canExecute()) throw createError(EACCES, 'mkdir', filename); - // Check it is, in fact, a directory - if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', filename); - } else { - const parentNode = parent.getNode(); - if (!parentNode.canWrite()) throw createError(EACCES, 'mkdir', filename); + const steps = filenameToSteps(filename); - created = true; - link = parent.createChild(step, this.createNode(true, modeNum)); - } - return link; - }); - walker.walk(this.root, filenameToSteps(filename)); + let curr: Link | null = null; + let i = steps.length; + // Find the longest subpath of filename that still exists: + for (i = steps.length; i >= 0; i--) { + curr = this.getResolvedLink(steps.slice(0, i)); + if (curr) break; + } + if (!curr) { + curr = this.root; + i = 0; + } + // curr is now the last directory that still exists. + // (If none of them existed, curr is the root.) + // Check access the lazy way: + curr = this.getResolvedLinkOrThrow(sep + steps.slice(0, i).join(sep), 'mkdir'); + + // Start creating directories: + for (i; i < steps.length; i++) { + const node = curr.getNode(); + + if (node.isDirectory()) { + // Check we have permissions + if (!node.canExecute() || !node.canWrite()) throw createError(EACCES, 'mkdir', filename); + } else { + throw createError(ENOTDIR, 'mkdir', filename); + } + + created = true; + curr = curr.createChild(steps[i], this.createNode(true, modeNum)); + } return created ? filename : undefined; + // const walker = new Walker( + // (parent: Link, step: string): Link => { + // let link = step === '' ? this.root : parent.getChild(step); + + // if (link) { + // const node = link.getNode(); + // // Check we have permissions for traversing + // if (node.isDirectory() && !node.canExecute()) throw createError(EACCES, 'mkdir', filename); + // // Check it is, in fact, a directory + // if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', filename); + // } else { + // const parentNode = parent.getNode(); + // if (!parentNode.canWrite()) throw createError(EACCES, 'mkdir', filename); + + // created = true; + // link = parent.createChild(step, this.createNode(true, modeNum)); + // } + // return link; + // }); + // walker.walk(this.root, filenameToSteps(filename)); + // return created ? filename : undefined; + // const fullPath = resolve(filename); // const fullPathSansSlash = fullPath.substring(1); // const steps = !fullPathSansSlash ? [] : fullPathSansSlash.split(sep); diff --git a/src/walker.ts b/src/walker.ts deleted file mode 100644 index b4ae5fe86..000000000 --- a/src/walker.ts +++ /dev/null @@ -1,81 +0,0 @@ -import type { Node, Link, File } from './node'; -import { createError } from './node/util'; - -const ENOENT = 'ENOENT'; -const EACCES = 'EACCES'; - -export interface IVisitor { - (link: Link | null, step: string): Link | null -} - - -/** - * Helper class that models different ways to walk a directory (sub-)tree, visiting each link and subjecting - * it to a sequence of functions. - */ -export default class Walker { - private fns: IVisitor[]; - /** - * Construct a `Walker` instance that subjects each visited link to the given functions when walking. - * Functions will be bound to the walker instance when called. - */ - constructor(...fns: IVisitor[]) { - this.fns = fns.map((fn: IVisitor): IVisitor => fn.bind(this)); - } - - /** - * Select the child given in `step` from the `link`. - * If `step` is the empty string, or `link` is not defined, return `link` itself. - * Returns `null` if there is no such child for `link`. - */ - static step(link: Link | null, step: string): Link | null { - if (step === '' || !link) return link; - return link.getChild(step) ?? null; - } - - /** - * Check that `link` exists. Throws `ENOENT` if it doesn't. - * Returns `link`. - */ - static checkExistence(link: Link | null, filename: string, funcName?: string): Link { - if (!link) throw createError(ENOENT, funcName, filename); - return link; - } - - /** - * If `link` is a directory, checks that access permissions are granted. Throws `EACCES` otherwise. - * Returns `link`. - */ - static checkAccess(link: Link, filename: string, funcName?: string): Link { - const node = link.getNode(); - if (node.isDirectory() && !node.canExecute()) - throw createError(EACCES, funcName, filename); - return link; - } - - /** - * If `link` is a symlink, resolves it and returns the symlink target. - * Otherwise returns `link`. - */ - static resolveSymlink(link: Link | null): Link | null { - const node = link?.getNode(); - if (node && node.isSymlink()) { - return (this as unknown as Walker).walk(link!.vol.root, node.symlink); - } - return link; - } - - /** - * Walk from `root` the path given by `steps`, subjecting each visited link to this walker's functions. - */ - walk(root: Link, steps: string[]): Link | null { - let link: Link | null = root; - - for (let step of steps) { - for (let fn of this.fns) { - link = fn(link, step); - } - } - return link; - } -} \ No newline at end of file From 1030618737ada09554690dd63075ffbe1f90cde9 Mon Sep 17 00:00:00 2001 From: chris Date: Sat, 14 Sep 2024 08:10:04 +0200 Subject: [PATCH 11/16] chore: cleanup --- src/volume.ts | 100 -------------------------------------------------- 1 file changed, 100 deletions(-) diff --git a/src/volume.ts b/src/volume.ts index b409c0e8e..557da4d61 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -472,61 +472,24 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Returns a `Link` (hard link) referenced by path "split" into steps. getLink(steps: string[]): Link | null { return this.walk(steps, false, false, false); - // return this.root.walk(steps); } // Just link `getLink`, but throws a correct user error, if link to found. - // getLinkOrThrow(filename: string, funcName?: string): Link { return this.walk(filename, false, true, true, funcName)!; - // const steps = filenameToSteps(filename); - // const link = this.getLink(steps); - // if (!link) throw createError(ENOENT, funcName, filename); - // return link; } // Just like `getLink`, but also dereference/resolves symbolic links. getResolvedLink(filenameOrSteps: string | string[]): Link | null { return this.walk(filenameOrSteps, true, false, false); - // let steps: string[] = typeof filenameOrSteps === 'string' ? filenameToSteps(filenameOrSteps) : filenameOrSteps; - - // let link: Link | undefined = this.root; - // let i = 0; - // while (i < steps.length) { - // const step = steps[i]; - // link = link.getChild(step); - // if (!link) return null; - - // const node = link.getNode(); - // if (node.isSymlink()) { - // steps = node.symlink.concat(steps.slice(i + 1)); - // link = this.root; - // i = 0; - // continue; - // } - - // i++; - // } - - // return link; } // Just like `getLinkOrThrow`, but also dereference/resolves symbolic links. getResolvedLinkOrThrow(filename: string, funcName?: string): Link { return this.walk(filename, true, true, true, funcName)!; - // const link = this.getResolvedLink(filename); - // if (!link) throw createError(ENOENT, funcName, filename); - // return link; } resolveSymlinks(link: Link): Link | null { - // let node: Node = link.getNode(); - // while(link && node.isSymlink()) { - // link = this.getLink(node.symlink); - // if(!link) return null; - // node = link.getNode(); - // } - // return link; return this.getResolvedLink(link.steps.slice(1)); } @@ -540,16 +503,10 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Get the immediate parent directory of the link. private getLinkParent(steps: string[]): Link | null { return this.getLink(steps.slice(0, -1)); - // return this.walk(steps.slice(0, steps.length - 1), false, false, false); - // return this.root.walk(steps, steps.length - 1); } private getLinkParentAsDirOrThrow(filenameOrSteps: string | string[], funcName?: string): Link { const steps: string[] = (filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps)).slice(0, -1); - // const link = this.getLinkParent(steps); - // if (!link) throw createError(ENOENT, funcName, sep + steps.join(sep)); - - // const link = this.walk(steps, false, true, true, funcName)!; const filename: string = sep + steps.join(sep); const link = this.getLinkOrThrow(filename, funcName); if (!link.getNode().isDirectory()) throw createError(ENOTDIR, funcName, filename); @@ -1804,8 +1761,6 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { /** * Creates directory tree recursively. - * @param filename - * @param modeNum */ private mkdirpBase(filename: string, modeNum: number) { let created = false; @@ -1842,49 +1797,6 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { curr = curr.createChild(steps[i], this.createNode(true, modeNum)); } return created ? filename : undefined; - - // const walker = new Walker( - // (parent: Link, step: string): Link => { - // let link = step === '' ? this.root : parent.getChild(step); - - // if (link) { - // const node = link.getNode(); - // // Check we have permissions for traversing - // if (node.isDirectory() && !node.canExecute()) throw createError(EACCES, 'mkdir', filename); - // // Check it is, in fact, a directory - // if (!node.isDirectory()) throw createError(ENOTDIR, 'mkdir', filename); - // } else { - // const parentNode = parent.getNode(); - // if (!parentNode.canWrite()) throw createError(EACCES, 'mkdir', filename); - - // created = true; - // link = parent.createChild(step, this.createNode(true, modeNum)); - // } - // return link; - // }); - // walker.walk(this.root, filenameToSteps(filename)); - // return created ? filename : undefined; - - // const fullPath = resolve(filename); - // const fullPathSansSlash = fullPath.substring(1); - // const steps = !fullPathSansSlash ? [] : fullPathSansSlash.split(sep); - // let link = this.root; - // let created = false; - // for (let i = 0; i < steps.length; i++) { - // const step = steps[i]; - - // if (!link.getNode().isDirectory()) throw createError(ENOTDIR, 'mkdir', link.getPath()); - - // const child = link.getChild(step); - // if (child) { - // if (child.getNode().isDirectory()) link = child; - // else throw createError(ENOTDIR, 'mkdir', child.getPath()); - // } else { - // link = link.createChild(step, this.createNode(true, modeNum)); - // created = true; - // } - // } - // return created ? fullPath : undefined; } mkdirSync(path: PathLike, options: opts.IMkdirOptions & { recursive: true }): string | undefined; @@ -1985,18 +1897,6 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { if (!link.parent.getNode().canWrite()) throw createError(EACCES, 'rm', filename); this.deleteLink(link); - - // if (!link) { - // // "stat" is used to match Node's native error message. - // if (!options.force) throw createError(ENOENT, 'stat', filename); - // return; - // } - // if (link.getNode().isDirectory()) { - // if (!options.recursive) { - // throw createError(ERR_FS_EISDIR, 'rm', filename); - // } - // } - // this.deleteLink(link); } public rmSync(path: PathLike, options?: opts.IRmOptions): void { From 9422c7b1d1b040c98af90fb5718c6d539bcf48f4 Mon Sep 17 00:00:00 2001 From: chris Date: Sat, 14 Sep 2024 09:01:10 +0200 Subject: [PATCH 12/16] style: Run prettier --- src/__tests__/promises.test.ts | 2 +- src/__tests__/util.ts | 4 +- src/__tests__/volume/ReadStream.test.ts | 32 +++--- src/__tests__/volume/WriteStream.test.ts | 50 +++++---- src/__tests__/volume/appendFile.test.ts | 12 +- src/__tests__/volume/chmodSync.test.ts | 106 +++++++++--------- src/__tests__/volume/copyFile.test.ts | 12 +- src/__tests__/volume/copyFileSync.test.ts | 10 +- src/__tests__/volume/exists.test.ts | 2 +- src/__tests__/volume/mkdirSync.test.ts | 15 +-- src/__tests__/volume/openSync.test.ts | 18 +-- src/__tests__/volume/readSync.test.ts | 4 +- src/__tests__/volume/realpathSync.test.ts | 2 +- src/__tests__/volume/rename.test.ts | 10 +- src/__tests__/volume/renameSync.test.ts | 4 +- src/__tests__/volume/rmPromise.test.ts | 14 ++- src/__tests__/volume/rmSync.test.ts | 2 +- src/__tests__/volume/statSync.test.ts | 2 +- src/__tests__/volume/write.test.ts | 4 +- src/__tests__/volume/writeFileSync.test.ts | 8 +- src/__tests__/volume/writeSync.test.ts | 4 +- src/volume.ts | 124 ++++++++++++--------- 22 files changed, 238 insertions(+), 203 deletions(-) diff --git a/src/__tests__/promises.test.ts b/src/__tests__/promises.test.ts index d3fcb2f01..6a768b677 100644 --- a/src/__tests__/promises.test.ts +++ b/src/__tests__/promises.test.ts @@ -44,7 +44,7 @@ describe('Promises API', () => { vol.fromJSON({ '/foo': 'bar', }); - }); + }); it('Change mode of existing file', async () => { const { promises } = vol; const fileHandle = await promises.open('/foo', 'a'); diff --git a/src/__tests__/util.ts b/src/__tests__/util.ts index df6c47446..6a8851d21 100644 --- a/src/__tests__/util.ts +++ b/src/__tests__/util.ts @@ -10,8 +10,8 @@ export const multitest = (_done: (err?: Error) => void, times: number) => { return function done(_err?: Error) { err ??= _err; if (!--times) _done(_err); - } -} + }; +}; export const create = (json: { [s: string]: string } = { '/foo': 'bar' }) => { const vol = Volume.fromJSON(json); diff --git a/src/__tests__/volume/ReadStream.test.ts b/src/__tests__/volume/ReadStream.test.ts index 9fa2a9ff7..2ad132337 100644 --- a/src/__tests__/volume/ReadStream.test.ts +++ b/src/__tests__/volume/ReadStream.test.ts @@ -22,24 +22,28 @@ describe('ReadStream', () => { it('should emit EACCES error when file has insufficient permissions', done => { const fs = createFs({ '/test': 'test' }); fs.chmodSync('/test', 0o333); // wx - new fs.ReadStream('/test').on('error', err => { - expect(err).toBeInstanceOf(Error); - expect(err).toHaveProperty('code', 'EACCES'); - done(); - }).on('open', () => { - done(new Error('Expected ReadStream to emit EACCES but it didn\'t')); - });; + new fs.ReadStream('/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected ReadStream to emit EACCES but it didn't")); + }); }); it('should emit EACCES error when containing directory has insufficient permissions', done => { const fs = createFs({ '/foo/test': 'test' }); fs.chmodSync('/foo', 0o666); // rw - new fs.ReadStream('/foo/test').on('error', err => { - expect(err).toBeInstanceOf(Error); - expect(err).toHaveProperty('code', 'EACCES'); - done(); - }).on('open', () => { - done(new Error('Expected ReadStream to emit EACCES but it didn\'t')); - }); + new fs.ReadStream('/foo/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected ReadStream to emit EACCES but it didn't")); + }); }); }); diff --git a/src/__tests__/volume/WriteStream.test.ts b/src/__tests__/volume/WriteStream.test.ts index b77bc55f1..d8157188e 100644 --- a/src/__tests__/volume/WriteStream.test.ts +++ b/src/__tests__/volume/WriteStream.test.ts @@ -23,36 +23,42 @@ describe('WriteStream', () => { it('should emit EACCES error when file has insufficient permissions', done => { const fs = createFs({ '/test': 'test' }); fs.chmodSync('/test', 0o555); // rx - new fs.WriteStream('/test').on('error', err => { - expect(err).toBeInstanceOf(Error); - expect(err).toHaveProperty('code', 'EACCES'); - done(); - }).on('open', () => { - done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); - });; + new fs.WriteStream('/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected WriteStream to emit EACCES but it didn't")); + }); }); it('should emit EACCES error for an existing file when containing directory has insufficient permissions', done => { const fs = createFs({ '/foo/test': 'test' }); fs.chmodSync('/foo', 0o666); // rw - new fs.WriteStream('/foo/test').on('error', err => { - expect(err).toBeInstanceOf(Error); - expect(err).toHaveProperty('code', 'EACCES'); - done(); - }).on('open', () => { - done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); - }); + new fs.WriteStream('/foo/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected WriteStream to emit EACCES but it didn't")); + }); }); it('should emit EACCES error for an non-existant file when containing directory has insufficient permissions', done => { const fs = createFs({}); - fs.mkdirSync('/foo', { mode: 0o555 }); // rx - new fs.WriteStream('/foo/test').on('error', err => { - expect(err).toBeInstanceOf(Error); - expect(err).toHaveProperty('code', 'EACCES'); - done(); - }).on('open', () => { - done(new Error('Expected WriteStream to emit EACCES but it didn\'t')); - }); + fs.mkdirSync('/foo', { mode: 0o555 }); // rx + new fs.WriteStream('/foo/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected WriteStream to emit EACCES but it didn't")); + }); }); }); diff --git a/src/__tests__/volume/appendFile.test.ts b/src/__tests__/volume/appendFile.test.ts index 168b1510c..51b452243 100644 --- a/src/__tests__/volume/appendFile.test.ts +++ b/src/__tests__/volume/appendFile.test.ts @@ -24,7 +24,7 @@ describe('appendFile(file, data[, options], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(x) { + } catch (x) { done(x); } }); @@ -33,22 +33,22 @@ describe('appendFile(file, data[, options], callback)', () => { it('Appending gives EACCES if file does not exist and containing directory has insufficient permissions', _done => { const perms = [ 0o555, // rx across the board - 0o666 // rw across the board + 0o666, // rw across the board ]; const done = multitest(_done, perms.length); - + perms.forEach(perm => { - const vol = create({}); + const vol = create({}); vol.mkdirSync('/foo', { mode: perm }); vol.appendFile('/foo/test', 'bar', err => { try { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(x) { + } catch (x) { done(x); } }); - }) + }); }); }); diff --git a/src/__tests__/volume/chmodSync.test.ts b/src/__tests__/volume/chmodSync.test.ts index e8b06b1b7..03b6b76a2 100644 --- a/src/__tests__/volume/chmodSync.test.ts +++ b/src/__tests__/volume/chmodSync.test.ts @@ -1,60 +1,60 @@ import { create } from '../util'; describe('chmodSync', () => { - it('should be able to chmod files and directories owned by the UID regardless of their permissions', () => { - const perms = [ - 0o777, // rwx - 0o666, // rw - 0o555, // rx - 0o444, // r - 0o333, // wx - 0o222, // w - 0o111, // x - 0o000 // none - ] - // Check for directories - perms.forEach(perm => { - const vol = create({}); - vol.mkdirSync('/foo', { mode: perm }); - expect(() => { - vol.chmodSync('/foo', 0o777); - }).not.toThrow(); - }); - // Check for files - perms.forEach(perm => { - const vol = create({ '/foo': 'foo' }); - expect(() => { - vol.chmodSync('/foo', 0o777); - }).not.toThrow(); - }); - }); + it('should be able to chmod files and directories owned by the UID regardless of their permissions', () => { + const perms = [ + 0o777, // rwx + 0o666, // rw + 0o555, // rx + 0o444, // r + 0o333, // wx + 0o222, // w + 0o111, // x + 0o000, // none + ]; + // Check for directories + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: perm }); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).not.toThrow(); + }); + // Check for files + perms.forEach(perm => { + const vol = create({ '/foo': 'foo' }); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).not.toThrow(); + }); + }); - it.skip('should throw EPERM when trying to chmod targets not owned by the uid', () => { - const uid = process.getuid() + 1; - // Check for directories - const vol = create({}); - vol.mkdirSync('/foo'); - vol.chownSync('/foo', uid, process.getgid()); - expect(() => { - vol.chmodSync('/foo', 0o777); - }).toThrow(/PERM/); - }); + it.skip('should throw EPERM when trying to chmod targets not owned by the uid', () => { + const uid = process.getuid() + 1; + // Check for directories + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chownSync('/foo', uid, process.getgid()); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).toThrow(/PERM/); + }); - it('should throw ENOENT when target doesn\'t exist', () => { - const vol = create({}); - expect(() => { - vol.chmodSync('/foo', 0o777); - }).toThrow(/ENOENT/); - }); + it("should throw ENOENT when target doesn't exist", () => { + const vol = create({}); + expect(() => { + vol.chmodSync('/foo', 0o777); + }).toThrow(/ENOENT/); + }); - it('should chmod the target of a symlink, not the symlink itself', () => { - const vol = create({ '/target': 'contents' }); - vol.symlinkSync('/target', '/link'); - const expectedLink = vol.lstatSync('/link').mode; - const expectedTarget = vol.statSync('/target').mode & ~(0o777) - vol.chmodSync('/link', 0); + it('should chmod the target of a symlink, not the symlink itself', () => { + const vol = create({ '/target': 'contents' }); + vol.symlinkSync('/target', '/link'); + const expectedLink = vol.lstatSync('/link').mode; + const expectedTarget = vol.statSync('/target').mode & ~0o777; + vol.chmodSync('/link', 0); - expect(vol.lstatSync('/link').mode).toEqual(expectedLink); - expect(vol.statSync('/target').mode).toEqual(expectedTarget); - }); -}); \ No newline at end of file + expect(vol.lstatSync('/link').mode).toEqual(expectedLink); + expect(vol.statSync('/target').mode).toEqual(expectedTarget); + }); +}); diff --git a/src/__tests__/volume/copyFile.test.ts b/src/__tests__/volume/copyFile.test.ts index 3f3cb7062..2594ad236 100644 --- a/src/__tests__/volume/copyFile.test.ts +++ b/src/__tests__/volume/copyFile.test.ts @@ -72,12 +72,12 @@ describe('copyFile(src, dest[, flags], callback)', () => { }); }); - it('copying yields EACCES with insufficient permissions on the destination directory', _done => { + it('copying yields EACCES with insufficient permissions on the destination directory', _done => { const perms = [ - 0o555, // rx - 0o666, // rw - 0o111, // x - 0o222 // w + 0o555, // rx + 0o666, // rw + 0o111, // x + 0o222, // w ]; const done = multitest(_done, perms.length); @@ -90,7 +90,7 @@ describe('copyFile(src, dest[, flags], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(err) { + } catch (err) { done(err); } }); diff --git a/src/__tests__/volume/copyFileSync.test.ts b/src/__tests__/volume/copyFileSync.test.ts index 0ab968ef3..00f73fe1c 100644 --- a/src/__tests__/volume/copyFileSync.test.ts +++ b/src/__tests__/volume/copyFileSync.test.ts @@ -111,10 +111,10 @@ describe('copyFileSync(src, dest[, flags])', () => { it('copying throws EACCES with insufficient permissions on the destination directory', () => { const perms = [ - 0o555, // rx - 0o666, // rw - 0o111, // x - 0o222 // w + 0o555, // rx + 0o666, // rw + 0o111, // x + 0o222, // w ]; perms.forEach(perm => { const vol = create({ '/foo': 'foo' }); @@ -122,7 +122,7 @@ describe('copyFileSync(src, dest[, flags])', () => { vol.chmodSync('/bar', perm); expect(() => { vol.copyFileSync('/foo', '/bar/foo'); - }).toThrowError(/EACCES/); + }).toThrowError(/EACCES/); }); }); }); diff --git a/src/__tests__/volume/exists.test.ts b/src/__tests__/volume/exists.test.ts index 58a8a820e..fe9a75b0b 100644 --- a/src/__tests__/volume/exists.test.ts +++ b/src/__tests__/volume/exists.test.ts @@ -41,7 +41,7 @@ describe('exists(path, callback)', () => { try { expect(exists).toEqual(false); done(); - } catch(err) { + } catch (err) { done(err); } }); diff --git a/src/__tests__/volume/mkdirSync.test.ts b/src/__tests__/volume/mkdirSync.test.ts index d0ec67e7c..e31b13d83 100644 --- a/src/__tests__/volume/mkdirSync.test.ts +++ b/src/__tests__/volume/mkdirSync.test.ts @@ -67,15 +67,15 @@ describe('mkdirSync', () => { it('throws EACCES with insufficient permissions on containing directory', () => { const perms = [ 0o666, // rw across the board - 0o555 // rx across the bord - ] + 0o555, // rx across the bord + ]; perms.forEach(perm => { const vol = create({}); vol.mkdirSync('/foo'); vol.chmodSync('/foo', perm); expect(() => { vol.mkdirSync(`/foo/bar`); - }).toThrowError(/EACCES/) + }).toThrowError(/EACCES/); }); }); @@ -120,8 +120,9 @@ describe('mkdirSync', () => { it('throws ENOTDIR when trying to create under something that is not a directory', () => { const vol = create({ '/a': 'I am a file' }); - expect(() => {debugger - vol.mkdirSync('/a/b/c', { recursive: true }); + expect(() => { + debugger; + vol.mkdirSync('/a/b/c', { recursive: true }); }).toThrow(/ENOTDIR/); }); @@ -130,8 +131,8 @@ describe('mkdirSync', () => { 0o666, // rw 0o555, // rx 0o111, // x - 0o222 // w - ] + 0o222, // w + ]; perms.forEach(perm => { const vol = create({}); vol.mkdirSync('/a', { mode: perm }); diff --git a/src/__tests__/volume/openSync.test.ts b/src/__tests__/volume/openSync.test.ts index 48b1c7eeb..8cadcf896 100644 --- a/src/__tests__/volume/openSync.test.ts +++ b/src/__tests__/volume/openSync.test.ts @@ -12,50 +12,50 @@ describe('openSync(path, mode[, flags])', () => { expect(() => { fs.openSync('/foo/baz', 'a'); - }).toThrow(/ENOTDIR/); + }).toThrow(/ENOTDIR/); }); describe('permissions', () => { it('opening for writing throws EACCES without sufficient permissions', () => { - const flags = [ 'a', 'w', 'r+' ]; // append, write, read+write + const flags = ['a', 'w', 'r+']; // append, write, read+write flags.forEach(intent => { const fs = createFs(); fs.chmodSync('/foo', 0o555); // rx across the board expect(() => { fs.openSync('/foo', intent); - }).toThrowError(/EACCES/) + }).toThrowError(/EACCES/); }); }); it('opening for reading throws EACCES without sufficient permissions', () => { - const flags = [ 'a+', 'r', 'w+' ]; // append+read, read, write+read + const flags = ['a+', 'r', 'w+']; // append+read, read, write+read flags.forEach(intent => { const fs = createFs(); fs.chmodSync('/foo', 0o333); // wx across the board expect(() => { - fs.openSync('/foo', intent); + fs.openSync('/foo', intent); }).toThrowError(/EACCES/); }); }); it('opening for anything throws EACCES without sufficient permissions on the containing directory of an existing file', () => { - const flags = [ 'a+', 'r', 'w' ]; // append+read, read, write + const flags = ['a+', 'r', 'w']; // append+read, read, write flags.forEach(intent => { const fs = createFs({ '/foo/bar': 'test' }); fs.chmodSync('/foo', 0o666); // wr across the board expect(() => { - fs.openSync('/foo/bar', intent); + fs.openSync('/foo/bar', intent); }).toThrowError(/EACCES/); }); }); it('opening for anything throws EACCES without sufficient permissions on the containing directory of an non-existent file', () => { - const flags = [ 'a+', 'r', 'w' ]; // append+read, read, write + const flags = ['a+', 'r', 'w']; // append+read, read, write flags.forEach(intent => { const fs = createFs({}); fs.mkdirSync('/foo', { mode: 0o666 }); // wr expect(() => { - fs.openSync('/foo/bar', intent); + fs.openSync('/foo/bar', intent); }).toThrowError(/EACCES/); }); }); diff --git a/src/__tests__/volume/readSync.test.ts b/src/__tests__/volume/readSync.test.ts index 7c3133689..daacef584 100644 --- a/src/__tests__/volume/readSync.test.ts +++ b/src/__tests__/volume/readSync.test.ts @@ -40,8 +40,8 @@ describe('.readSync(fd, buffer, offset, length, position)', () => { }); xit('Negative tests', () => {}); - /* - * No need for permissions tests, because readSync requires a file descriptor, which can only be + /* + * No need for permissions tests, because readSync requires a file descriptor, which can only be * obtained from open or openSync. */ }); diff --git a/src/__tests__/volume/realpathSync.test.ts b/src/__tests__/volume/realpathSync.test.ts index 661795109..4b345819c 100644 --- a/src/__tests__/volume/realpathSync.test.ts +++ b/src/__tests__/volume/realpathSync.test.ts @@ -21,5 +21,5 @@ describe('.realpathSync(...)', () => { expect(() => { vol.realpathSync('/foo/bar'); }).toThrow(/EACCES/); - }); + }); }); diff --git a/src/__tests__/volume/rename.test.ts b/src/__tests__/volume/rename.test.ts index f7c3d8a87..568cf4f53 100644 --- a/src/__tests__/volume/rename.test.ts +++ b/src/__tests__/volume/rename.test.ts @@ -12,7 +12,7 @@ describe('renameSync(fromPath, toPath)', () => { it('gives EACCES when source directory has insufficient permissions', _done => { const perms = [ 0o666, // rw - 0o555 // rx - insufficient because the file will be removed from this directory during renaming + 0o555, // rx - insufficient because the file will be removed from this directory during renaming ]; const done = multitest(_done, perms.length); perms.forEach(perm => { @@ -24,7 +24,7 @@ describe('renameSync(fromPath, toPath)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(x) { + } catch (x) { done(x); } }); @@ -34,7 +34,7 @@ describe('renameSync(fromPath, toPath)', () => { it('gives EACCES when destination directory has insufficient permissions', _done => { const perms = [ 0o666, // rw - 0o555 // rx + 0o555, // rx ]; const done = multitest(_done, perms.length); perms.forEach(perm => { @@ -45,10 +45,10 @@ describe('renameSync(fromPath, toPath)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(x) { + } catch (x) { done(x); } }); }); }); -}); \ No newline at end of file +}); diff --git a/src/__tests__/volume/renameSync.test.ts b/src/__tests__/volume/renameSync.test.ts index 5346cc2d3..721d24d61 100644 --- a/src/__tests__/volume/renameSync.test.ts +++ b/src/__tests__/volume/renameSync.test.ts @@ -75,7 +75,7 @@ describe('renameSync(fromPath, toPath)', () => { it('throws EACCES when source directory has insufficient permissions', () => { const perms = [ 0o666, // rw - 0o555 // rx - insufficient because the file will be removed from this directory during renaming + 0o555, // rx - insufficient because the file will be removed from this directory during renaming ]; perms.forEach(perm => { const vol = create({ '/src/test': 'test' }); @@ -90,7 +90,7 @@ describe('renameSync(fromPath, toPath)', () => { it('throws EACCES when destination directory has insufficient permissions', () => { const perms = [ 0o666, // rw - 0o555 // rx + 0o555, // rx ]; perms.forEach(perm => { const vol = create({ '/src/test': 'test' }); diff --git a/src/__tests__/volume/rmPromise.test.ts b/src/__tests__/volume/rmPromise.test.ts index aa8065433..a8ec5af12 100644 --- a/src/__tests__/volume/rmPromise.test.ts +++ b/src/__tests__/volume/rmPromise.test.ts @@ -141,12 +141,14 @@ describe('rmSync', () => { const perms = [ 0o666, // rw 0o555, // rx - 0o111 // x + 0o111, // x ]; - return Promise.all(perms.map(perm => { - const vol = create({ '/foo/test': 'test' }); - vol.chmodSync('/foo', perm); - return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); - })); + return Promise.all( + perms.map(perm => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', perm); + return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); + }), + ); }); }); diff --git a/src/__tests__/volume/rmSync.test.ts b/src/__tests__/volume/rmSync.test.ts index 57edab3ef..e46a91c53 100644 --- a/src/__tests__/volume/rmSync.test.ts +++ b/src/__tests__/volume/rmSync.test.ts @@ -119,7 +119,7 @@ describe('rmSync', () => { const perms = [ 0o666, // rw 0o555, // rx - 0o111 // x + 0o111, // x ]; perms.forEach(perm => { const vol = create({ '/foo/test': 'test' }); diff --git a/src/__tests__/volume/statSync.test.ts b/src/__tests__/volume/statSync.test.ts index 928182619..5ca757a01 100644 --- a/src/__tests__/volume/statSync.test.ts +++ b/src/__tests__/volume/statSync.test.ts @@ -30,7 +30,7 @@ describe('.statSync(...)', () => { it('throws EACCES when containing directory does not have sufficient permissions', () => { const vol = create({ '/foo/test': 'test' }); vol.chmodSync('/foo', 0o666); // rw - + expect(() => { vol.statSync('/foo/test'); }).toThrowError(/EACCES/); diff --git a/src/__tests__/volume/write.test.ts b/src/__tests__/volume/write.test.ts index fc31a6f1c..21829ec49 100644 --- a/src/__tests__/volume/write.test.ts +++ b/src/__tests__/volume/write.test.ts @@ -18,8 +18,8 @@ describe('write(fs, str, position, encoding, callback)', () => { }); }); - /* - * No need for permissions tests, because write requires a file descriptor, which can only be + /* + * No need for permissions tests, because write requires a file descriptor, which can only be * obtained from open or openSync. */ }); diff --git a/src/__tests__/volume/writeFileSync.test.ts b/src/__tests__/volume/writeFileSync.test.ts index 0341d4af6..8b04742ee 100644 --- a/src/__tests__/volume/writeFileSync.test.ts +++ b/src/__tests__/volume/writeFileSync.test.ts @@ -48,16 +48,16 @@ describe('writeFileSync(path, data[, options])', () => { it('Write throws EACCES without sufficient permissions on containing directory', () => { const perms = [ 0o666, // rw - 0o555 // rx, only when target file does not exist yet - ] + 0o555, // rx, only when target file does not exist yet + ]; perms.forEach(perm => { const vol = create({}); vol.mkdirSync('/foo'); - vol.chmodSync('/foo', perm); + vol.chmodSync('/foo', perm); expect(() => { vol.writeFileSync('/foo/test', 'test'); }).toThrowError(/EACCES/); - }); + }); // If the target file exists, it should not care about the write permission on containing dir const vol = create({ '/foo/test': 'test' }); diff --git a/src/__tests__/volume/writeSync.test.ts b/src/__tests__/volume/writeSync.test.ts index 99d5defc0..3ed63822e 100644 --- a/src/__tests__/volume/writeSync.test.ts +++ b/src/__tests__/volume/writeSync.test.ts @@ -26,8 +26,8 @@ describe('.writeSync(fd, buffer, offset, length, position)', () => { expect(fs.readFileSync('/foo', 'utf8')).toBe('1x3'); }); - /* - * No need for permissions tests, because write requires a file descriptor, which can only be + /* + * No need for permissions tests, because write requires a file descriptor, which can only be * obtained from open or openSync. */ }); diff --git a/src/volume.ts b/src/volume.ts index 557da4d61..e95c48780 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -401,33 +401,50 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.releasedInos.push(node.ino); } - private walk(steps: string[], resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null - private walk(filename: string, resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null - private walk(link: Link, resolveSymlinks: boolean, checkExistence: boolean, checkAccess: boolean, funcName?: string): Link | null private walk( - stepsOrFilenameOrLink: string[] | string | Link, - resolveSymlinks: boolean, - checkExistence: boolean, - checkAccess: boolean, - funcName?: string): Link | null; + steps: string[], + resolveSymlinks: boolean, + checkExistence: boolean, + checkAccess: boolean, + funcName?: string, + ): Link | null; private walk( - stepsOrFilenameOrLink: string[] | string | Link, - resolveSymlinks: boolean = false, - checkExistence: boolean = false, - checkAccess: boolean = false, - funcName?: string): Link | null { - + filename: string, + resolveSymlinks: boolean, + checkExistence: boolean, + checkAccess: boolean, + funcName?: string, + ): Link | null; + private walk( + link: Link, + resolveSymlinks: boolean, + checkExistence: boolean, + checkAccess: boolean, + funcName?: string, + ): Link | null; + private walk( + stepsOrFilenameOrLink: string[] | string | Link, + resolveSymlinks: boolean, + checkExistence: boolean, + checkAccess: boolean, + funcName?: string, + ): Link | null; + private walk( + stepsOrFilenameOrLink: string[] | string | Link, + resolveSymlinks: boolean = false, + checkExistence: boolean = false, + checkAccess: boolean = false, + funcName?: string, + ): Link | null { let steps: string[]; let filename: string; if (stepsOrFilenameOrLink instanceof Link) { steps = stepsOrFilenameOrLink.steps; filename = sep + steps.join(sep); - } - else if (typeof stepsOrFilenameOrLink === 'string') { - steps = filenameToSteps(stepsOrFilenameOrLink) + } else if (typeof stepsOrFilenameOrLink === 'string') { + steps = filenameToSteps(stepsOrFilenameOrLink); filename = stepsOrFilenameOrLink; - } - else { + } else { steps = stepsOrFilenameOrLink; filename = sep + steps.join(sep); } @@ -440,7 +457,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { if (node.isDirectory()) { if (checkAccess && !node.canExecute()) { throw createError(EACCES, funcName, filename); - } + } } else { if (i < steps.length - 1) throw createError(ENOTDIR, funcName, filename); } @@ -449,10 +466,8 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Check existence of current link if (!curr) - if (checkExistence) - throw createError(ENOENT, funcName, filename); - else - return null; + if (checkExistence) throw createError(ENOENT, funcName, filename); + else return null; node = curr?.getNode(); // Resolve symlink @@ -506,7 +521,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private getLinkParentAsDirOrThrow(filenameOrSteps: string | string[], funcName?: string): Link { - const steps: string[] = (filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps)).slice(0, -1); + const steps: string[] = ( + filenameOrSteps instanceof Array ? filenameOrSteps : filenameToSteps(filenameOrSteps) + ).slice(0, -1); const filename: string = sep + steps.join(sep); const link = this.getLinkOrThrow(filename, funcName); if (!link.getNode().isDirectory()) throw createError(ENOTDIR, funcName, filename); @@ -679,8 +696,8 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } // Resolve symlinks. - // - // @TODO: This should be superfluous. This method is only ever called by openFile(), which does its own symlink resolution + // + // @TODO: This should be superfluous. This method is only ever called by openFile(), which does its own symlink resolution // prior to calling. let realLink: Link | null = link; if (resolveSymlinks) realLink = this.getResolvedLinkOrThrow(link.getPath(), 'open'); @@ -730,8 +747,8 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // This is an error, see https://pubs.opengroup.org/onlinepubs/009695399/functions/open.html: // "If O_CREAT and O_EXCL are set, open() shall fail if the file exists." if (link && flagsNum & O_CREAT && flagsNum & O_EXCL) throw createError(EEXIST, 'open', filename); - } catch(err) { - // Try creating a new file, if it does not exist and O_CREAT flag is set. + } catch (err) { + // Try creating a new file, if it does not exist and O_CREAT flag is set. // Note that this will still throw if the ENOENT came from one of the // intermediate directories instead of the file itself. if (err.code === ENOENT && flagsNum & O_CREAT) { @@ -742,16 +759,14 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Check that the place we create the new file is actually a directory and that we are allowed to do so: if (!dirNode.isDirectory()) throw createError(ENOTDIR, 'open', filename); if (!dirNode.canExecute() || !dirNode.canWrite()) throw createError(EACCES, 'open', filename); - + // This is a difference to the original implementation, which would simply not create a file unless modeNum was specified. // However, current Node versions will default to 0o666. modeNum ??= 0o666; - link = this.createLink(dirLink, steps[steps.length - 1], false, modeNum); - } else - throw err; - } + } else throw err; + } if (link) return this.openLink(link, flagsNum, resolveSymlinks); throw createError(ENOENT, 'open', filename); @@ -1142,17 +1157,17 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link1: Link; try { link1 = this.getLinkOrThrow(filename1, 'link'); - } catch(err) { + } catch (err) { // Augment error with filename2 if (err.code) err = createError(err.code, 'link', filename1, filename2); throw err; } - + const dirname2 = pathModule.dirname(filename2); let dir2: Link; try { dir2 = this.getLinkOrThrow(dirname2, 'link'); - } catch(err) { + } catch (err) { // Augment error with filename1 if (err.code) err = createError(err.code, 'link', filename1, filename2); throw err; @@ -1260,10 +1275,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let dirLink; try { dirLink = this.getLinkParentAsDirOrThrow(pathSteps); - } catch(err) { + } catch (err) { // Catch error to populate with the correct fields - getLinkParentAsDirOrThrow won't be aware of the second path - if (err.code) - err = createError(err.code, 'symlink', targetFilename, pathFilename); + if (err.code) err = createError(err.code, 'symlink', targetFilename, pathFilename); throw err; } @@ -1273,7 +1287,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { if (dirLink.getChild(name)) throw createError(EEXIST, 'symlink', targetFilename, pathFilename); // Check permissions on the path where we are creating the symlink. - // Note we're not checking permissions on the target path: It is not an error to create a symlink to a + // Note we're not checking permissions on the target path: It is not an error to create a symlink to a // non-existent or inaccessible target const node = dirLink.getNode(); if (!node.canExecute() || !node.canWrite()) throw createError(EACCES, 'symlink', targetFilename, pathFilename); @@ -1300,7 +1314,8 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { this.wrapAsync(this.symlinkBase, [targetFilename, pathFilename], callback); } - private realpathBase(filename: string, encoding: TEncodingExtended | undefined): TDataOut {debugger + private realpathBase(filename: string, encoding: TEncodingExtended | undefined): TDataOut { + debugger; const realLink = this.getResolvedLinkOrThrow(filename, 'realpath'); return strToEncoding(realLink.getPath() || '/', encoding); @@ -1326,7 +1341,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link: Link; try { link = this.getLinkOrThrow(filename, 'lstat'); - } catch(err) { + } catch (err) { if (err.code === ENOENT && !throwIfNoEntry) return undefined; else throw err; } @@ -1362,7 +1377,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link: Link; try { link = this.getResolvedLinkOrThrow(filename, 'stat'); - } catch(err) { + } catch (err) { if (err.code === ENOENT && !throwIfNoEntry) return undefined; else throw err; } @@ -1417,7 +1432,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link: Link; try { link = this.getResolvedLinkOrThrow(oldPathFilename); - } catch(err) { + } catch (err) { // Augment err with newPathFilename if (err.code) err = createError(err.code, 'rename', oldPathFilename, newPathFilename); throw err; @@ -1429,7 +1444,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let newPathDirLink: Link; try { newPathDirLink = this.getLinkParentAsDirOrThrow(newPathFilename); - } catch(err) { + } catch (err) { // Augment error with oldPathFilename if (err.code) err = createError(err.code, 'rename', oldPathFilename, newPathFilename); throw err; @@ -1444,8 +1459,13 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { // Check we have access and write permissions in both places const oldParentNode: Node = oldLinkParent.getNode(); const newPathDirNode: Node = newPathDirLink.getNode(); - if (!oldParentNode.canExecute() || !oldParentNode.canWrite() || !newPathDirNode.canExecute() || !newPathDirNode.canWrite()) { - throw createError(EACCES, 'rename', oldPathFilename, newPathFilename); + if ( + !oldParentNode.canExecute() || + !oldParentNode.canWrite() || + !newPathDirNode.canExecute() || + !newPathDirNode.canWrite() + ) { + throw createError(EACCES, 'rename', oldPathFilename, newPathFilename); } oldLinkParent.deleteChild(link); @@ -1783,7 +1803,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { curr = this.getResolvedLinkOrThrow(sep + steps.slice(0, i).join(sep), 'mkdir'); // Start creating directories: - for (i; i < steps.length; i++) { + for (i; i < steps.length; i++) { const node = curr.getNode(); if (node.isDirectory()) { @@ -1885,7 +1905,7 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { let link: Link; try { link = this.getResolvedLinkOrThrow(filename, 'stat'); - } catch(err) { + } catch (err) { // Silently ignore missing paths if force option is true if (err.code === ENOENT && options.force) return; else throw err; @@ -1924,7 +1944,9 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private chmodBase(filename: string, modeNum: number, followSymlinks: boolean = true) { - const link = followSymlinks ? this.getResolvedLinkOrThrow(filename, 'chmod') : this.getLinkOrThrow(filename, 'chmod'); + const link = followSymlinks + ? this.getResolvedLinkOrThrow(filename, 'chmod') + : this.getLinkOrThrow(filename, 'chmod'); const node = link.getNode(); node.chmod(modeNum); } From e6b8229d55fb554124f6e961e397d1d5869ad084 Mon Sep 17 00:00:00 2001 From: chris Date: Sun, 15 Sep 2024 18:15:31 +0200 Subject: [PATCH 13/16] test: Improve testing for missing permissions on intermediate directories --- src/__tests__/volume/ReadStream.test.ts | 14 +++++++++ src/__tests__/volume/WriteStream.test.ts | 16 ++++++++++- src/__tests__/volume/appendFile.test.ts | 25 ++++++++++++---- src/__tests__/volume/appendFileSync.test.ts | 9 +++++- src/__tests__/volume/chmodSync.test.ts | 32 +++++++++++++++------ src/__tests__/volume/copyFile.test.ts | 24 +++++++++++++--- src/__tests__/volume/copyFileSync.test.ts | 9 ++++++ src/__tests__/volume/exists.test.ts | 4 +-- src/__tests__/volume/mkdirSync.test.ts | 9 ++++++ src/__tests__/volume/openSync.test.ts | 15 ++++++++-- src/__tests__/volume/readFile.test.ts | 6 ++++ src/__tests__/volume/realpathSync.test.ts | 8 ++++++ src/__tests__/volume/rename.test.ts | 23 ++++++++++++--- src/__tests__/volume/renameSync.test.ts | 9 ++++++ src/__tests__/volume/rmPromise.test.ts | 6 ++++ src/__tests__/volume/rmSync.test.ts | 8 ++++++ src/__tests__/volume/statSync.test.ts | 9 ++++++ src/__tests__/volume/writeFileSync.test.ts | 15 ++++++++-- 18 files changed, 211 insertions(+), 30 deletions(-) diff --git a/src/__tests__/volume/ReadStream.test.ts b/src/__tests__/volume/ReadStream.test.ts index 2ad132337..5d40a7406 100644 --- a/src/__tests__/volume/ReadStream.test.ts +++ b/src/__tests__/volume/ReadStream.test.ts @@ -46,4 +46,18 @@ describe('ReadStream', () => { done(new Error("Expected ReadStream to emit EACCES but it didn't")); }); }); + + it('should emit EACCES error when intermediate directory has insufficient permissions', done => { + const fs = createFs({ '/foo/test': 'test' }); + fs.chmodSync('/', 0o666); // rw + new fs.ReadStream('/foo/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected ReadStream to emit EACCES but it didn't")); + }); + }); }); diff --git a/src/__tests__/volume/WriteStream.test.ts b/src/__tests__/volume/WriteStream.test.ts index d8157188e..02ca7196d 100644 --- a/src/__tests__/volume/WriteStream.test.ts +++ b/src/__tests__/volume/WriteStream.test.ts @@ -48,7 +48,21 @@ describe('WriteStream', () => { }); }); - it('should emit EACCES error for an non-existant file when containing directory has insufficient permissions', done => { + it('should emit EACCES error for when intermediate directory has insufficient permissions', done => { + const fs = createFs({ '/foo/test': 'test' }); + fs.chmodSync('/', 0o666); // rw + new fs.WriteStream('/foo/test') + .on('error', err => { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + }) + .on('open', () => { + done(new Error("Expected WriteStream to emit EACCES but it didn't")); + }); + }); + + it('should emit EACCES error for a non-existent file when containing directory has insufficient permissions', done => { const fs = createFs({}); fs.mkdirSync('/foo', { mode: 0o555 }); // rx new fs.WriteStream('/foo/test') diff --git a/src/__tests__/volume/appendFile.test.ts b/src/__tests__/volume/appendFile.test.ts index 51b452243..2c322c87e 100644 --- a/src/__tests__/volume/appendFile.test.ts +++ b/src/__tests__/volume/appendFile.test.ts @@ -16,7 +16,7 @@ describe('appendFile(file, data[, options], callback)', () => { }); }); - it('Appending gives EACCES without sufficient permissions', done => { + it('Appending gives EACCES without sufficient permissions on the file', done => { const vol = create({ '/foo': 'foo' }); vol.chmodSync('/foo', 0o555); // rx across the board vol.appendFile('/foo', 'bar', err => { @@ -24,8 +24,8 @@ describe('appendFile(file, data[, options], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch (x) { - done(x); + } catch (failure) { + done(failure); } }); }); @@ -45,10 +45,25 @@ describe('appendFile(file, data[, options], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch (x) { - done(x); + } catch (failure) { + done(failure); } }); }); }); + + it('Appending gives EACCES if intermediate directory has insufficient permissions', done => { + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chmodSync('/', 0o666); // rw + vol.appendFile('/foo/test', 'bar', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch (failure) { + done(failure); + } + }); + }); }); diff --git a/src/__tests__/volume/appendFileSync.test.ts b/src/__tests__/volume/appendFileSync.test.ts index 515f829f5..af834e057 100644 --- a/src/__tests__/volume/appendFileSync.test.ts +++ b/src/__tests__/volume/appendFileSync.test.ts @@ -11,7 +11,7 @@ describe('appendFileSync(file, data, options)', () => { vol.appendFileSync('/a', 'c'); expect(vol.readFileSync('/a', 'utf8')).toEqual('bc'); }); - it('Appending throws EACCES without sufficient permissions', () => { + it('Appending throws EACCES without sufficient permissions on the file', () => { const vol = create({ '/foo': 'foo' }); vol.chmodSync('/foo', 0o555); // rx across the board expect(() => { @@ -33,4 +33,11 @@ describe('appendFileSync(file, data, options)', () => { }).toThrowError(/EACCES/); }); }); + it('Appending throws EACCES if intermediate directory has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.appendFileSync('/foo/test', 'bar'); + }).toThrowError(/EACCES/); + }); }); diff --git a/src/__tests__/volume/chmodSync.test.ts b/src/__tests__/volume/chmodSync.test.ts index 03b6b76a2..668acc120 100644 --- a/src/__tests__/volume/chmodSync.test.ts +++ b/src/__tests__/volume/chmodSync.test.ts @@ -29,6 +29,17 @@ describe('chmodSync', () => { }); }); + it('should chmod the target of a symlink, not the symlink itself', () => { + const vol = create({ '/target': 'contents' }); + vol.symlinkSync('/target', '/link'); + const expectedLink = vol.lstatSync('/link').mode; + const expectedTarget = vol.statSync('/target').mode & ~0o777; + vol.chmodSync('/link', 0); + + expect(vol.lstatSync('/link').mode).toEqual(expectedLink); + expect(vol.statSync('/target').mode).toEqual(expectedTarget); + }); + it.skip('should throw EPERM when trying to chmod targets not owned by the uid', () => { const uid = process.getuid() + 1; // Check for directories @@ -47,14 +58,19 @@ describe('chmodSync', () => { }).toThrow(/ENOENT/); }); - it('should chmod the target of a symlink, not the symlink itself', () => { - const vol = create({ '/target': 'contents' }); - vol.symlinkSync('/target', '/link'); - const expectedLink = vol.lstatSync('/link').mode; - const expectedTarget = vol.statSync('/target').mode & ~0o777; - vol.chmodSync('/link', 0); + it('should throw EACCES when containing directory has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o666); // rw + expect(() => { + vol.chmodSync('/foo/test', 0o777); + }).toThrow(/EACCES/); + }); - expect(vol.lstatSync('/link').mode).toEqual(expectedLink); - expect(vol.statSync('/target').mode).toEqual(expectedTarget); + it('should throw EACCES when intermediate directory has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.chmodSync('/foo/test', 0o777); + }).toThrow(/EACCES/); }); }); diff --git a/src/__tests__/volume/copyFile.test.ts b/src/__tests__/volume/copyFile.test.ts index 2594ad236..308d3a1b6 100644 --- a/src/__tests__/volume/copyFile.test.ts +++ b/src/__tests__/volume/copyFile.test.ts @@ -66,13 +66,14 @@ describe('copyFile(src, dest[, flags], callback)', () => { try { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); - } finally { done(); + } catch(failure) { + done(failure); } }); }); - it('copying yields EACCES with insufficient permissions on the destination directory', _done => { + it('copying gives EACCES with insufficient permissions on the destination directory', _done => { const perms = [ 0o555, // rx 0o666, // rw @@ -90,11 +91,26 @@ describe('copyFile(src, dest[, flags], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch (err) { - done(err); + } catch (failure) { + done(failure); } }); }); }); }); + + it('copying gives EACCES with insufficient permissions on an intermediate directory', done => { + const vol = create({ '/foo/test': 'test' }); + vol.mkdirSync('/bar'); + vol.chmodSync('/', 0o666); // rw + vol.copyFile('/foo/test', '/bar/test', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch (failure) { + done(failure); + } + }); + }); }); diff --git a/src/__tests__/volume/copyFileSync.test.ts b/src/__tests__/volume/copyFileSync.test.ts index 00f73fe1c..694422c86 100644 --- a/src/__tests__/volume/copyFileSync.test.ts +++ b/src/__tests__/volume/copyFileSync.test.ts @@ -125,5 +125,14 @@ describe('copyFileSync(src, dest[, flags])', () => { }).toThrowError(/EACCES/); }); }); + it('copying throws EACCES with insufficient permissions an intermediate directory', () => { + const vol = create({ '/foo/test': 'test' }); + vol.mkdirSync('/bar'); + vol.chmodSync('/', 0o666); // rw across the board + expect(() => { + vol.copyFileSync('/foo/test', '/bar/test'); + }).toThrowError(/EACCES/); + }); + }); }); diff --git a/src/__tests__/volume/exists.test.ts b/src/__tests__/volume/exists.test.ts index fe9a75b0b..042d54e25 100644 --- a/src/__tests__/volume/exists.test.ts +++ b/src/__tests__/volume/exists.test.ts @@ -41,8 +41,8 @@ describe('exists(path, callback)', () => { try { expect(exists).toEqual(false); done(); - } catch (err) { - done(err); + } catch (failure) { + done(failure); } }); }); diff --git a/src/__tests__/volume/mkdirSync.test.ts b/src/__tests__/volume/mkdirSync.test.ts index e31b13d83..46d4bf7ca 100644 --- a/src/__tests__/volume/mkdirSync.test.ts +++ b/src/__tests__/volume/mkdirSync.test.ts @@ -141,5 +141,14 @@ describe('mkdirSync', () => { }).toThrow(/EACCES/); }); }); + + it('throws EACCES with insufficient permissions on intermediate directory', () => { + const vol = create({}); + vol.mkdirSync('/a'); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.mkdirSync('/a/b/c', { recursive: true }); + }).toThrow(/EACCES/); + }); }); }); diff --git a/src/__tests__/volume/openSync.test.ts b/src/__tests__/volume/openSync.test.ts index 8cadcf896..73d89b34e 100644 --- a/src/__tests__/volume/openSync.test.ts +++ b/src/__tests__/volume/openSync.test.ts @@ -16,7 +16,7 @@ describe('openSync(path, mode[, flags])', () => { }); describe('permissions', () => { - it('opening for writing throws EACCES without sufficient permissions', () => { + it('opening for writing throws EACCES without sufficient permissions on the file', () => { const flags = ['a', 'w', 'r+']; // append, write, read+write flags.forEach(intent => { const fs = createFs(); @@ -27,7 +27,7 @@ describe('openSync(path, mode[, flags])', () => { }); }); - it('opening for reading throws EACCES without sufficient permissions', () => { + it('opening for reading throws EACCES without sufficient permissions on the file', () => { const flags = ['a+', 'r', 'w+']; // append+read, read, write+read flags.forEach(intent => { const fs = createFs(); @@ -49,6 +49,17 @@ describe('openSync(path, mode[, flags])', () => { }); }); + it('opening for anything throws EACCES without sufficient permissions on an intermediate directory', () => { + const flags = ['a+', 'r', 'w']; // append+read, read, write + flags.forEach(intent => { + const fs = createFs({ '/foo/bar': 'test' }); + fs.chmodSync('/', 0o666); // wr across the board + expect(() => { + fs.openSync('/foo/bar', intent); + }).toThrowError(/EACCES/); + }); + }); + it('opening for anything throws EACCES without sufficient permissions on the containing directory of an non-existent file', () => { const flags = ['a+', 'r', 'w']; // append+read, read, write flags.forEach(intent => { diff --git a/src/__tests__/volume/readFile.test.ts b/src/__tests__/volume/readFile.test.ts index 6c63f5c2d..8721aea53 100644 --- a/src/__tests__/volume/readFile.test.ts +++ b/src/__tests__/volume/readFile.test.ts @@ -26,4 +26,10 @@ describe('.readFile()', () => { fs.chmodSync('/foo', 0o666); // rw return expect(fs.promises.readFile('/foo/bar')).rejects.toThrow(/EACCES/); }); + + it('throws EACCES if intermediate directory has insufficient permissions', async () => { + const { fs } = memfs({ '/foo/bar': 'test' }); + fs.chmodSync('/', 0o666); // rw + return expect(fs.promises.readFile('/foo/bar')).rejects.toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/realpathSync.test.ts b/src/__tests__/volume/realpathSync.test.ts index 4b345819c..db9126b7d 100644 --- a/src/__tests__/volume/realpathSync.test.ts +++ b/src/__tests__/volume/realpathSync.test.ts @@ -22,4 +22,12 @@ describe('.realpathSync(...)', () => { vol.realpathSync('/foo/bar'); }).toThrow(/EACCES/); }); + + it('throws EACCES when an intermediate directory does not have sufficient permissions', () => { + const vol = create({ '/foo/bar': 'bar' }); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.realpathSync('/foo/bar'); + }).toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/rename.test.ts b/src/__tests__/volume/rename.test.ts index 568cf4f53..f5be74d0a 100644 --- a/src/__tests__/volume/rename.test.ts +++ b/src/__tests__/volume/rename.test.ts @@ -24,8 +24,8 @@ describe('renameSync(fromPath, toPath)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch (x) { - done(x); + } catch (failure) { + done(failure); } }); }); @@ -45,10 +45,25 @@ describe('renameSync(fromPath, toPath)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch (x) { - done(x); + } catch (failure) { + done(failure); } }); }); }); + + it('gives EACCES when intermediate directory has insufficient permissions', done => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest'); + vol.chmodSync('/', 0o666); // rw + vol.rename('/src/test', '/dest/fail', err => { + try { + expect(err).toBeInstanceOf(Error); + expect(err).toHaveProperty('code', 'EACCES'); + done(); + } catch (failure) { + done(failure); + } + }); + }); }); diff --git a/src/__tests__/volume/renameSync.test.ts b/src/__tests__/volume/renameSync.test.ts index 721d24d61..90cd37985 100644 --- a/src/__tests__/volume/renameSync.test.ts +++ b/src/__tests__/volume/renameSync.test.ts @@ -100,4 +100,13 @@ describe('renameSync(fromPath, toPath)', () => { }).toThrowError(/EACCES/); }); }); + + it('throws EACCES when intermediate directory has insufficient permissions', () => { + const vol = create({ '/src/test': 'test' }); + vol.mkdirSync('/dest'); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.renameSync('/src/test', '/dest/fail'); + }).toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/rmPromise.test.ts b/src/__tests__/volume/rmPromise.test.ts index a8ec5af12..61f28e068 100644 --- a/src/__tests__/volume/rmPromise.test.ts +++ b/src/__tests__/volume/rmPromise.test.ts @@ -151,4 +151,10 @@ describe('rmSync', () => { }), ); }); + + it('throws EACCES when intermediate directory has insufficient permissions', async () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o666); // rw + return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/rmSync.test.ts b/src/__tests__/volume/rmSync.test.ts index e46a91c53..c5c2b9404 100644 --- a/src/__tests__/volume/rmSync.test.ts +++ b/src/__tests__/volume/rmSync.test.ts @@ -129,4 +129,12 @@ describe('rmSync', () => { }).toThrowError(/EACCES/); }); }); + + it('throws EACCES when intermediate directory has insufficient permissions', async () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o666); // rw + expect(() => { + vol.rmSync('/foo/test'); + }).toThrow(/EACCES/); + }); }); diff --git a/src/__tests__/volume/statSync.test.ts b/src/__tests__/volume/statSync.test.ts index 5ca757a01..c78819985 100644 --- a/src/__tests__/volume/statSync.test.ts +++ b/src/__tests__/volume/statSync.test.ts @@ -40,4 +40,13 @@ describe('.statSync(...)', () => { vol.statSync('/foo/test', { throwIfNoEntry: false }); }).toThrowError(/EACCES/); }); + + it('throws EACCES when intermediate directory does not have sufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/', 0o666); // rw + + expect(() => { + vol.statSync('/foo/test'); + }).toThrowError(/EACCES/); + }); }); diff --git a/src/__tests__/volume/writeFileSync.test.ts b/src/__tests__/volume/writeFileSync.test.ts index 8b04742ee..3615a8a29 100644 --- a/src/__tests__/volume/writeFileSync.test.ts +++ b/src/__tests__/volume/writeFileSync.test.ts @@ -45,6 +45,14 @@ describe('writeFileSync(path, data[, options])', () => { } }); + it('Write throws EACCES if file exists but has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo/test', 0o555); // rx + expect(() => { + vol.writeFileSync('/foo/test', 'test'); + }).toThrowError(/EACCES/); + }); + it('Write throws EACCES without sufficient permissions on containing directory', () => { const perms = [ 0o666, // rw @@ -67,9 +75,10 @@ describe('writeFileSync(path, data[, options])', () => { }).not.toThrowError(); }); - it('Write throws EACCES if file exists but has insufficient permissions', () => { - const vol = create({ '/foo/test': 'test' }); - vol.chmodSync('/foo/test', 0o555); // rx + it('Write throws EACCES without sufficient permissions on intermediate directory', () => { + const vol = create({}); + vol.mkdirSync('/foo'); + vol.chmodSync('/', 0o666); // rw expect(() => { vol.writeFileSync('/foo/test', 'test'); }).toThrowError(/EACCES/); From f80037f5a6f87966e84eb55a7aeffcf71352a4e8 Mon Sep 17 00:00:00 2001 From: chris Date: Sun, 15 Sep 2024 18:30:38 +0200 Subject: [PATCH 14/16] chore: cleanup --- src/node.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/node.ts b/src/node.ts index c5c74119a..8128e364c 100644 --- a/src/node.ts +++ b/src/node.ts @@ -446,24 +446,6 @@ export class Link extends EventEmitter { // this.vol = null; // } - /** - * Walk the tree path and return the `Link` at that location, if any. - * @param steps {string[]} Desired location. - * @param stop {number} Max steps to go into. - * @param i {number} Current step in the `steps` array. - * - * @return {Link|null} - */ - // walk(steps: string[], stop: number = steps.length, i: number = 0): Link | null { - // if (i >= steps.length) return this; - // if (i >= stop) return this; - - // const step = steps[i]; - // const link = this.getChild(step); - // if (!link) return null; - // return link.walk(steps, stop, i + 1); - // } - toJSON() { return { steps: this.steps, From 63172a8d4e7b4e214c5b14046f1712557a4c0a16 Mon Sep 17 00:00:00 2001 From: chris Date: Mon, 16 Sep 2024 15:57:25 +0200 Subject: [PATCH 15/16] style: run prettier --- src/__tests__/volume/chmodSync.test.ts | 2 +- src/__tests__/volume/copyFile.test.ts | 4 ++-- src/__tests__/volume/copyFileSync.test.ts | 1 - src/__tests__/volume/rmPromise.test.ts | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/__tests__/volume/chmodSync.test.ts b/src/__tests__/volume/chmodSync.test.ts index 668acc120..7a36b9d1d 100644 --- a/src/__tests__/volume/chmodSync.test.ts +++ b/src/__tests__/volume/chmodSync.test.ts @@ -66,7 +66,7 @@ describe('chmodSync', () => { }).toThrow(/EACCES/); }); - it('should throw EACCES when intermediate directory has insufficient permissions', () => { + it('should throw EACCES when intermediate directory has insufficient permissions', () => { const vol = create({ '/foo/test': 'test' }); vol.chmodSync('/', 0o666); // rw expect(() => { diff --git a/src/__tests__/volume/copyFile.test.ts b/src/__tests__/volume/copyFile.test.ts index 308d3a1b6..b7d701554 100644 --- a/src/__tests__/volume/copyFile.test.ts +++ b/src/__tests__/volume/copyFile.test.ts @@ -67,7 +67,7 @@ describe('copyFile(src, dest[, flags], callback)', () => { expect(err).toBeInstanceOf(Error); expect(err).toHaveProperty('code', 'EACCES'); done(); - } catch(failure) { + } catch (failure) { done(failure); } }); @@ -111,6 +111,6 @@ describe('copyFile(src, dest[, flags], callback)', () => { } catch (failure) { done(failure); } - }); + }); }); }); diff --git a/src/__tests__/volume/copyFileSync.test.ts b/src/__tests__/volume/copyFileSync.test.ts index 694422c86..e9de89e1e 100644 --- a/src/__tests__/volume/copyFileSync.test.ts +++ b/src/__tests__/volume/copyFileSync.test.ts @@ -133,6 +133,5 @@ describe('copyFileSync(src, dest[, flags])', () => { vol.copyFileSync('/foo/test', '/bar/test'); }).toThrowError(/EACCES/); }); - }); }); diff --git a/src/__tests__/volume/rmPromise.test.ts b/src/__tests__/volume/rmPromise.test.ts index 61f28e068..365fa4d2c 100644 --- a/src/__tests__/volume/rmPromise.test.ts +++ b/src/__tests__/volume/rmPromise.test.ts @@ -155,6 +155,6 @@ describe('rmSync', () => { it('throws EACCES when intermediate directory has insufficient permissions', async () => { const vol = create({ '/foo/test': 'test' }); vol.chmodSync('/foo', 0o666); // rw - return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); + return expect(vol.promises.rm('/foo/test')).rejects.toThrow(/EACCES/); }); }); From 5668c73b02904e594ced5a4e3963227b7dcd6d9e Mon Sep 17 00:00:00 2001 From: chris Date: Tue, 17 Sep 2024 17:12:52 +0200 Subject: [PATCH 16/16] fix: utimes should not require permissions --- src/__tests__/volume/utimesSync.test.ts | 70 +++++++++++++++++++++++++ src/volume.ts | 10 ++-- 2 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/volume/utimesSync.test.ts diff --git a/src/__tests__/volume/utimesSync.test.ts b/src/__tests__/volume/utimesSync.test.ts new file mode 100644 index 000000000..e76f438ec --- /dev/null +++ b/src/__tests__/volume/utimesSync.test.ts @@ -0,0 +1,70 @@ +import { create } from '../util'; + +describe('utimesSync', () => { + it('should be able to utimes files and directories regardless of their permissions', () => { + const perms = [ + 0o777, // rwx + 0o666, // rw + 0o555, // rx + 0o444, // r + 0o333, // wx + 0o222, // w + 0o111, // x + 0o000, // none + ]; + // Check for directories + perms.forEach(perm => { + const vol = create({}); + vol.mkdirSync('/foo', { mode: perm }); + expect(() => { + vol.utimesSync('/foo', 0, 0); + }).not.toThrow(); + }); + // Check for files + perms.forEach(perm => { + const vol = create({ '/foo': 'foo' }); + expect(() => { + vol.utimesSync('/foo', 0, 0); + }).not.toThrow(); + }); + }); + + it('should set atime and mtime on a file', () => { + const vol = create({ '/foo/test': 'test' }); + vol.utimesSync('/foo/test', new Date(1), new Date(2)); + const { atime, mtime } = vol.statSync('/foo/test'); + expect(atime).toEqual(new Date(1)); + expect(mtime).toEqual(new Date(2)); + }); + + it('should set atime and mtime on a directory', () => { + const vol = create({ '/foo/test': 'test' }); + vol.utimesSync('/foo', new Date(1), new Date(2)); + const { atime, mtime } = vol.statSync('/foo'); + expect(atime).toEqual(new Date(1)); + expect(mtime).toEqual(new Date(2)); + }); + + it("should throw ENOENT when target doesn't exist", () => { + const vol = create({}); + expect(() => { + vol.utimesSync('/foo', 0, 0); + }).toThrow(/ENOENT/); + }); + + it('should throw EACCES when containing directory has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/foo', 0o666); // rw + expect(() => { + vol.utimesSync('/foo/test', 0, 0); + }).toThrow(/EACCES/); + }); + + it('should throw EACCES when intermediate directory has insufficient permissions', () => { + const vol = create({ '/foo/test': 'test' }); + vol.chmodSync('/', 0o666); // rw + expect(() => { + vol.utimesSync('/foo/test', 0, 0); + }).toThrow(/EACCES/); + }); +}); diff --git a/src/volume.ts b/src/volume.ts index e95c48780..7aa272be5 100644 --- a/src/volume.ts +++ b/src/volume.ts @@ -1743,12 +1743,10 @@ export class Volume implements FsCallbackApi, FsSynchronousApi { } private utimesBase(filename: string, atime: number, mtime: number) { - const fd = this.openSync(filename, 'r'); - try { - this.futimesBase(fd, atime, mtime); - } finally { - this.closeSync(fd); - } + const link = this.getResolvedLinkOrThrow(filename, 'utimes'); + const node = link.getNode(); + node.atime = new Date(atime * 1000); + node.mtime = new Date(mtime * 1000); } utimesSync(path: PathLike, atime: TTime, mtime: TTime) {