diff --git a/src/noConfigDebugInit.ts b/src/noConfigDebugInit.ts index b0da4f01..2f84ee6b 100644 --- a/src/noConfigDebugInit.ts +++ b/src/noConfigDebugInit.ts @@ -8,6 +8,7 @@ import * as vscode from 'vscode'; import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper"; import { getJavaHome } from "./utility"; +import { buildNoConfigPathAppendValue } from "./pathUtil"; /** * Registers the configuration-less debugging setup for the extension. @@ -91,14 +92,7 @@ export async function registerNoConfigDebug( } const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); - const pathSeparator = process.platform === 'win32' ? ';' : ':'; - - // Check if the current PATH already ends with a path separator to avoid double separators - const currentPath = process.env.PATH || ''; - const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); - const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; - - collection.append('PATH', pathValueToAppend); + collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir)); // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written const fileSystemWatcher = vscode.workspace.createFileSystemWatcher( diff --git a/src/pathUtil.ts b/src/pathUtil.ts new file mode 100644 index 00000000..a9f680b3 --- /dev/null +++ b/src/pathUtil.ts @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +/** + * Builds the value to append to PATH for the noConfigScripts directory. + * + * `vscode.EnvironmentVariableCollection.append()` does literal string + * concatenation, so we always prepend a separator to avoid gluing our + * directory onto the last entry of the user's PATH (see issue #1637). + * + * @param platform defaults to `process.platform`; injectable for tests. + */ +export function buildNoConfigPathAppendValue( + scriptsDir: string, + platform: NodeJS.Platform = process.platform, +): string { + const pathSeparator = platform === 'win32' ? ';' : ':'; + return `${pathSeparator}${scriptsDir}`; +} diff --git a/test/pathUtil.test.ts b/test/pathUtil.test.ts new file mode 100644 index 00000000..9f76c8f2 --- /dev/null +++ b/test/pathUtil.test.ts @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as assert from "assert"; + +import { buildNoConfigPathAppendValue } from "../src/pathUtil"; + +// Regression tests for issue #1637. +suite("buildNoConfigPathAppendValue", () => { + + const winDir = "C:\\Users\\me\\.vscode\\extensions\\vscjava.vscode-java-debug-0.59.0\\bundled\\scripts\\noConfigScripts"; + const posixDir = "/home/me/.vscode/extensions/vscjava.vscode-java-debug-0.59.0/bundled/scripts/noConfigScripts"; + + test("uses ';' as separator on Windows", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.strictEqual(result, `;${winDir}`); + }); + + test("uses ':' as separator on Linux", () => { + const result = buildNoConfigPathAppendValue(posixDir, "linux"); + assert.strictEqual(result, `:${posixDir}`); + }); + + test("uses ':' as separator on macOS", () => { + const result = buildNoConfigPathAppendValue(posixDir, "darwin"); + assert.strictEqual(result, `:${posixDir}`); + }); + + test("always starts with a path separator (Windows)", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.ok(result.startsWith(";")); + }); + + test("always starts with a path separator (POSIX)", () => { + const result = buildNoConfigPathAppendValue(posixDir, "linux"); + assert.ok(result.startsWith(":")); + }); + + test("never collapses scriptsDir into the previous PATH entry on Windows", () => { + // #1637 scenario: last user PATH entry has no trailing separator. + const userPath = "C:\\foo;C:\\Program Files\\jreleaser\\"; + const entries = (userPath + buildNoConfigPathAppendValue(winDir, "win32")).split(";"); + assert.ok(entries.includes("C:\\Program Files\\jreleaser\\")); + assert.ok(entries.includes(winDir)); + }); + + test("never collapses scriptsDir into the previous PATH entry on POSIX", () => { + const userPath = "/usr/bin:/opt/jreleaser/bin"; + const entries = (userPath + buildNoConfigPathAppendValue(posixDir, "linux")).split(":"); + assert.ok(entries.includes("/opt/jreleaser/bin")); + assert.ok(entries.includes(posixDir)); + }); + + test("yields only an empty (harmless) entry when the user's PATH already ends with a separator", () => { + const userPath = "C:\\foo;C:\\bar;"; + const entries = (userPath + buildNoConfigPathAppendValue(winDir, "win32")).split(";"); + assert.ok(entries.includes(winDir)); + assert.ok(!entries.some((e) => e !== winDir && e.endsWith(winDir))); + }); + + test("scriptsDir appears unchanged at the end of the appended value", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.ok(result.endsWith(winDir)); + }); +});