Fix #4461 Dacpac: Extract lets me save to invalid file path like 'sd' and never adds .dacpac / .bacpac (#6424)

* check for valid path

* add unit tests

* bump package version

* addressing comments

extensions/dacpac/src/wizard/api/dacFxConfigPage.ts

@@ -12,7 +12,7 @@ import * as path from 'path';
 import { DataTierApplicationWizard } from '../dataTierApplicationWizard';
 import { DacFxDataModel } from './models';
 import { BasePage } from './basePage';
-import { sanitizeStringForFilename } from './utils';
+import { sanitizeStringForFilename, isValidBasename } from './utils';
 
 const localize = nls.loadMessageBundle();
 
@@ -142,9 +142,12 @@ export abstract class DacFxConfigPage extends BasePage {
 	}
 
 	protected async createFileBrowserParts() {
-		this.fileTextBox = this.view.modelBuilder.inputBox().withProperties({
-			required: true
-		}).component();
+		this.fileTextBox = this.view.modelBuilder.inputBox().withValidation(
+			component => isValidBasename(component.value)
+		)
+			.withProperties({
+				required: true
+			}).component();
 
 		this.fileButton = this.view.modelBuilder.button().withProperties({
 			label: '•••',
@@ -164,6 +167,14 @@ export abstract class DacFxConfigPage extends BasePage {
 		// return rootpath of opened folder in file explorer if one is open, otherwise default to user home directory
 		return vscode.workspace.rootPath ? vscode.workspace.rootPath : os.homedir();
 	}
+
+	protected appendFileExtensionIfNeeded() {
+		// make sure filepath ends in proper file extension if it's a valid name
+		if (!this.model.filePath.endsWith(this.fileExtension) && isValidBasename(this.model.filePath)) {
+			this.model.filePath += this.fileExtension;
+			this.fileTextBox.value = this.model.filePath;
+		}
+	}
 }
 
 interface ConnectionDropdownValue extends azdata.CategoryValue {

extensions/dacpac/src/wizard/api/utils.ts

@@ -3,8 +3,12 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 import * as os from 'os';
-const INVALID_FILE_CHARS_Windows = /[\\/:\*\?"<>\|]/g;
-const INVALID_FILE_CHARS = /[\\/]/g;
+import * as path from 'path';
+
+const WINDOWS_INVALID_FILE_CHARS = /[\\/:\*\?"<>\|]/g;
+const UNIX_INVALID_FILE_CHARS = /[\\/]/g;
+const isWindows = os.platform() === 'win32';
+const WINDOWS_FORBIDDEN_NAMES = /^(con|prn|aux|clock\$|nul|lpt[0-9]|com[0-9])$/i;
 
 /**
  * Determines if a given character is a valid filename character
@@ -16,18 +20,17 @@ export function isValidFilenameCharacter(c: string): boolean {
 		return false;
 	}
 	let isWindows = os.platform() === 'win32';
-	INVALID_FILE_CHARS_Windows.lastIndex = 0;
-	INVALID_FILE_CHARS.lastIndex = 0;
-	if (isWindows && INVALID_FILE_CHARS_Windows.test(c)) {
+	WINDOWS_INVALID_FILE_CHARS.lastIndex = 0;
+	UNIX_INVALID_FILE_CHARS.lastIndex = 0;
+	if (isWindows && WINDOWS_INVALID_FILE_CHARS.test(c)) {
 		return false;
-	} else if (!isWindows && INVALID_FILE_CHARS.test(c)) {
+	} else if (!isWindows && UNIX_INVALID_FILE_CHARS.test(c)) {
 		return false;
 	}
 
 	return true;
 }
 
-
 /**
  * Replaces invalid filename characters in a string with underscores
  * @param s The string to be sanitized for a filename
@@ -40,4 +43,49 @@ export function sanitizeStringForFilename(s: string): string {
 	}
 
 	return result;
+}
+
+/**
+ * Returns true if the string is a valid filename
+ * Logic is copied from src\vs\base\common\extpath.ts
+ * @param name filename to check
+ */
+export function isValidBasename(name: string | null | undefined): boolean {
+	const invalidFileChars = isWindows ? WINDOWS_INVALID_FILE_CHARS : UNIX_INVALID_FILE_CHARS;
+
+	if (!name) {
+		return false;
+	}
+
+	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;
 }

extensions/dacpac/src/wizard/dataTierApplicationWizard.ts

@@ -161,22 +161,17 @@ export class DataTierApplicationWizard {
 		});
 
 		this.wizard.onPageChanged(async (event) => {
-			let idx = event.newPage;
-			let page = this.getPage(idx);
-
-			if (page !== undefined) {
-				page.dacFxPage.setupNavigationValidator();
-				page.dacFxPage.onPageEnter();
+			let idxLast = event.lastPage;
+			let lastPage = this.getPage(idxLast);
+			if (lastPage) {
+				lastPage.dacFxPage.onPageLeave();
 			}
 
-			//do onPageLeave for summary page so that GenerateScript button only shows up if upgrading database
-			let idxLast = event.lastPage;
-
-			if (this.isSummaryPage(idxLast)) {
-				let lastPage = this.pages.get('summary');
-				if (lastPage) {
-					lastPage.dacFxPage.onPageLeave();
-				}
+			let idx = event.newPage;
+			let page = this.getPage(idx);
+			if (page) {
+				page.dacFxPage.setupNavigationValidator();
+				page.dacFxPage.onPageEnter();
 			}
 		});
 

extensions/dacpac/src/wizard/pages/exportConfigPage.ts

@@ -52,6 +52,12 @@ export class ExportConfigPage extends DacFxConfigPage {
 		return r1 && r2;
 	}
 
+
+	async onPageLeave(): Promise<boolean> {
+		this.appendFileExtensionIfNeeded();
+		return true;
+	}
+
 	public setupNavigationValidator() {
 		this.instance.registerNavigationValidator(() => {
 			if (this.databaseLoader.loading) {

extensions/dacpac/src/wizard/pages/extractConfigPage.ts

@@ -55,6 +55,11 @@ export class ExtractConfigPage extends DacFxConfigPage {
 		return r1 && r2;
 	}
 
+	async onPageLeave(): Promise<boolean> {
+		this.appendFileExtensionIfNeeded();
+		return true;
+	}
+
 	public setupNavigationValidator() {
 		this.instance.registerNavigationValidator(() => {
 			if (this.databaseLoader.loading) {