#2088: skip malformed PATH entries instead of failing with InvalidPathException

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

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}.

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);