From 403c27140536b27b1137d68d493851d360b7c90f Mon Sep 17 00:00:00 2001 From: streamich Date: Wed, 21 Jun 2023 17:41:52 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20improve=20file=20opening?= =?UTF-8?q?=20and=20closing=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/fsa-to-node/FsaNodeCore.ts | 53 +++++++++++++--- src/fsa-to-node/FsaNodeFs.ts | 67 ++++++++++++--------- src/fsa-to-node/__tests__/FsaNodeFs.test.ts | 26 ++++++++ src/fsa-to-node/__tests__/util.test.ts | 4 ++ src/fsa-to-node/util.ts | 1 + 5 files changed, 117 insertions(+), 34 deletions(-) diff --git a/src/fsa-to-node/FsaNodeCore.ts b/src/fsa-to-node/FsaNodeCore.ts index 6da020b08..20aea8e1f 100644 --- a/src/fsa-to-node/FsaNodeCore.ts +++ b/src/fsa-to-node/FsaNodeCore.ts @@ -4,6 +4,7 @@ import { ERRSTR } from '../node/constants'; import { FsaToNodeConstants } from './constants'; import { FsaNodeFsOpenFile } from './FsaNodeFsOpenFile'; import { FLAG } from '../consts/FLAG'; +import * as util from '../node/util'; import type * as fsa from '../fsa/types'; import type * as misc from '../node/types/misc'; import type { FsaNodeSyncAdapter } from './types'; @@ -81,10 +82,10 @@ export class FsaNodeCore { protected async getFileOrDir( path: string[], name: string, - funcName?: string, - create?: boolean, + funcName?: string ): Promise { const dir = await this.getDir(path, false, funcName); + if (!name) return dir; try { const file = await dir.getFileHandle(name); return file; @@ -126,12 +127,11 @@ export class FsaNodeCore { public async __getFileById( id: misc.TFileId, funcName?: string, - create?: boolean, ): Promise { if (typeof id === 'number') return (await this.getFileByFd(id, funcName)).file; const filename = pathToFilename(id); const [folder, name] = pathToLocation(filename); - return await this.getFile(folder, name, funcName, create); + return await this.getFile(folder, name, funcName); } protected async getFileByIdOrCreate(id: misc.TFileId, funcName?: string): Promise { @@ -144,9 +144,41 @@ export class FsaNodeCore { protected async __open(filename: string, flags: number, mode: number): Promise { const [folder, name] = pathToLocation(filename); - const createIfMissing = !!(flags & FLAG.O_CREAT); - const fsaFile = await this.getFile(folder, name, 'open', createIfMissing); - return this.__open2(fsaFile, filename, flags, mode); + const throwIfExists = !!(flags & FLAG.O_EXCL); + if (throwIfExists) { + try { + await this.getFile(folder, name, 'open', false); + throw util.createError('EEXIST', 'writeFile'); + } catch (error) { + const file404 = (error && typeof error === 'object' && (error.code === 'ENOENT' || error.name === 'NotFoundError')); + if (!file404) { + if (error && typeof error === 'object') { + switch (error.name) { + case 'TypeMismatchError': + throw createError('ENOTDIR', 'open', filename); + case 'NotFoundError': + throw createError('ENOENT', 'open', filename); + } + } + throw error; + } + } + } + try { + const createIfMissing = !!(flags & FLAG.O_CREAT); + const fsaFile = await this.getFile(folder, name, 'open', createIfMissing); + return this.__open2(fsaFile, filename, flags, mode); + } catch (error) { + if (error && typeof error === 'object') { + switch (error.name) { + case 'TypeMismatchError': + throw createError('ENOTDIR', 'open', filename); + case 'NotFoundError': + throw createError('ENOENT', 'open', filename); + } + } + throw error; + } } protected __open2( @@ -161,6 +193,13 @@ export class FsaNodeCore { return file; } + protected async __close(fd: number): Promise { + const openFile = await this.getFileByFdAsync(fd, 'close'); + await openFile.close(); + this.fds.delete(fd); + this.releasedFds.push(fd); + }; + protected getFileName(id: misc.TFileId): string { if (typeof id === 'number') { const openFile = this.fds.get(id); diff --git a/src/fsa-to-node/FsaNodeFs.ts b/src/fsa-to-node/FsaNodeFs.ts index 5c3c5ba2d..af11cc7ce 100644 --- a/src/fsa-to-node/FsaNodeFs.ts +++ b/src/fsa-to-node/FsaNodeFs.ts @@ -63,18 +63,8 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono public readonly close: FsCallbackApi['close'] = (fd: number, callback: misc.TCallback): void => { util.validateFd(fd); - this.getFileByFdAsync(fd, 'close') - .then(file => file.close()) - .then( - () => { - this.fds.delete(fd); - this.releasedFds.push(fd); - callback(null); - }, - error => { - callback(error); - }, - ); + this.__close(fd) + .then(() => callback(null), error => callback(error)); }; public readonly read: FsCallbackApi['read'] = ( @@ -115,16 +105,27 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono optHelpers.getReadFileOptions, )(a, b); const flagsNum = util.flagsToNumber(opts.flag); - return this.__getFileById(id, 'readFile') - .then(file => file.getFile()) - .then(file => file.arrayBuffer()) - .then(data => { - const buffer = Buffer.from(data); - callback(null, util.bufferToEncoding(buffer, opts.encoding)); - }) - .catch(error => { - callback(error); - }); + (async () => { + let fd: number = typeof id === 'number' ? id : -1; + const originalFd = fd; + try { + if (fd === -1) { + const filename = util.pathToFilename(id as misc.PathLike); + fd = (await this.__open(filename, flagsNum, 0)).fd; + } + const handle = await this.__getFileById(fd, 'readFile'); + const file = await handle.getFile(); + const buffer = Buffer.from(await file.arrayBuffer()); + return util.bufferToEncoding(buffer, opts.encoding) + } finally { + try { + const idWasFd = typeof originalFd === 'number' && originalFd >= 0; + if (idWasFd) await this.__close(originalFd); + } catch {} + } + })() + .then(data => callback(null, data)) + .catch(error => callback(error)); }; public readonly write: FsCallbackApi['write'] = ( @@ -198,11 +199,23 @@ export class FsaNodeFs extends FsaNodeCore implements FsCallbackApi, FsSynchrono const modeNum = util.modeToNumber(opts.mode); const buf = util.dataToBuffer(data, opts.encoding); (async () => { - const createIfMissing = !!(flagsNum & FLAG.O_CREAT); - const file = await this.__getFileById(id, 'writeFile', createIfMissing); - const writable = await file.createWritable({ keepExistingData: false }); - await writable.write(buf); - await writable.close(); + let fd: number = typeof id === 'number' ? id : -1; + const originalFd = fd; + try { + if (fd === -1) { + const filename = util.pathToFilename(id as misc.PathLike); + fd = (await this.__open(filename, flagsNum, modeNum)).fd; + } + const file = await this.__getFileById(fd, 'writeFile'); + const writable = await file.createWritable({ keepExistingData: false }); + await writable.write(buf); + await writable.close(); + } finally { + try { + const idWasFd = typeof originalFd === 'number' && originalFd >= 0; + if (idWasFd) await this.__close(originalFd); + } catch {} + } })().then( () => cb(null), error => cb(error), diff --git a/src/fsa-to-node/__tests__/FsaNodeFs.test.ts b/src/fsa-to-node/__tests__/FsaNodeFs.test.ts index d35408f7a..e1b0d03e4 100644 --- a/src/fsa-to-node/__tests__/FsaNodeFs.test.ts +++ b/src/fsa-to-node/__tests__/FsaNodeFs.test.ts @@ -5,6 +5,7 @@ import { IDirent, IStats } from '../../node/types/misc'; import { FsaNodeFs } from '../FsaNodeFs'; import { tick, until, of } from 'thingies'; import { onlyOnNode20 } from '../../__tests__/util'; +import {FLAG} from '../../consts/FLAG'; const setup = (json: NestedDirectoryJSON | null = null, mode: 'read' | 'readwrite' = 'readwrite') => { const mfs = memfs({ mountpoint: json }) as IFsWithVolume; @@ -26,6 +27,17 @@ onlyOnNode20('FsaNodeFs', () => { expect(mfs.statSync('/mountpoint/test').isDirectory()).toBe(true); }); + test('can create a sub-folder with trailing slash', async () => { + const { fs, mfs } = setup(); + await new Promise((resolve, reject) => + fs.mkdir('/test/', err => { + if (err) return reject(err); + return resolve(); + }), + ); + expect(mfs.statSync('/mountpoint/test').isDirectory()).toBe(true); + }); + test('throws when creating sub-sub-folder', async () => { const { fs } = setup(); try { @@ -654,6 +666,20 @@ onlyOnNode20('FsaNodeFs', () => { '/mountpoint/f.html': 'test', }); }); + + test('throws "EEXIST", if file already exists and O_EXCL flag set', async () => { + const { fs } = setup({ folder: { file: 'test' }, 'empty-folder': null, 'f.html': 'test' }); + const [, err] = await of(fs.promises.writeFile('/folder/file', 'bar', { flag: 'wx' })); + expect(err).toBeInstanceOf(Error); + expect((err).code).toBe('EEXIST'); + }); + + test('throws "ENOENT", if file does not exist and O_CREAT flag not set', async () => { + const { fs } = setup({ folder: { file: 'test' }, 'empty-folder': null, 'f.html': 'test' }); + const [, err] = await of(fs.promises.writeFile('/folder/file2', 'bar', { flag: FLAG.O_RDWR })); + expect(err).toBeInstanceOf(Error); + expect((err).code).toBe('ENOENT'); + }); }); describe('.read()', () => { diff --git a/src/fsa-to-node/__tests__/util.test.ts b/src/fsa-to-node/__tests__/util.test.ts index 7e065ed14..a43bde594 100644 --- a/src/fsa-to-node/__tests__/util.test.ts +++ b/src/fsa-to-node/__tests__/util.test.ts @@ -13,6 +13,10 @@ describe('pathToLocation()', () => { expect(pathToLocation('scary.exe')).toStrictEqual([[], 'scary.exe']); }); + test('strips trailing slash', () => { + expect(pathToLocation('scary.exe/')).toStrictEqual([[], 'scary.exe']); + }); + test('multiple steps in the path', () => { expect(pathToLocation('/gg/wp/hf/gl.txt')).toStrictEqual([['gg', 'wp', 'hf'], 'gl.txt']); expect(pathToLocation('gg/wp/hf/gl.txt')).toStrictEqual([['gg', 'wp', 'hf'], 'gl.txt']); diff --git a/src/fsa-to-node/util.ts b/src/fsa-to-node/util.ts index 32623ab45..d040888ed 100644 --- a/src/fsa-to-node/util.ts +++ b/src/fsa-to-node/util.ts @@ -4,6 +4,7 @@ import type { FsLocation } from './types'; export const pathToLocation = (path: string): FsLocation => { if (path[0] === FsaToNodeConstants.Separator) path = path.slice(1); + if (path[path.length - 1] === FsaToNodeConstants.Separator) path = path.slice(0, -1); const lastSlashIndex = path.lastIndexOf(FsaToNodeConstants.Separator); if (lastSlashIndex === -1) return [[], path]; const file = path.slice(lastSlashIndex + 1);