From a84e388fd105bec5e7aa3ece66cbf8998da5a2a5 Mon Sep 17 00:00:00 2001
From: "bazel.build machine account"
 <15028808+bazel-io@users.noreply.github.com>
Date: Thu, 8 Feb 2024 12:23:58 -0500
Subject: [PATCH] [7.1.0] Error on invalid path characters in `.bazelignore`
 (#21259)

Fixes https://github.com/bazelbuild/bazel/issues/20906.
cc @meteorcloudy

Closes #21170.

Commit
https://github.com/bazelbuild/bazel/commit/026f493a5a403fa5d770cae0b3a3baf8dcf33488

PiperOrigin-RevId: 605291552
Change-Id: I8d42729f176b4325ee402c0c6143db9d534c5e0b

Co-authored-by: Sushain Cherivirala <sushain@skc.name>
---
 .../IgnoredPackagePrefixesFunction.java       | 26 ++++++++++++++++++-
 .../build/lib/skyframe/SkyframeExecutor.java  |  4 ++-
 src/test/shell/bazel/bazelignore_test.sh      | 11 ++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java
index e3ec760e96a43e..f5ffb25d802ad4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/IgnoredPackagePrefixesFunction.java
@@ -31,6 +31,8 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
 import javax.annotation.Nullable;
 
 /**
@@ -60,13 +62,16 @@ public static void getIgnoredPackagePrefixes(
                   + " is not readable because: "
                   + errorMessage
                   + ". Was it modified mid-build?"));
+    } catch (InvalidPathException e) {
+      throw new IgnoredPatternsFunctionException(
+          new InvalidIgnorePathException(e, patternFile.asPath().toString()));
     }
   }
 
   @Nullable
   @Override
   public SkyValue compute(SkyKey key, Environment env)
-      throws SkyFunctionException, InterruptedException {
+      throws IgnoredPatternsFunctionException, InterruptedException {
     RepositoryName repositoryName = (RepositoryName) key.argument();
 
     ImmutableSet.Builder<PathFragment> ignoredPackagePrefixesBuilder = ImmutableSet.builder();
@@ -122,6 +127,15 @@ private static final class PathFragmentLineProcessor
     public boolean processLine(String line) {
       if (!line.isEmpty() && !line.startsWith("#")) {
         fragments.add(PathFragment.create(line));
+
+        // This is called for its side-effects rather than its output.
+        // Specifically, it validates that the line is a valid path. This
+        // doesn't do much on UNIX machines where only NUL is an invalid
+        // character but can reject paths on Windows.
+        //
+        // This logic would need to be adjusted if wildcards are ever supported
+        // (https://github.com/bazelbuild/bazel/issues/7093).
+        var unused = Path.of(line);
       }
       return true;
     }
@@ -132,9 +146,19 @@ public ImmutableSet<PathFragment> getResult() {
     }
   }
 
+  private static class InvalidIgnorePathException extends Exception {
+    public InvalidIgnorePathException(InvalidPathException e, String path) {
+      super("Invalid path in " + path + ": " + e);
+    }
+  }
+
   private static final class IgnoredPatternsFunctionException extends SkyFunctionException {
     public IgnoredPatternsFunctionException(InconsistentFilesystemException e) {
       super(e, Transience.TRANSIENT);
     }
+
+    public IgnoredPatternsFunctionException(InvalidIgnorePathException e) {
+      super(e, Transience.PERSISTENT);
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index e994c099f5dd0a..d41dba2b218e34 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -3127,7 +3127,9 @@ protected WorkspaceInfoFromDiff handleDiffs(
         // Ignored package prefixes are specified relative to the workspace root
         // by definition of .bazelignore. So, we only use ignored paths when the
         // package root is equal to the workspace path.
-        if (workspacePath != null && workspacePath.equals(pathEntry.asPath())) {
+        if (workspacePath != null
+            && workspacePath.equals(pathEntry.asPath())
+            && ignoredPackagePrefixesValue != null) {
           ignoredPaths =
               ignoredPackagePrefixesValue.getPatterns().stream()
                   .map(pathEntry::getRelative)
diff --git a/src/test/shell/bazel/bazelignore_test.sh b/src/test/shell/bazel/bazelignore_test.sh
index 30eb92ca13853f..f011aaeff10725 100755
--- a/src/test/shell/bazel/bazelignore_test.sh
+++ b/src/test/shell/bazel/bazelignore_test.sh
@@ -206,4 +206,15 @@ EOI
     assert_not_contains "ignoreme" output
 }
 
+test_invalid_path() {
+    rm -rf work && mkdir work && cd work
+    create_workspace_with_default_repos WORKSPACE
+    echo -e "foo/\0/bar" > .bazelignore
+    echo 'filegroup(name="f", srcs=glob(["**"]))' > BUILD
+    if bazel build //... 2> "$TEST_log"; then
+      fail "Bazel build should have failed"
+    fi
+    expect_log "java.nio.file.InvalidPathException: Nul character not allowed"
+}
+
 run_suite "Integration tests for .bazelignore"