Fix project name validation (#22547)

* Fix project name validation * Add/update tests * Address comments * Fix error
2026-02-16 10:58:30 -05:00 · 2023-03-31 08:46:58 -07:00
parent 6cc5e9a70d
commit 9d8006562d
11 changed files with 124 additions and 153 deletions
--- a/extensions/data-workspace/src/common/constants.ts
+++ b/extensions/data-workspace/src/common/constants.ts
@@ -67,7 +67,7 @@ export const whitespaceFilenameErrorMessage = localize('whitespaceFilenameErrorM
 export const invalidFileCharsErrorMessage = localize('invalidFileCharsErrorMessage', "Invalid file characters");
 export const reservedWindowsFilenameErrorMessage = localize('reservedWindowsFilenameErrorMessage', "This file name is reserved for use by Windows. Choose another name and try again");
 export const reservedValueErrorMessage = localize('reservedValueErrorMessage', "Reserved file name. Choose another name and try again");
-export const trailingWhitespaceErrorMessage = localize('trailingWhitespaceErrorMessage', "File name cannot end with a whitespace");
+export const trailingWhitespaceErrorMessage = localize('trailingWhitespaceErrorMessage', "File name cannot start or end with whitespace");
 export const tooLongFilenameErrorMessage = localize('tooLongFilenameErrorMessage', "File name cannot be over 255 characters");

 //Open Existing Dialog
--- a/extensions/data-workspace/src/common/dataWorkspaceExtension.ts
+++ b/extensions/data-workspace/src/common/dataWorkspaceExtension.ts
@@ -59,7 +59,7 @@ export class DataWorkspaceExtension implements IExtension {
 		return isValidBasename(name);
 	}

-	isValidBasenameErrorMessage(name?: string): string {
+	isValidBasenameErrorMessage(name?: string): string | undefined {
 		return isValidBasenameErrorMessage(name);
 	}

--- a/extensions/data-workspace/src/common/pathUtilsHelper.ts
+++ b/extensions/data-workspace/src/common/pathUtilsHelper.ts
@@ -5,7 +5,6 @@

 import * as constants from './constants';
 import * as os from 'os';
-import * as path from 'path';

 const WINDOWS_INVALID_FILE_CHARS = /[\\/:\*\?"<>\|]/g;
 const UNIX_INVALID_FILE_CHARS = /[\\/]/g;
@@ -49,88 +48,57 @@ export function sanitizeStringForFilename(s: string): string {
 /**
 * Returns true if the string is a valid filename
 * Logic is copied from src\vs\base\common\extpath.ts
- * @param name filename to check
+ * @param fileName filename to check (without file path)
 */
-export function isValidBasename(name?: string): boolean {
-	const invalidFileChars = isWindows ? WINDOWS_INVALID_FILE_CHARS : UNIX_INVALID_FILE_CHARS;
-
-	if (!name) {
-		return false;
+export function isValidBasename(fileName?: string): boolean {
+	if (isValidBasenameErrorMessage(fileName) !== undefined) {
+		return false;	//Return false depicting filename is invalid
+	} else {
+		return true;
 	}

-	if (isWindows && name[name.length - 1] === '.') {
-		return false; // Windows: file cannot end with a "."
-	}

-	let basename = path.parse(name).name;
-	if (!basename || basename.length === 0 || /^\s+$/.test(basename)) {
-		return false; // require a name that is not just whitespace
-	}
-
-	invalidFileChars.lastIndex = 0;
-	if (invalidFileChars.test(basename)) {
-		return false; // check for certain invalid file characters
-	}
-
-	if (isWindows && WINDOWS_FORBIDDEN_NAMES.test(basename)) {
-		return false; // check for certain invalid file names
-	}
-
-	if (basename === '.' || basename === '..') {
-		return false; // check for reserved values
-	}
-
-	if (isWindows && basename.length !== basename.trim().length) {
-		return false; // Windows: file cannot end with a whitespace
-	}
-
-	if (basename.length > 255) {
-		return false; // most file systems do not allow files > 255 length
-	}
-
-	return true;
 }

 /**
- * Returns specific error message if file name is invalid
+ * Returns specific error message if file name is invalid otherwise returns undefined
 * Logic is copied from src\vs\base\common\extpath.ts
- * @param name filename to check
+ * @param fileName filename to check (without file path)
 */
-export function isValidBasenameErrorMessage(name?: string): string {
+export function isValidBasenameErrorMessage(fileName?: string): string | undefined {
 	const invalidFileChars = isWindows ? WINDOWS_INVALID_FILE_CHARS : UNIX_INVALID_FILE_CHARS;
-	if (!name) {
+	if (!fileName) {
 		return constants.undefinedFilenameErrorMessage;
 	}

-	if (isWindows && name[name.length - 1] === '.') {
+	if (isWindows && fileName[fileName.length - 1] === '.') {
 		return constants.filenameEndingIsPeriodErrorMessage; // Windows: file cannot end with a "."
 	}

-	let basename = path.parse(name).name;
-	if (!basename || basename.length === 0 || /^\s+$/.test(basename)) {
+	if (!fileName || fileName.length === 0 || /^\s+$/.test(fileName)) {
 		return constants.whitespaceFilenameErrorMessage; // require a name that is not just whitespace
 	}

 	invalidFileChars.lastIndex = 0;
-	if (invalidFileChars.test(basename)) {
+	if (invalidFileChars.test(fileName)) {
 		return constants.invalidFileCharsErrorMessage; // check for certain invalid file characters
 	}

-	if (isWindows && WINDOWS_FORBIDDEN_NAMES.test(basename)) {
+	if (isWindows && WINDOWS_FORBIDDEN_NAMES.test(fileName)) {
 		return constants.reservedWindowsFilenameErrorMessage; // check for certain invalid file names
 	}

-	if (basename === '.' || basename === '..') {
+	if (fileName === '.' || fileName === '..') {
 		return constants.reservedValueErrorMessage; // check for reserved values
 	}

-	if (isWindows && basename.length !== basename.trim().length) {
-		return constants.trailingWhitespaceErrorMessage; // Windows: file cannot end with a whitespace
+	if (isWindows && fileName.length !== fileName.trim().length) {
+		return constants.trailingWhitespaceErrorMessage; // Windows: file cannot start or end with a whitespace
 	}

-	if (basename.length > 255) {
+	if (fileName.length > 255) {
 		return constants.tooLongFilenameErrorMessage; // most file systems do not allow files > 255 length
 	}

-	return '';
+	return undefined;
 }
--- a/extensions/data-workspace/src/dataworkspace.d.ts
+++ b/extensions/data-workspace/src/dataworkspace.d.ts
@@ -75,11 +75,11 @@ declare module 'dataworkspace' {
 		isValidBasename(name: string | null | undefined): boolean;

 		/**
-		 * Returns specific error message if file name is invalid
+		 * Returns specific error message if file name is invalid otherwise returns undefined
 		 * Logic is copied from src\vs\base\common\extpath.ts
 		 * @param name filename to check
 		 */
-		isValidBasenameErrorMessage(name: string | null | undefined): string;
+		isValidBasenameErrorMessage(name: string | null | undefined): string | undefined;
 	}

 	/**
--- a/extensions/data-workspace/src/dialogs/newProjectDialog.ts
+++ b/extensions/data-workspace/src/dialogs/newProjectDialog.ts
@@ -187,7 +187,7 @@ export class NewProjectDialog extends DialogBase {

 		this.register(projectNameTextBox.onTextChanged(text => {
 			const errorMessage = isValidBasenameErrorMessage(text);
-			if (errorMessage) {
+			if (errorMessage !== undefined) {
 				// Set validation error message if project name is invalid
 				return void projectNameTextBox.updateProperty('validationErrorMessage', errorMessage);
 			} else {
--- a/extensions/data-workspace/src/dialogs/newProjectQuickpick.ts
+++ b/extensions/data-workspace/src/dialogs/newProjectQuickpick.ts
@@ -9,7 +9,7 @@ import * as constants from '../common/constants';
 import { directoryExist, showInfoMessageWithLearnMoreLink } from '../common/utils';
 import { defaultProjectSaveLocation } from '../common/projectLocationHelper';
 import { WorkspaceService } from '../services/workspaceService';
-import { isValidBasename, isValidBasenameErrorMessage } from '../common/pathUtilsHelper';
+import { isValidBasenameErrorMessage } from '../common/pathUtilsHelper';

 /**
 * Create flow for a New Project using only VS Code-native APIs such as QuickPick
@@ -40,7 +40,7 @@ export async function createNewProjectWithQuickpick(workspaceService: WorkspaceS
 		{
 			title: constants.EnterProjectName,
 			validateInput: (value) => {
-				return isValidBasename(value) ? undefined : isValidBasenameErrorMessage(value);
+				return isValidBasenameErrorMessage(value);
 			},
 			ignoreFocusOut: true
 		});
--- a/extensions/data-workspace/src/test/dialogs/pathUtilsHelper.test.ts
+++ b/extensions/data-workspace/src/test/dialogs/pathUtilsHelper.test.ts
@@ -6,7 +6,6 @@
 import * as should from 'should';
 import * as constants from '../../common/constants';
 import * as os from 'os';
-import * as path from 'path';
 import { isValidBasename, isValidBasenameErrorMessage } from '../../common/pathUtilsHelper';

 const isWindows = os.platform() === 'win32';
@@ -14,21 +13,21 @@ const isWindows = os.platform() === 'win32';
 suite('Check for invalid filename tests', function (): void {
 	test('Should determine invalid filenames', async () => {
 		// valid filename
-		should(isValidBasename(formatFileName('ValidName.sqlproj'))).equal(true);
+		should(isValidBasename('ValidName')).equal(true);

 		// invalid for both Windows and non-Windows
 		let invalidNames: string[] = [
-			'	.sqlproj',
-			' .sqlproj',
-			'  	.sqlproj',
-			'..sqlproj',
-			'...sqlproj',
+			'	',
+			' ',
+			'  	',
+			'.',
+			'..',
 			// most file systems do not allow files > 255 length
-			'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.sqlproj'
+			'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
 		];

 		for (let invalidName of invalidNames) {
-			should(isValidBasename(formatFileName(invalidName))).equal(false);
+			should(isValidBasename(invalidName)).equal(false, `InvalidName that failed:${invalidName}`);
 		}

 		should(isValidBasename(undefined)).equal(false);
@@ -39,52 +38,55 @@ suite('Check for invalid filename tests', function (): void {
 	test('Should determine invalid Windows filenames', async () => {
 		let invalidNames: string[] = [
 			// invalid characters only for Windows
-			'?.sqlproj',
-			':.sqlproj',
-			'*.sqlproj',
-			'<.sqlproj',
-			'>.sqlproj',
-			'|.sqlproj',
-			'".sqlproj',
-			// Windows filenames cannot end with a whitespace
-			'test   .sqlproj',
-			'test	.sqlproj'
+			'?',
+			':',
+			'*',
+			'<',
+			'>',
+			'|',
+			'"',
+			'/',
+			'\\',
+			// Windows filenames cannot start or end with a whitespace
+			'test   ',
+			'test	',
+			' test'
 		];

 		for (let invalidName of invalidNames) {
-			should(isValidBasename(formatFileName(invalidName))).equal(isWindows ? false : true);
+			should(isValidBasename(invalidName)).equal(isWindows ? false : true, `InvalidName that failed:${invalidName}`);
 		}
 	});

 	test('Should determine Windows forbidden filenames', async () => {
 		let invalidNames: string[] = [
 			// invalid only for Windows
-			'CON.sqlproj',
-			'PRN.sqlproj',
-			'AUX.sqlproj',
-			'NUL.sqlproj',
-			'COM1.sqlproj',
-			'COM2.sqlproj',
-			'COM3.sqlproj',
-			'COM4.sqlproj',
-			'COM5.sqlproj',
-			'COM6.sqlproj',
-			'COM7.sqlproj',
-			'COM8.sqlproj',
-			'COM9.sqlproj',
-			'LPT1.sqlproj',
-			'LPT2.sqlproj',
-			'LPT3.sqlproj',
-			'LPT4.sqlproj',
-			'LPT5.sqlproj',
-			'LPT6.sqlproj',
-			'LPT7.sqlproj',
-			'LPT8.sqlproj',
-			'LPT9.sqlproj',
+			'CON',
+			'PRN',
+			'AUX',
+			'NUL',
+			'COM1',
+			'COM2',
+			'COM3',
+			'COM4',
+			'COM5',
+			'COM6',
+			'COM7',
+			'COM8',
+			'COM9',
+			'LPT1',
+			'LPT2',
+			'LPT3',
+			'LPT4',
+			'LPT5',
+			'LPT6',
+			'LPT7',
+			'LPT8',
+			'LPT9',
 		];

 		for (let invalidName of invalidNames) {
-			should(isValidBasename(formatFileName(invalidName))).equal(isWindows ? false : true);
+			should(isValidBasename(invalidName)).equal(isWindows ? false : true);
 		}
 	});
 });
@@ -92,76 +94,77 @@ suite('Check for invalid filename tests', function (): void {
 suite('Check for invalid filename error tests', function (): void {
 	test('Should determine invalid filenames', async () => {
 		// valid filename
-		should(isValidBasenameErrorMessage(formatFileName('ValidName.sqlproj'))).equal('');
+		should(isValidBasenameErrorMessage('ValidName')).equal(undefined);

 		// invalid for both Windows and non-Windows
-		should(isValidBasenameErrorMessage(formatFileName('	.sqlproj'))).equal(constants.whitespaceFilenameErrorMessage);
-		should(isValidBasenameErrorMessage(formatFileName(' .sqlproj'))).equal(constants.whitespaceFilenameErrorMessage);
-		should(isValidBasenameErrorMessage(formatFileName('  	.sqlproj'))).equal(constants.whitespaceFilenameErrorMessage);
-		should(isValidBasenameErrorMessage(formatFileName('..sqlproj'))).equal(constants.reservedValueErrorMessage);
-		should(isValidBasenameErrorMessage(formatFileName('...sqlproj'))).equal(constants.reservedValueErrorMessage);
+		should(isValidBasenameErrorMessage('	')).equal(constants.whitespaceFilenameErrorMessage);
+		should(isValidBasenameErrorMessage(' ')).equal(constants.whitespaceFilenameErrorMessage);
+		should(isValidBasenameErrorMessage('  	')).equal(constants.whitespaceFilenameErrorMessage);
+		should(isValidBasenameErrorMessage('.')).equal(constants.filenameEndingIsPeriodErrorMessage);
+		should(isValidBasenameErrorMessage('..')).equal(constants.filenameEndingIsPeriodErrorMessage);
 		should(isValidBasenameErrorMessage(undefined)).equal(constants.undefinedFilenameErrorMessage);
-		should(isValidBasenameErrorMessage('\\')).equal(isWindows ? constants.whitespaceFilenameErrorMessage : constants.invalidFileCharsErrorMessage);
-		should(isValidBasenameErrorMessage('/')).equal(constants.whitespaceFilenameErrorMessage);
+		should(isValidBasenameErrorMessage('\\')).equal(constants.invalidFileCharsErrorMessage);
+		should(isValidBasenameErrorMessage('/')).equal(constants.invalidFileCharsErrorMessage);
+		should(isValidBasenameErrorMessage(' ')).equal(constants.whitespaceFilenameErrorMessage);

 		// most file systems do not allow files > 255 length
-		should(isValidBasenameErrorMessage(formatFileName('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.sqlproj'))).equal(constants.tooLongFilenameErrorMessage);
+		should(isValidBasenameErrorMessage('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')).equal(constants.tooLongFilenameErrorMessage);
 	});

 	test('Should determine invalid Windows filenames', async () => {
 		let invalidNames: string[] = [
 			// invalid characters only for Windows
-			'?.sqlproj',
-			':.sqlproj',
-			'*.sqlproj',
-			'<.sqlproj',
-			'>.sqlproj',
-			'|.sqlproj',
-			'".sqlproj'
+			'?',
+			':',
+			'*',
+			'<',
+			'>',
+			'|',
+			'"',
+			'\\',
+			'/'
 		];

 		for (let invalidName of invalidNames) {
-			should(isValidBasenameErrorMessage(formatFileName(invalidName))).equal(isWindows ? constants.invalidFileCharsErrorMessage : '');
+			should(isValidBasenameErrorMessage(invalidName)).equal(isWindows ? constants.invalidFileCharsErrorMessage : '');
 		}
-		// Windows filenames cannot end with a whitespace
-		should(isValidBasenameErrorMessage(formatFileName('test   .sqlproj'))).equal(isWindows ? constants.trailingWhitespaceErrorMessage : '');
-		should(isValidBasenameErrorMessage(formatFileName('test	.sqlproj'))).equal(isWindows ? constants.trailingWhitespaceErrorMessage : '');
+		// Windows filenames cannot start or end with a whitespace
+		should(isValidBasenameErrorMessage('test   ')).equal(isWindows ? constants.trailingWhitespaceErrorMessage : '');
+		should(isValidBasenameErrorMessage('test	')).equal(isWindows ? constants.trailingWhitespaceErrorMessage : '');
+		should(isValidBasenameErrorMessage(' test')).equal(isWindows ? constants.trailingWhitespaceErrorMessage : '');
 	});

 	test('Should determine Windows forbidden filenames', async () => {
 		let invalidNames: string[] = [
 			// invalid only for Windows
-			'CON.sqlproj',
-			'PRN.sqlproj',
-			'AUX.sqlproj',
-			'NUL.sqlproj',
-			'COM1.sqlproj',
-			'COM2.sqlproj',
-			'COM3.sqlproj',
-			'COM4.sqlproj',
-			'COM5.sqlproj',
-			'COM6.sqlproj',
-			'COM7.sqlproj',
-			'COM8.sqlproj',
-			'COM9.sqlproj',
-			'LPT1.sqlproj',
-			'LPT2.sqlproj',
-			'LPT3.sqlproj',
-			'LPT4.sqlproj',
-			'LPT5.sqlproj',
-			'LPT6.sqlproj',
-			'LPT7.sqlproj',
-			'LPT8.sqlproj',
-			'LPT9.sqlproj',
+			'CON',
+			'PRN',
+			'AUX',
+			'NUL',
+			'COM1',
+			'COM2',
+			'COM3',
+			'COM4',
+			'COM5',
+			'COM6',
+			'COM7',
+			'COM8',
+			'COM9',
+			'LPT1',
+			'LPT2',
+			'LPT3',
+			'LPT4',
+			'LPT5',
+			'LPT6',
+			'LPT7',
+			'LPT8',
+			'LPT9',
 		];

 		for (let invalidName of invalidNames) {
-			should(isValidBasenameErrorMessage(formatFileName(invalidName))).equal(isWindows ? constants.reservedWindowsFilenameErrorMessage : '');
+			should(isValidBasenameErrorMessage(invalidName)).equal(isWindows ? constants.reservedWindowsFilenameErrorMessage : '', `InvalidName that failed:${invalidName}`);
 		}
 	});
 });

-function formatFileName(filename: string): string {
-	return path.join(os.tmpdir(), filename);
-}