From d52c6e3bf8cd8a96658964721dfc8bf4076acceb Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 12:02:19 -0700 Subject: [PATCH 1/9] Turn off shell by default --- vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 4f78fb0176..c5e574ffb2 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -440,7 +440,7 @@ ${stderr}`)); options.cwd ??= path.resolve(__dirname); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - options.shell ??= true; + options.shell ??= false; // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access options.encoding = 'utf8'; @@ -455,7 +455,7 @@ ${stderr}`)); else { options = { - cwd: path.resolve(__dirname), shell: true, encoding: 'utf8', env: + cwd: path.resolve(__dirname), shell: false, encoding: 'utf8', env: { ...process.env, DOTNET_CLI_UI_LANGUAGE: 'en-US', DOTNET_NOLOGO: 'true' } }; } From 4418f00744af13ab9fe9a89c9a4625ec1ed5ba39 Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 13:06:46 -0700 Subject: [PATCH 2/9] only disable shell on win --- .../src/Utils/CommandExecutor.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index c5e574ffb2..013207bbcd 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -440,7 +440,11 @@ ${stderr}`)); options.cwd ??= path.resolve(__dirname); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - options.shell ??= false; + options.shell ??= os.platform() === 'win32' ? false : true; + // ^ Windows systemcalls (node.js process library uses process_wrap which uses process.cc which makes system calls) + // Seem to resolve the PATH by default. Unix system calls do not. + // We could further improve Unix performance in the future by re-implementing our own PATH resolution. + // And turning SHELL off by default on Unix as well. // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access options.encoding = 'utf8'; @@ -455,7 +459,7 @@ ${stderr}`)); else { options = { - cwd: path.resolve(__dirname), shell: false, encoding: 'utf8', env: + cwd: path.resolve(__dirname), shell: os.platform() === 'win32' ? false : true;, encoding: 'utf8', env: { ...process.env, DOTNET_CLI_UI_LANGUAGE: 'en-US', DOTNET_NOLOGO: 'true' } }; } From 3a31ed1522a5cdcd80b39f39d2995dd1486cb9cf Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 13:12:27 -0700 Subject: [PATCH 3/9] Fix build error --- vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 013207bbcd..5c34e35f07 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -442,7 +442,7 @@ ${stderr}`)); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access options.shell ??= os.platform() === 'win32' ? false : true; // ^ Windows systemcalls (node.js process library uses process_wrap which uses process.cc which makes system calls) - // Seem to resolve the PATH by default. Unix system calls do not. + // Windows seems to resolve the PATH by default in a system call. Unix system calls do not. // We could further improve Unix performance in the future by re-implementing our own PATH resolution. // And turning SHELL off by default on Unix as well. @@ -459,7 +459,7 @@ ${stderr}`)); else { options = { - cwd: path.resolve(__dirname), shell: os.platform() === 'win32' ? false : true;, encoding: 'utf8', env: + cwd: path.resolve(__dirname), shell: os.platform() === 'win32' ? false : true, encoding: 'utf8', env: { ...process.env, DOTNET_CLI_UI_LANGUAGE: 'en-US', DOTNET_NOLOGO: 'true' } }; } From 88d78e40fd6bc6193821e83da7615d8259e1049c Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 13:59:10 -0700 Subject: [PATCH 4/9] use shell where it is required some of these are not necessary bc they should only run on unix but you never know --- vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts | 1 + vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts | 2 +- vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 5c34e35f07..8dc658f540 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -654,6 +654,7 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`), try { const shellEditResponse = (await this.execute(setShellVariable, null, false)).status; + const shellEditResponse = (await this.execute(setShellVariable, { shell: true }, false)).status; environmentEditExitCode += Number(shellEditResponse[0]); const systemEditResponse = (await this.execute(setSystemVariable, null, false)).status environmentEditExitCode += Number(systemEditResponse[0]); diff --git a/vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts b/vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts index 91bef85c37..71bec8dd9f 100644 --- a/vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts +++ b/vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts @@ -318,7 +318,7 @@ export class FileUtilities extends IFileUtilities { try { - const commandResult = await executor.execute(CommandExecutor.makeCommand('id', ['-u']), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false); + const commandResult = await executor.execute(CommandExecutor.makeCommand('id', ['-u']), { shell: true, dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false); return commandResult.status === '0'; } catch (error: any) diff --git a/vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts b/vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts index ce33c41006..2ac4fcbae1 100644 --- a/vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts +++ b/vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts @@ -49,7 +49,7 @@ export async function isRunningUnderWSL(acquisitionContext: IAcquisitionWorkerCo const command = CommandExecutor.makeCommand('grep', ['-i', 'Microsoft', '/proc/version']); executor ??= new CommandExecutor(acquisitionContext, utilityContext); - const commandResult = await executor.execute(command, {}, false); + const commandResult = await executor.execute(command, { shell: true }, false); if (!commandResult || !commandResult.stdout) { From 08573652319195c21fea4418bb903e727334563c Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 14:05:57 -0700 Subject: [PATCH 5/9] fix conflict --- vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 8dc658f540..0df8943c33 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -653,7 +653,6 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`), const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `"${value}"`]); try { - const shellEditResponse = (await this.execute(setShellVariable, null, false)).status; const shellEditResponse = (await this.execute(setShellVariable, { shell: true }, false)).status; environmentEditExitCode += Number(shellEditResponse[0]); const systemEditResponse = (await this.execute(setSystemVariable, null, false)).status From 5f0a2fa42415e66bf94b56cda9ed0228000a2e6c Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Fri, 13 Jun 2025 15:34:58 -0700 Subject: [PATCH 6/9] WIP Use Spawn over exec only spawn can use shell = false spawn uses the direct string. escaping with " " causes it to not find the executable. need to confirm with weird path names that caused issues before but program files seems to work. setx "" parsing also no longer functions properly. noticed that --list-sdks --arch is missing the arch value so fixed that as well --- .../Acquisition/DotnetConditionValidator.ts | 8 +- .../DotnetCoreAcquisitionWorker.ts | 5 +- .../src/Acquisition/WinMacGlobalInstaller.ts | 10 +-- .../src/Utils/CommandExecutor.ts | 84 ++++++++++++++----- 4 files changed, 73 insertions(+), 34 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts b/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts index d87befd6e7..e7d704124d 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts @@ -87,7 +87,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator return ''; } - const infoCommand = CommandExecutor.makeCommand(`"${hostPath}"`, ['--info']); + const infoCommand = CommandExecutor.makeCommand(`${hostPath}`, ['--info']); const envWithForceEnglish = process.env; envWithForceEnglish.DOTNET_CLI_UI_LANGUAGE = 'en-US'; // System may not have english installed, but CDK already calls this without issue -- the .NET SDK language invocation is also wrapped by a runtime library and natively includes english assets @@ -123,7 +123,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement return []; } - const findSDKsCommand = CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-sdks', '--arch', requestedArchitecture]); + const findSDKsCommand = CommandExecutor.makeCommand(`${existingPath}`, ['--list-sdks', '--arch', requestedArchitecture]); const sdkInfo = await (this.executor!).execute(findSDKsCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false).then(async (result) => { @@ -162,7 +162,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement // We don't check that the version is 10.0 or later after 2026 when .NET 11 starts rolling out, as It will be slower to check all of the numbers in the output for versions >= 10. const hostMaySupportArchFlag = listDotnetInstallsStdout.includes("10.0") || listDotnetInstallsStdout.includes("11.0") || Date.now() >= new Date('2026-03-01').getTime(); // Use runtimes instead of sdks, as sdks will always have a runtime, and runtime search can be cached across both mode calls. - const findInvalidCommand = CommandExecutor.makeCommand(`"${dotnetExecutablePath}"`, ['--list-runtimes', '--arch', 'invalid-arch']); + const findInvalidCommand = CommandExecutor.makeCommand(`${dotnetExecutablePath}`, ['--list-runtimes', '--arch', 'invalid-arch']); const hostSupportsArchFlag = hostMaySupportArchFlag ? (await (this.executor!).execute(findInvalidCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false)).status !== '0' : false; return hostSupportsArchFlag; } @@ -311,7 +311,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement } requestedArchitecture ??= DotnetCoreAcquisitionWorker.defaultArchitecture() - const findRuntimesCommand = CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-runtimes', '--arch', requestedArchitecture]); + const findRuntimesCommand = CommandExecutor.makeCommand(`${existingPath}`, ['--list-runtimes', '--arch', requestedArchitecture]); const windowsDesktopString = 'Microsoft.WindowsDesktop.App'; const aspnetCoreString = 'Microsoft.AspNetCore.App'; diff --git a/vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts b/vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts index 26017dba0d..ae1d30465c 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts @@ -35,8 +35,7 @@ import DotnetWSLSecurityError, EventBasedError, EventCancellationError, - SuppressedAcquisitionError, - UtilizingExistingInstallPromise + SuppressedAcquisitionError } from '../EventStream/EventStreamEvents'; import * as versionUtils from './VersionUtilities'; @@ -360,7 +359,7 @@ To keep your .NET version up to date, please reconnect to the internet at your s private async sdkIsFound(context: IAcquisitionWorkerContext, version: string): Promise { const executor = new CommandExecutor(context, this.utilityContext); - const listSDKsCommand = CommandExecutor.makeCommand('dotnet', ['--list-sdks', '--arch']); + const listSDKsCommand = CommandExecutor.makeCommand('dotnet', ['--list-sdks', '--arch', context.acquisitionContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture()], false); const result = await executor.execute(listSDKsCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false); if (result.status !== '0') diff --git a/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts b/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts index fdf6dd413b..7ad29ba951 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts @@ -262,7 +262,7 @@ This report should be made at https://github.com/dotnet/vscode-dotnet-runtime/is { if (os.platform() === 'win32') // Windows does not have chmod +x ability with nodejs. { - const permissionsCommand = CommandExecutor.makeCommand('icacls', [`"${installerPath}"`, '/grant:r', `"%username%":F`, '/t', '/c']); + const permissionsCommand = CommandExecutor.makeCommand('icacls', [`${installerPath}`, '/grant:r', `%username%:F`, '/t', '/c']); const commandRes = await this.commandRunner.execute(permissionsCommand, {}, false); if (commandRes.stderr !== '') { @@ -334,7 +334,7 @@ Please try again, or download the .NET Installer file yourself. You may also rep else if (error?.message?.includes('EPERM')) { this.acquisitionContext.eventStream.post(new DotnetFileIntegrityFailureEvent(`The file ${installerFile} did not have the correct permissions scope to be assessed. -Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`"${installerFile}"`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`)); +Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`${installerFile}`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`)); } return ask ? this.userChoosesToContinueWithInvalidHash() : false; } @@ -436,7 +436,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s } else if (workingCommand.commandRoot === 'command') { - workingCommand = CommandExecutor.makeCommand(`open`, [`-W`, `"${path.resolve(installerPath)}"`]); + workingCommand = CommandExecutor.makeCommand(`open`, [`-W`, `${path.resolve(installerPath)}`]); } this.acquisitionContext.eventStream.post(new NetInstallerBeginExecutionEvent(`The OS X .NET Installer has been launched.`)); @@ -450,7 +450,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s } else { - const command = `"${path.resolve(installerPath)}"`; + const command = `${path.resolve(installerPath)}`; let commandOptions: string[] = []; if (await this.file.isElevated(this.acquisitionContext, this.utilityContext)) { @@ -478,7 +478,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s // Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access error.message = `The installer does not have permission to execute. Please try running as an administrator. ${error?.message}. -Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`"${installerPath}"`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`; +Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`${installerPath}`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`; } // Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 0df8943c33..77b7e9a73e 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -6,7 +6,6 @@ import * as proc from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; -import { promisify } from 'util'; import open = require('open'); import path = require('path'); @@ -519,39 +518,79 @@ ${stderr}`)); } const commandStartTime = process.hrtime.bigint(); - const commandResult: CommandExecutorResult = await promisify(proc.exec)(fullCommandString, options).then( - fulfilled => + const commandResult: CommandExecutorResult = await this.asyncSpawn(command, options, terminalFailure); + this.logCommandResult(commandResult, fullCommandString, commandStartTime, command.commandRoot); + + if (useCache) + { + LocalMemoryCacheSingleton.getInstance().putCommand({ command, options }, commandResult, this.context); + } + return commandResult; + } + } + + private asyncSpawn(commandToExecute: CommandExecutorCommand, options: any, terminalFailure: boolean): Promise + { + return new Promise((resolve, reject) => + { + const child = proc.spawn(commandToExecute.commandRoot, commandToExecute.commandParts, { + ...options, + stdio: ['pipe', 'pipe', 'pipe'], // Capture stdout and stderr + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data: any) => + { + stdout += data.toString(); + }); + + child.stderr.on('data', (data: any) => + { + stderr += data.toString(); + }); + + child.on('close', (code: any) => + { + if (code === 0) + { + return resolve({ stdout: stdout, stderr: stderr, status: '0' }); + } + else { - // If any status besides 0 is returned, an error is thrown by nodejs - return { stdout: fulfilled.stdout?.toString() ?? '', stderr: fulfilled.stderr?.toString() ?? '', status: '0' }; - }, - rejected => // Rejected object: error type with stderr : Buffer, stdout : Buffer ... with .code (number) or .signal (string)} - { // see https://nodejs.org/api/child_process.html#child_processexeccommand-options-callback - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const result = { stdout: rejected?.stdout?.toString() ?? '', stderr: rejected?.stderr?.toString() ?? '', status: rejected?.code?.toString() ?? rejected?.signal?.toString() ?? '' }; + const result = { stdout: stdout, stderr: stderr, status: code?.toString() ?? '1' }; if (terminalFailure) { - this.logCommandResult(result, fullCommandString, commandStartTime, command.commandRoot); - throw rejected ?? new Error(`Spawning ${fullCommandString} failed with an unspecified error.`); // according to nodejs spec, this should never be possible + this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot); + return reject(new CommandExecutionNonZeroExitFailure(new EventBasedError('CommandExecutionNonZeroExitFailure', ''), null)); } else { // signal is a string or obj, code is a number - return result; + return resolve(result); } } - ); + }); // We don't need to handle exit, close is when all exits have been called. (stderr, stdout) - this.logCommandResult(commandResult, fullCommandString, commandStartTime, command.commandRoot); - - if (useCache) + child.on('error', (error) => { - LocalMemoryCacheSingleton.getInstance().putCommand({ command, options }, commandResult, this.context); - } - return commandResult; - } + const result = { stdout: stdout, stderr: stderr, status: error.name?.toString() ?? '' }; + if (terminalFailure) + { + this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot); + return reject(new CommandExecutionNonZeroExitFailure(new EventBasedError('CommandExecutionNonZeroExitFailure', ''), null)); + } + else + { + // signal is a string or obj, code is a number + return resolve(result); + } + }); + }); } + private logCommandResult(commandResult: CommandExecutorResult, fullCommandStringForTelemetryOnly: string, commandStartTime: bigint, commandRoot: string) { const durationMs = (Number(process.hrtime.bigint() - commandStartTime) / 1000000).toFixed(2); @@ -650,7 +689,8 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`), if (os.platform() === 'win32') { const setShellVariable = CommandExecutor.makeCommand(`set`, [`${variable}=${value}`]); - const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `"${value}"`]); + // "" does not escape but instead provides it as a string + const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `${value}`]); try { const shellEditResponse = (await this.execute(setShellVariable, { shell: true }, false)).status; From 60228e119ae30f2c08f94803fd1da238ab636ade Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Mon, 16 Jun 2025 14:13:06 -0700 Subject: [PATCH 7/9] ignore stdin I don't believe we ever use that. --- vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 77b7e9a73e..32f7859020 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -535,7 +535,7 @@ ${stderr}`)); { const child = proc.spawn(commandToExecute.commandRoot, commandToExecute.commandParts, { ...options, - stdio: ['pipe', 'pipe', 'pipe'], // Capture stdout and stderr + stdio: ['ignore', 'pipe', 'pipe'], // Ignore stdin (we won't send any input), Capture stdout and stderr }); let stdout = ''; @@ -689,7 +689,6 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`), if (os.platform() === 'win32') { const setShellVariable = CommandExecutor.makeCommand(`set`, [`${variable}=${value}`]); - // "" does not escape but instead provides it as a string const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `${value}`]); try { From 4b099a26495a10506c83486020567ab21c71ca49 Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Mon, 16 Jun 2025 14:25:18 -0700 Subject: [PATCH 8/9] Fix some test bugs, ignore stdin --- .../functional/DotnetCoreAcquisitionExtension.test.ts | 2 +- .../src/Acquisition/WinMacGlobalInstaller.ts | 4 ++-- .../src/Utils/CommandExecutor.ts | 6 +++--- .../src/test/unit/WinMacGlobalInstaller.test.ts | 8 +++++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts b/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts index f573b46620..5d75e67bc9 100644 --- a/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts +++ b/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts @@ -138,7 +138,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function () context.architecture = arch; } const result = await vscode.commands.executeCommand('dotnet.acquire', context); - assert.exists(result, 'Command results a result'); + assert.exists(result, 'Acquire Command results a result'); assert.exists(result!.dotnetPath, 'The return type of the local runtime install command has a .dotnetPath property'); assert.isTrue(fs.existsSync(result!.dotnetPath), 'The returned path of .net does exist'); assert.include(result!.dotnetPath, '.dotnet', '.dotnet is in the path of the local runtime install'); diff --git a/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts b/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts index 7ad29ba951..0c0de3adbd 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts @@ -454,11 +454,11 @@ Please correct your PATH variable or make sure the 'open' utility is installed s let commandOptions: string[] = []; if (await this.file.isElevated(this.acquisitionContext, this.utilityContext)) { - commandOptions = [`/quiet`, `/install`, `/norestart`]; + commandOptions = [`/install`, `/quiet`, `/norestart`]; } else { - commandOptions = [`/passive`, `/install`, `/norestart`] + commandOptions = [`/install`, `/passive`, `/norestart`] } this.acquisitionContext.eventStream.post(new NetInstallerBeginExecutionEvent(`The Windows .NET Installer has been launched.`)); diff --git a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts index 32f7859020..d7a855d99e 100644 --- a/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts +++ b/vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts @@ -555,11 +555,11 @@ ${stderr}`)); { if (code === 0) { - return resolve({ stdout: stdout, stderr: stderr, status: '0' }); + return resolve({ stdout, stderr, status: '0' }); } else { - const result = { stdout: stdout, stderr: stderr, status: code?.toString() ?? '1' }; + const result = { stdout, stderr, status: code?.toString() ?? '1' }; if (terminalFailure) { this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot); @@ -575,7 +575,7 @@ ${stderr}`)); child.on('error', (error) => { - const result = { stdout: stdout, stderr: stderr, status: error.name?.toString() ?? '' }; + const result = { stdout, stderr, status: error.name?.toString() ?? '' }; if (terminalFailure) { this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot); diff --git a/vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts b/vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts index 76c26efc9e..6696f3018a 100644 --- a/vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts +++ b/vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts @@ -112,17 +112,19 @@ suite('Windows & Mac Global Installer Tests', function () { assert.isTrue(mockExecutor.attemptedCommand.startsWith('open'), `It ran the right mac command, open. Command found: ${mockExecutor.attemptedCommand}`); assert.isTrue(mockExecutor.attemptedCommand.includes('-W'), 'It used the -W flag'); - assert.isTrue(mockExecutor.attemptedCommand.includes('"'), 'It put the installer in quotes for username with space in it'); + const executablePath = mockExecutor.attemptedCommand.split('-W')[1].trim(); + assert.isTrue(fs.existsSync(executablePath), 'It used a full path of an executable that exists'); } else if (os.platform() === 'win32') { - const returnedPath = mockExecutor.attemptedCommand.split(' ')[0].slice(1, -1); + const returnedPath = mockExecutor.attemptedCommand.split('/install')[0].trim(); assert.isTrue(fs.existsSync(returnedPath), `It ran a command to an executable that exists: ${returnedPath}`); - assert.isTrue(mockExecutor.attemptedCommand.includes('"'), 'It put the installer in quotes for username with space in it'); + assert.isTrue(mockExecutor.attemptedCommand.includes('.exe'), 'It has the .exe in the path of the installer, and did not skip / split incorrectly via a path space'); if (await new FileUtilities().isElevated(mockSdkContext, utilContext)) { assert.include(mockExecutor.attemptedCommand, ' /quiet /install /norestart', 'It ran under the hood if it had privileges already'); } + } mockSdkContext // Rerun install to clean it up. From 0e36e186b9cb6427c50be959a63c114dab5d6d1a Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Tue, 1 Jul 2025 14:01:09 -0700 Subject: [PATCH 9/9] Fix merge --- .../src/Acquisition/DotnetPathFinder.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts b/vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts index fba1577e75..619207ebdd 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts @@ -411,7 +411,7 @@ export class DotnetPathFinder implements IDotnetPathFinder // /usr/local/bin/dotnet becomes /snap/dotnet-sdk/current/dotnet in reality, may have different behavior in shells. if (os.platform() === 'win32') { - LocalMemoryCacheSingleton.getInstance().aliasCommandAsAnotherCommandRoot(`"${truePath}"`, `"${tentativePath}"`, this.workerContext.eventStream); + LocalMemoryCacheSingleton.getInstance().aliasCommandAsAnotherCommandRoot(`${truePath}`, `${tentativePath}`, this.workerContext.eventStream); } } else