Narrowed rename test error code

Fixed `rename` not throwing `EISDIR`/`ENOTDIR` correctly
Change default log level for tests to ERR

Author: james-pre

src/backends/store/fs.ts

@@ -242,9 +242,8 @@ export class StoreFS<T extends Store = Store> extends FileSystem {
 			: decodeDirListing((await tx.get(newDirNode.data)) ?? _throw(withErrno('ENODATA')));
 
 		if (newDirList[_new.base]) {
-			// If it's a file, delete it, if it's a directory, throw a permissions error.
 			const existing = new Inode((await tx.get(newDirList[_new.base])) ?? _throw(withErrno('ENOENT')));
-			if (!existing.toStats().isFile()) throw withErrno('EPERM');
+			if (!existing.toStats().isFile()) throw withErrno('EISDIR');
 
 			await tx.remove(existing.data);
 			await tx.remove(newDirList[_new.base]);
@@ -288,9 +287,8 @@ export class StoreFS<T extends Store = Store> extends FileSystem {
 		const newDirList: typeof oldDirList = sameParent ? oldDirList : decodeDirListing(tx.getSync(newDirNode.data) ?? _throw(withErrno('ENODATA')));
 
 		if (newDirList[_new.base]) {
-			// If it's a file, delete it, if it's a directory, throw a permissions error.
 			const existing = new Inode(tx.getSync(newDirList[_new.base]) ?? _throw(withErrno('ENOENT')));
-			if (!existing.toStats().isFile()) throw withErrno('EPERM');
+			if (!existing.toStats().isFile()) throw withErrno('EISDIR');
 
 			tx.removeSync(existing.data);
 			tx.removeSync(newDirList[_new.base]);

src/vfs/promises.ts

@@ -209,7 +209,6 @@ export class FileHandle implements promises.FileHandle {
 		position: number = this.position
 	): Promise<{ bytesRead: number; buffer: TBuffer }> {
 		if (this.closed) throw UV('EBADF', 'read', this.path);
-
 		if (this.flag & constants.O_WRONLY) throw UV('EBADF', 'read', this.path);
 
 		if (!(this.inode.flags! & InodeFlags.NoAtime)) {
@@ -286,7 +285,8 @@ export class FileHandle implements promises.FileHandle {
 		if (flag & constants.O_WRONLY) throw UV('EBADF', 'read', this.path);
 
 		const { size } = await this.stat();
-		const { buffer: data } = await this._read(new Uint8Array(size), 0, size, 0);
+		const data = new Uint8Array(size);
+		await this._read(data, 0, size, 0);
 		const buffer = Buffer.from(data);
 		return options.encoding ? buffer.toString(options.encoding) : buffer;
 	}
@@ -518,9 +518,18 @@ export async function rename(this: V_Context, oldPath: fs.PathLike, newPath: fs.
 
 	if (src.fs !== dst.fs) throw UV('EXDEV', $ex);
 
-	const stats = await stat.call(this, dirname(oldPath)).catch(rethrow($ex));
+	const parent = (await stat.call(this, dirname(oldPath)).catch(rethrow($ex))) as Stats;
+	const stats = (await stat.call(this, oldPath).catch(rethrow($ex))) as Stats;
+	const newParent = (await stat.call(this, dirname(newPath)).catch(rethrow($ex))) as Stats;
+	const newStats = (await stat.call(this, newPath).catch((e: Exception) => {
+		if (e.code == 'ENOENT') return null;
+		throw setUVMessage(Object.assign(e, $ex));
+	})) as Stats;
+
+	if (checkAccess && (!parent.hasAccess(constants.R_OK, this) || !newParent.hasAccess(constants.W_OK, this))) throw UV('EACCES', $ex);
 
-	if (checkAccess && !stats.hasAccess(constants.W_OK, this)) throw UV('EACCES', $ex);
+	if (newStats && !isDirectory(stats) && isDirectory(newStats)) throw UV('EISDIR', $ex);
+	if (newStats && isDirectory(stats) && !isDirectory(newStats)) throw UV('ENOTDIR', $ex);
 
 	await src.fs.rename(src.path, dst.path).catch(rethrow($ex));
 
@@ -618,6 +627,8 @@ async function _open($: V_Context, path: fs.PathLike, opt: OpenOptions): Promise
 
 		if (!isDirectory(parentStats)) throw UV('ENOTDIR', 'open', dirname(path));
 
+		if (!opt.allowDirectory && mode & constants.S_IFDIR) throw UV('EISDIR', 'open', path);
+
 		const { euid: uid, egid: gid } = $?.credentials ?? defaultContext.credentials;
 
 		const inode = await fs.createFile(resolved, {
@@ -630,17 +641,12 @@ async function _open($: V_Context, path: fs.PathLike, opt: OpenOptions): Promise
 	}
 
 	if (checkAccess && !hasAccess($, stats, flags.toMode(flag))) throw UV('EACCES', $ex);
-
 	if (flag & constants.O_EXCL) throw UV('EEXIST', $ex);
 
 	const handle = new FileHandle($, toFD(new SyncHandle($, path, fs, resolved, flag, stats)));
 
-	/*
-		In a previous implementation, we deleted the file and
-		re-created it. However, this created a race condition if another
-		asynchronous request was trying to read the file, as the file
-		would not exist for a small period of time.
-	*/
+	if (!opt.allowDirectory && mode & constants.S_IFDIR) throw UV('EISDIR', 'open', path);
+
 	if (flag & constants.O_TRUNC) await handle.truncate(0);
 
 	return handle;

src/vfs/sync.ts

@@ -29,14 +29,26 @@ export function renameSync(this: V_Context, oldPath: fs.PathLike, newPath: fs.Pa
 	const oldMount = resolveMount(oldPath, this);
 	const newMount = resolveMount(newPath, this);
 
-	const oldStats = statSync.call<V_Context, Parameters<fs.StatSyncFn>, Stats>(this, dirname(oldPath));
-
 	const $ex = { syscall: 'rename', path: oldPath, dest: newPath };
 
-	if (checkAccess && !oldStats.hasAccess(constants.W_OK, this)) throw UV('EACCES', $ex);
-
 	if (oldMount.fs !== newMount.fs) throw UV('EXDEV', $ex);
 
+	const oldStats = statSync.call<V_Context, Parameters<fs.StatSyncFn>, Stats>(this, oldPath);
+	const oldParent = statSync.call<V_Context, Parameters<fs.StatSyncFn>, Stats>(this, dirname(oldPath));
+	const newParent = statSync.call<V_Context, Parameters<fs.StatSyncFn>, Stats>(this, dirname(newPath));
+	let newStats: Stats | undefined;
+	try {
+		newStats = statSync.call<V_Context, Parameters<fs.StatSyncFn>, Stats>(this, newPath);
+	} catch (e: any) {
+		setUVMessage(Object.assign(e, $ex));
+		if (e.code != 'ENOENT') throw e;
+	}
+
+	if (checkAccess && (!oldParent.hasAccess(constants.R_OK, this) || !newParent.hasAccess(constants.W_OK, this))) throw UV('EACCES', $ex);
+
+	if (newStats && !isDirectory(oldStats) && isDirectory(newStats)) throw UV('EISDIR', $ex);
+	if (newStats && isDirectory(oldStats) && !isDirectory(newStats)) throw UV('ENOTDIR', $ex);
+
 	try {
 		oldMount.fs.renameSync(oldMount.path, newMount.path);
 	} catch (e: any) {
@@ -175,10 +187,10 @@ function _openSync(this: V_Context, path: fs.PathLike, opt: OpenOptions): SyncHa
 
 	const file = new SyncHandle(this, path, fs, resolved, flag, stats);
 
-	if (flag & constants.O_TRUNC) file.truncate(0);
-
 	if (!opt.allowDirectory && stats.mode & constants.S_IFDIR) throw UV('EISDIR', 'open', path);
 
+	if (flag & constants.O_TRUNC) file.truncate(0);
+
 	return file;
 }
 

tests/fs/rename.test.ts

@@ -76,8 +76,8 @@ suite('Rename', () => {
 		await fs.promises.mkdir(dir);
 		await fs.promises.writeFile(file, 'file contents go here');
 
-		assert.rejects(fs.promises.rename(file, dir), { code: /EISDIR|EPERM/ });
-		assert.throws(() => fs.renameSync(file, dir), { code: /EISDIR|EPERM/ });
+		assert.rejects(fs.promises.rename(file, dir), { code: 'EISDIR' });
+		assert.throws(() => fs.renameSync(file, dir), { code: 'EISDIR' });
 	});
 
 	test('rename directory inside itself', async () => {

tests/logs.js

@@ -3,7 +3,7 @@ import { log } from 'kerium';
 export function setupLogs(prefix) {
 	const { ZENFS_LOG_LEVEL } = process.env;
 
-	let level = log.Level.CRIT;
+	let level = log.Level.ERR;
 
 	if (ZENFS_LOG_LEVEL) {
 		const tmp = parseInt(ZENFS_LOG_LEVEL);