diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 3450719c05..182b8e80d9 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -12,6 +12,7 @@ Release with new features and bugfixes: * https://github.com/devonfw/IDEasy/issues/1056[#1056]: Improve MSI license display by generating formatted RTF from AsciiDoc * https://github.com/devonfw/IDEasy/issues/2052[#2052]: Split issue statistics in 2 pie-charts * https://github.com/devonfw/IDEasy/issues/822[#822] : Added Cwd path limit in shell +* https://github.com/devonfw/IDEasy/issues/2088[#2088]: Normalize and skip malformed PATH entries instead of failing with InvalidPathException * https://github.com/devonfw/IDEasy/issues/1689[#1689]: Fix misleading message in `set-version` when the version is already installed * https://github.com/devonfw/IDEasy/issues/1517[#1517]: Fix IDEasy MSI does not configure Windows Terminal for git-bash * https://github.com/devonfw/IDEasy/issues/1227[#1227]: Added Setup instrcutions to the Windows installer diff --git a/cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java b/cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java index 87ff6b0352..310acc00ba 100644 --- a/cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java +++ b/cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java @@ -1,8 +1,8 @@ package com.devonfw.tools.ide.common; -import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.ArrayList; @@ -16,6 +16,9 @@ import java.util.regex.Pattern; import java.util.stream.Stream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.devonfw.tools.ide.context.IdeContext; import com.devonfw.tools.ide.os.SystemInfoImpl; import com.devonfw.tools.ide.os.WindowsPathSyntax; @@ -33,6 +36,8 @@ */ public class SystemPath { + private static final Logger LOG = LoggerFactory.getLogger(SystemPath.class); + private static final Pattern REGEX_WINDOWS_PATH = Pattern.compile("([a-zA-Z]:)?(\\\\[a-zA-Z0-9\\s_.-]+)+\\\\?"); private final char pathSeparator; @@ -78,7 +83,7 @@ public SystemPath(IdeContext context, String envPath) { */ public SystemPath(IdeContext context, String envPath, Path ideRoot, Path softwarePath) { - this(context, envPath, ideRoot, softwarePath, File.pathSeparatorChar, Collections.emptyList()); + this(context, envPath, ideRoot, softwarePath, System.getProperty("path.separator").charAt(0), Collections.emptyList()); } /** @@ -96,7 +101,10 @@ public SystemPath(IdeContext context, String envPath, Path ideRoot, Path softwar this(context, pathSeparator, extraPathEntries, new HashMap<>(), new ArrayList<>()); String[] envPaths = envPath.split(Character.toString(pathSeparator)); for (String segment : envPaths) { - Path path = Path.of(segment); + Path path = parsePathSegment(segment); + if (path == null) { + continue; + } String tool = getTool(path, ideRoot); if (tool == null) { this.paths.add(path); @@ -105,6 +113,51 @@ public SystemPath(IdeContext context, String envPath, Path ideRoot, Path softwar collectToolPath(softwarePath); } + /** + * Parses a single {@code PATH} segment into a {@link Path}. The segment is first {@link #normalizePathSegment(String) normalized} by removing illegal control + * characters (e.g. CR, LF, NUL, TAB) that may sneak into the {@code PATH} variable (e.g. via broken tool configurations) and would otherwise render the entry + * invalid. This way a still usable entry is kept instead of being dropped. If nothing usable remains after normalization or the segment cannot be parsed as a + * {@link Path} even after normalization, {@code null} is returned so the entry is skipped without aborting the entire {@link SystemPath} construction. + * + * @param segment the raw {@code PATH} segment. + * @return the parsed {@link Path} or {@code null} if the segment is invalid and should be ignored. + */ + private static Path parsePathSegment(String segment) { + + String normalized = normalizePathSegment(segment); + if (normalized.isEmpty()) { + if (!segment.isEmpty()) { + LOG.warn("Ignoring invalid PATH entry '{}' as it only contains illegal control characters.", segment); + } + return null; + } + if (!normalized.equals(segment)) { + LOG.warn("Normalized PATH entry '{}' by removing illegal control characters.", segment); + } + try { + return Path.of(normalized); + } catch (InvalidPathException e) { + LOG.warn("Ignoring invalid PATH entry '{}' - {}", segment, e.getMessage()); + return null; + } + } + + /** + * @param segment the raw {@code PATH} segment. + * @return the given {@code segment} with all {@link Character#isISOControl(char) ISO control characters} (e.g. CR, LF, NUL, TAB) removed. + */ + private static String normalizePathSegment(String segment) { + + StringBuilder sb = new StringBuilder(segment.length()); + for (int i = 0; i < segment.length(); i++) { + char c = segment.charAt(i); + if (!Character.isISOControl(c)) { + sb.append(c); + } + } + return sb.toString(); + } + /** * @param context {@link IdeContext} for the output of information. * @param softwarePath the {@link IdeContext#getSoftwarePath() software path}. diff --git a/cli/src/test/java/com/devonfw/tools/ide/common/SystemPathTest.java b/cli/src/test/java/com/devonfw/tools/ide/common/SystemPathTest.java index 3b7be3335a..a2e9c3853c 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/common/SystemPathTest.java +++ b/cli/src/test/java/com/devonfw/tools/ide/common/SystemPathTest.java @@ -12,6 +12,7 @@ import com.devonfw.tools.ide.context.AbstractIdeContextTest; import com.devonfw.tools.ide.context.IdeTestContext; +import com.devonfw.tools.ide.log.IdeLogLevel; import com.devonfw.tools.ide.os.SystemInfo; import com.devonfw.tools.ide.os.SystemInfoMock; @@ -142,6 +143,45 @@ void testFindBinaryFindsNothingWithoutFilter() { assertThat(result).isEqualTo(test); } + @Test + void testConstructorNormalizesPathEntryWithControlCharactersAndKeepsIt() { + // arrange + IdeTestContext context = newContext("find-binary", "project/workspaces", false); + String validEntry = "C:\\Tools\\bin"; + // an entry polluted with control characters (CR, LF, TAB, NUL) that must be stripped instead of dropping the whole entry + String cleanedEntry = "C:\\Other\\bin"; + String pollutedEntry = "C:\\Other" + "\r\n\t" + (char) 0 + "\\bin"; + String envPath = validEntry + ';' + pollutedEntry; + + // act + SystemPath systemPath = new SystemPath(context, envPath, context.getIdeRoot(), context.getSoftwarePath(), ';', new ArrayList<>()); + + // assert + String result = systemPath.toString(); + assertThat(result).contains(validEntry).contains(cleanedEntry); + assertThat(result).doesNotContain("\r").doesNotContain("\n").doesNotContain("\t").doesNotContain(Character.toString((char) 0)); + assertThat(context).log(IdeLogLevel.WARNING).hasMessageContaining("Normalized PATH entry"); + } + + @Test + void testConstructorSkipsPathEntryConsistingOnlyOfControlCharactersAndKeepsValidOnes() { + // arrange + IdeTestContext context = newContext("find-binary", "project/workspaces", false); + String validEntryBefore = "C:\\Tools\\bin"; + // an entry that has nothing usable left after normalization must be skipped without aborting the whole construction + String controlOnlyEntry = "\r\n\t" + (char) 0; + String validEntryAfter = "C:\\Other\\bin"; + String envPath = validEntryBefore + ';' + controlOnlyEntry + ';' + validEntryAfter; + + // act + SystemPath systemPath = new SystemPath(context, envPath, context.getIdeRoot(), context.getSoftwarePath(), ';', new ArrayList<>()); + + // assert + String result = systemPath.toString(); + assertThat(result).contains(validEntryBefore).contains(validEntryAfter); + assertThat(context).log(IdeLogLevel.WARNING).hasMessageContaining("Ignoring invalid PATH entry"); + } + private static boolean checkPathToIgnoreLowercase(Path p, String toIgnore) { String s = p.toAbsolutePath().toString().replace('\\', '/').toLowerCase(Locale.ROOT); return !s.contains(toIgnore);