File watcher stops working with malformed workspace file (fix microsoft#153881) (microsoft#154231)

Author: bpasero

src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as parcelWatcher from '@parcel/watcher';
-import { existsSync, unlinkSync } from 'fs';
+import { existsSync, statSync, unlinkSync } from 'fs';
 import { tmpdir } from 'os';
 import { DeferredPromise, RunOnceScheduler, ThrottledWorker } from 'vs/base/common/async';
 import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
@@ -653,7 +653,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
 		}
 	}
 
-	protected normalizeRequests(requests: IRecursiveWatchRequest[]): IRecursiveWatchRequest[] {
+	protected normalizeRequests(requests: IRecursiveWatchRequest[], validatePaths = true): IRecursiveWatchRequest[] {
 		const requestTrie = TernarySearchTree.forPaths<IRecursiveWatchRequest>(!isLinux);
 
 		// Sort requests by path length to have shortest first
@@ -674,16 +674,35 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
 				continue; // path is ignored entirely (via `**` glob exclude)
 			}
 
+			// Check for overlapping requests
 			if (requestTrie.findSubstr(request.path)) {
 				try {
 					const realpath = realpathSync(request.path);
 					if (realpath === request.path) {
 						this.trace(`ignoring a path for watching who's parent is already watched: ${request.path}`);
 
-						continue; // path is not a symbolic link or similar
+						continue;
 					}
 				} catch (error) {
-					continue; // invalid path - ignore from watching
+					this.trace(`ignoring a path for watching who's realpath failed to resolve: ${request.path} (error: ${error})`);
+
+					continue;
+				}
+			}
+
+			// Check for invalid paths
+			if (validatePaths) {
+				try {
+					const stat = statSync(request.path);
+					if (!stat.isDirectory()) {
+						this.trace(`ignoring a path for watching that is a file and not a folder: ${request.path}`);
+
+						continue;
+					}
+				} catch (error) {
+					this.trace(`ignoring a path for watching who's stat info failed to resolve: ${request.path} (error: ${error})`);
+
+					continue;
 				}
 			}
 

src/vs/platform/files/test/node/parcelWatcher.integrationTest.ts

@@ -33,7 +33,7 @@ import { ltrim } from 'vs/base/common/strings';
 				return { path, excludes, recursive: true };
 			});
 
-			return this.normalizeRequests(requests).map(request => request.path);
+			return this.normalizeRequests(requests, false /* validate paths skipped for tests */).map(request => request.path);
 		}
 
 		override async watch(requests: IRecursiveWatchRequest[]): Promise<void> {
@@ -155,7 +155,7 @@ import { ltrim } from 'vs/base/common/strings';
 	}
 
 	test('basics', async function () {
-		await watcher.watch([{ path: testDir, excludes: [], recursive: true }]); //
+		await watcher.watch([{ path: testDir, excludes: [], recursive: true }]);
 
 		// New file
 		const newFilePath = join(testDir, 'deep', 'newFile.txt');
@@ -430,6 +430,16 @@ import { ltrim } from 'vs/base/common/strings';
 		await changeFuture;
 	});
 
+	test('invalid path does not crash watcher', async function () {
+		await watcher.watch([
+			{ path: testDir, excludes: [], recursive: true },
+			{ path: join(testDir, 'invalid-folder'), excludes: [], recursive: true },
+			{ path: __filename, excludes: [], recursive: true }
+		]);
+
+		return basicCrudTest(join(testDir, 'deep', 'newFile.txt'));
+	});
+
 	test('subsequent watch updates watchers (excludes)', async function () {
 		await watcher.watch([{ path: testDir, excludes: [realpathSync(testDir)], recursive: true }]);
 		await watcher.watch([{ path: testDir, excludes: [], recursive: true }]);