Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions src/noConfigDebugInit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
19 changes: 19 additions & 0 deletions src/pathUtil.ts
Original file line number Diff line number Diff line change
@@ -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}`;
}
65 changes: 65 additions & 0 deletions test/pathUtil.test.ts
Original file line number Diff line number Diff line change
@@ -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));
});
});
Loading