Files - deal with dangling symbolic links (fix #90075)

src/vs/base/node/config.ts

@@ -126,8 +126,8 @@ export class ConfigWatcher<T> extends Disposable implements IConfigWatcher<T> {
 	}
 
 	private async handleSymbolicLink(): Promise<void> {
-		const { stat, isSymbolicLink } = await statLink(this._path);
-		if (isSymbolicLink && !stat.isDirectory()) {
+		const { stat, symbolicLink } = await statLink(this._path);
+		if (symbolicLink && !stat.isDirectory()) {
 			const realPath = await realpath(this._path);
 
 			this.watch(realPath, false);

src/vs/base/node/pfs.ts

@@ -178,30 +178,52 @@ export function stat(path: string): Promise<fs.Stats> {
 }
 
 export interface IStatAndLink {
+
+	// The stats of the file. If the file is a symbolic
+	// link, the stats will be of that target file and
+	// not the link itself.
+	// If the file is a symbolic link pointing to a non
+	// existing file, the stat will be of the link and
+	// the `dangling` flag will indicate this.
 	stat: fs.Stats;
-	isSymbolicLink: boolean;
+
+	// Will be provided if the resource is a symbolic link
+	// on disk. Use the `dangling` flag to find out if it
+	// points to a resource that does not exist on disk.
+	symbolicLink?: { dangling: boolean };
 }
 
 export async function statLink(path: string): Promise<IStatAndLink> {
 
 	// First stat the link
-	let linkStat: fs.Stats | undefined;
-	let linkStatError: NodeJS.ErrnoException | undefined;
+	let lstats: fs.Stats | undefined;
 	try {
-		linkStat = await lstat(path);
+		lstats = await lstat(path);
+
+		// Return early if the stat is not a symbolic link at all
+		if (!lstats.isSymbolicLink()) {
+			return { stat: lstats };
+		}
 	} catch (error) {
-		linkStatError = error;
+		/* ignore - use stat() instead */
 	}
 
-	// Then stat the target and return that
-	const isLink = !!(linkStat && linkStat.isSymbolicLink());
-	if (linkStatError || isLink) {
-		const fileStat = await stat(path);
+	// If the stat is a symbolic link or failed to stat, use fs.stat()
+	// which for symbolic links will stat the target they point to
+	try {
+		const stats = await stat(path);
 
-		return { stat: fileStat, isSymbolicLink: isLink };
-	}
+		return { stat: stats, symbolicLink: lstats?.isSymbolicLink() ? { dangling: false } : undefined };
+	} catch (error) {
 
-	return { stat: linkStat!, isSymbolicLink: false };
+		// If the link points to a non-existing file we still want
+		// to return it as result while setting dangling: true flag
+		if (error.code === 'ENOENT' && lstats) {
+			return { stat: lstats, symbolicLink: { dangling: true } };
+		}
+
+		throw error;
+	}
 }
 
 export function lstat(path: string): Promise<fs.Stats> {

src/vs/base/test/node/pfs/pfs.test.ts

@@ -334,7 +334,7 @@ suite('PFS', function () {
 
 	test('stat link', async () => {
 		if (isWindows) {
-			return Promise.resolve(); // Symlinks are not the same on win, and we can not create them programitically without admin privileges
+			return; // Symlinks are not the same on win, and we can not create them programitically without admin privileges
 		}
 
 		const id1 = uuid.generateUuid();
@@ -349,14 +349,38 @@ suite('PFS', function () {
 		fs.symlinkSync(directory, symbolicLink);
 
 		let statAndIsLink = await pfs.statLink(directory);
-		assert.ok(!statAndIsLink!.isSymbolicLink);
+		assert.ok(!statAndIsLink?.symbolicLink);
 
 		statAndIsLink = await pfs.statLink(symbolicLink);
-		assert.ok(statAndIsLink!.isSymbolicLink);
+		assert.ok(statAndIsLink?.symbolicLink);
+		assert.ok(!statAndIsLink?.symbolicLink?.dangling);
 
 		pfs.rimrafSync(directory);
 	});
 
+	test('stat link (non existing target)', async () => {
+		if (isWindows) {
+			return; // Symlinks are not the same on win, and we can not create them programitically without admin privileges
+		}
+
+		const id1 = uuid.generateUuid();
+		const parentDir = path.join(os.tmpdir(), 'vsctests', id1);
+		const directory = path.join(parentDir, 'pfs', id1);
+
+		const id2 = uuid.generateUuid();
+		const symbolicLink = path.join(parentDir, 'pfs', id2);
+
+		await pfs.mkdirp(directory, 493);
+
+		fs.symlinkSync(directory, symbolicLink);
+
+		pfs.rimrafSync(directory);
+
+		const statAndIsLink = await pfs.statLink(symbolicLink);
+		assert.ok(statAndIsLink?.symbolicLink);
+		assert.ok(statAndIsLink?.symbolicLink?.dangling);
+	});
+
 	test('readdir', async () => {
 		if (canNormalize && typeof process.versions['electron'] !== 'undefined' /* needs electron */) {
 			const id = uuid.generateUuid();
@@ -546,7 +570,7 @@ suite('PFS', function () {
 
 	test('writeFile (stream, error handling EACCES)', async () => {
 		if (isLinux) {
-			return Promise.resolve(); // somehow this test fails on Linux in our TFS builds
+			return; // somehow this test fails on Linux in our TFS builds
 		}
 
 		const id = uuid.generateUuid();

src/vs/platform/files/node/diskFileSystemProvider.ts

@@ -76,10 +76,10 @@ export class DiskFileSystemProvider extends Disposable implements
 
 	async stat(resource: URI): Promise<IStat> {
 		try {
-			const { stat, isSymbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly
+			const { stat, symbolicLink } = await statLink(this.toFilePath(resource)); // cannot use fs.stat() here to support links properly
 
 			return {
-				type: this.toType(stat, isSymbolicLink),
+				type: this.toType(stat, symbolicLink),
 				ctime: stat.birthtime.getTime(), // intentionally not using ctime here, we want the creation time
 				mtime: stat.mtime.getTime(),
 				size: stat.size
@@ -98,18 +98,7 @@ export class DiskFileSystemProvider extends Disposable implements
 				try {
 					let type: FileType;
 					if (child.isSymbolicLink()) {
-						try {
-							type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any
-						} catch (error) {
-							if (error.code !== FileSystemProviderErrorCode.FileNotFound) {
-								throw error; // any error that is not file not found is unexpected
-							}
-
-							// a symbolic link can point to a target that does
-							// not exist on the file system. in that case we
-							// still want to return the element as UNKNOWN.
-							type = FileType.SymbolicLink | FileType.Unknown;
-						}
+						type = (await this.stat(joinPath(resource, child.name))).type; // always resolve target the link points to if any
 					} else {
 						type = this.toType(child);
 					}
@@ -126,9 +115,24 @@ export class DiskFileSystemProvider extends Disposable implements
 		}
 	}
 
-	private toType(entry: Stats | Dirent, isSymbolicLink = entry.isSymbolicLink()): FileType {
-		let type = entry.isFile() ? FileType.File : entry.isDirectory() ? FileType.Directory : FileType.Unknown;
-		if (isSymbolicLink) {
+	private toType(entry: Stats | Dirent, symbolicLink?: { dangling: boolean }): FileType {
+
+		// Signal file type by checking for file / directory, except:
+		// - symbolic links pointing to non-existing files are FileType.Unknown
+		// - files that are neither file nor directory are FileType.Unknown
+		let type: FileType;
+		if (symbolicLink?.dangling) {
+			type = FileType.Unknown;
+		} else if (entry.isFile()) {
+			type = FileType.File;
+		} else if (entry.isDirectory()) {
+			type = FileType.Directory;
+		} else {
+			type = FileType.Unknown;
+		}
+
+		// Always signal symbolic link as file type additionally
+		if (symbolicLink) {
 			type |= FileType.SymbolicLink;
 		}
 

src/vs/platform/files/node/watcher/nodejs/watcherService.ts

@@ -35,14 +35,14 @@ export class FileWatcher extends Disposable {
 
 	private async startWatching(): Promise<void> {
 		try {
-			const { stat, isSymbolicLink } = await statLink(this.path);
+			const { stat, symbolicLink } = await statLink(this.path);
 
 			if (this.isDisposed) {
 				return;
 			}
 
 			let pathToWatch = this.path;
-			if (isSymbolicLink) {
+			if (symbolicLink) {
 				try {
 					pathToWatch = await realpath(pathToWatch);
 				} catch (error) {

src/vs/platform/files/test/node/diskFileService.test.ts

@@ -445,8 +445,7 @@ suite('Disk File Service', function () {
 			return; // not reliable on windows
 		}
 
-		const link = URI.file(join(testDir, 'foo'));
-		await symlink(link.fsPath, join(testDir, 'bar'));
+		await symlink(join(testDir, 'foo'), join(testDir, 'bar'));
 
 		const resolved = await service.resolve(URI.file(testDir));
 		assert.equal(resolved.isDirectory, true);
@@ -485,6 +484,52 @@ suite('Disk File Service', function () {
 		assert.equal((<FileOperationError>error).fileOperationResult, FileOperationResult.FILE_NOT_FOUND);
 	});
 
+	test('deleteFile - symbolic link (exists)', async () => {
+		if (isWindows) {
+			return; // not reliable on windows
+		}
+
+		const target = URI.file(join(testDir, 'lorem.txt'));
+		const link = URI.file(join(testDir, 'lorem.txt-linked'));
+		await symlink(target.fsPath, link.fsPath);
+
+		const source = await service.resolve(link);
+
+		let event: FileOperationEvent;
+		disposables.add(service.onAfterOperation(e => event = e));
+
+		await service.del(source.resource);
+
+		assert.equal(existsSync(source.resource.fsPath), false);
+
+		assert.ok(event!);
+		assert.equal(event!.resource.fsPath, link.fsPath);
+		assert.equal(event!.operation, FileOperation.DELETE);
+
+		assert.equal(existsSync(target.fsPath), true); // target the link pointed to is never deleted
+	});
+
+	test('deleteFile - symbolic link (pointing to non-existing file)', async () => {
+		if (isWindows) {
+			return; // not reliable on windows
+		}
+
+		const target = URI.file(join(testDir, 'foo'));
+		const link = URI.file(join(testDir, 'bar'));
+		await symlink(target.fsPath, link.fsPath);
+
+		let event: FileOperationEvent;
+		disposables.add(service.onAfterOperation(e => event = e));
+
+		await service.del(link);
+
+		assert.equal(existsSync(link.fsPath), false);
+
+		assert.ok(event!);
+		assert.equal(event!.resource.fsPath, link.fsPath);
+		assert.equal(event!.operation, FileOperation.DELETE);
+	});
+
 	test('deleteFolder (recursive)', async () => {
 		let event: FileOperationEvent;
 		disposables.add(service.onAfterOperation(e => event = e));