Preserve unnormalized symlink in sandboxed execution

bazelbuild · Jun 30, 2022 · 4a66912 · 4a66912
1 parent 43e67ba
commit 4a66912
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 19 deletions.
diff --git a/...main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/...main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
@@ -141,7 +141,7 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
           FileSystemUtils.createEmptyFile(key);
         }
       } else if (inputs.getSymlinks().containsKey(fragment)) {
-        PathFragment symlinkDest = inputs.getSymlinks().get(fragment);
+        String symlinkDest = inputs.getSymlinks().get(fragment);
         if (symlinkDest != null) {
           key.createSymbolicLink(symlinkDest);
         }

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
@@ -224,11 +224,10 @@ private static void cleanRecursively(
             LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX.getRelative(
                 absPath.relativeTo(execroot));
       }
-      Optional<PathFragment> destination =
-          getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
+      Optional<String> destination = getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
       if (destination.isPresent()) {
-        if (SYMLINK.equals(dirent.getType())
-            && PathFragment.create(absPath.readSymbolicLink()).equals(destination.get())) {
+        if (SYMLINK.equals(dirent.getType()) && absPath.readSymbolicLink()
+            .equals(destination.get())) {
           inputsToCreate.remove(pathRelativeToWorkDir);
         } else {
           absPath.delete();
@@ -248,14 +247,14 @@ private static void cleanRecursively(
   }
 
   /**
-   * Returns what the destination of the symlink {@code file} should be, according to {@code
-   * inputs}.
+   * Returns what the destination of the symlink {@code file} should be, according to
+   * {@code inputs}.
    */
-  static Optional<PathFragment> getExpectedSymlinkDestination(
+  static Optional<String> getExpectedSymlinkDestination(
       PathFragment fragment, SandboxInputs inputs) {
     Path file = inputs.getFiles().get(fragment);
     if (file != null) {
-      return Optional.of(file.asFragment());
+      return Optional.of(file.asFragment().getPathString());
     }
     return Optional.ofNullable(inputs.getSymlinks().get(fragment));
   }
@@ -369,7 +368,7 @@ public static final class SandboxInputs {
     private final Set<VirtualActionInput> virtualInputsWithDelayedMaterialization;
     // Virtual inputs that are materialized during {@link #processInputFiles}.
     private final Map<VirtualActionInput, byte[]> materializedVirtualInputs;
-    private final Map<PathFragment, PathFragment> symlinks;
+    private final Map<PathFragment, String> symlinks;
 
     private static final SandboxInputs EMPTY_INPUTS =
         new SandboxInputs(
@@ -379,7 +378,7 @@ public SandboxInputs(
         Map<PathFragment, Path> files,
         Set<VirtualActionInput> virtualInputsWithDelayedMaterialization,
         Map<VirtualActionInput, byte[]> materializedVirtualInputs,
-        Map<PathFragment, PathFragment> symlinks) {
+        Map<PathFragment, String> symlinks) {
       checkState(
           virtualInputsWithDelayedMaterialization.isEmpty() || materializedVirtualInputs.isEmpty(),
           "Either virtualInputsWithDelayedMaterialization or materializedVirtualInputs should be"
@@ -393,7 +392,7 @@ public SandboxInputs(
     public SandboxInputs(
         Map<PathFragment, Path> files,
         Set<VirtualActionInput> virtualInputsWithDelayedMaterialization,
-        Map<PathFragment, PathFragment> symlinks) {
+        Map<PathFragment, String> symlinks) {
       this(files, virtualInputsWithDelayedMaterialization, ImmutableMap.of(), symlinks);
     }
 
@@ -405,7 +404,7 @@ public Map<PathFragment, Path> getFiles() {
       return files;
     }
 
-    public Map<PathFragment, PathFragment> getSymlinks() {
+    public Map<PathFragment, String> getSymlinks() {
       return symlinks;
     }
 
@@ -531,7 +530,7 @@ public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap,
       throws IOException {
     Map<PathFragment, Path> inputFiles = new TreeMap<>();
     Set<VirtualActionInput> virtualInputsWithDelayedMaterialization = new HashSet<>();
-    Map<PathFragment, PathFragment> inputSymlinks = new TreeMap<>();
+    Map<PathFragment, String> inputSymlinks = new TreeMap<>();
     Map<VirtualActionInput, byte[]> materializedVirtualInputs = new HashMap<>();
 
     for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
@@ -549,7 +548,7 @@ public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap,
           }
         } else if (actionInput.isSymlink()) {
           Path inputPath = execRoot.getRelative(actionInput.getExecPath());
-          inputSymlinks.put(pathFragment, PathFragment.create(inputPath.readSymbolicLink()));
+          inputSymlinks.put(pathFragment, inputPath.readSymbolicLink());
         } else {
           Path inputPath = execRoot.getRelative(actionInput.getExecPath());
           inputFiles.put(pathFragment, inputPath);
@@ -564,7 +563,7 @@ public SandboxInputs processInputFiles(Map<PathFragment, ActionInput> inputMap,
 
         if (actionInput.isSymlink()) {
           Path inputPath = execRoot.getRelative(actionInput.getExecPath());
-          inputSymlinks.put(pathFragment, PathFragment.create(inputPath.readSymbolicLink()));
+          inputSymlinks.put(pathFragment, inputPath.readSymbolicLink());
         } else {
           Path inputPath =
               actionInput instanceof EmptyActionInput

diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java
@@ -79,7 +79,7 @@ static void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs in
           FileSystemUtils.createEmptyFile(key);
         }
       } else if (inputs.getSymlinks().containsKey(fragment)) {
-        PathFragment symlinkDest = inputs.getSymlinks().get(fragment);
+        String symlinkDest = inputs.getSymlinks().get(fragment);
         if (symlinkDest != null) {
           key.createSymbolicLink(symlinkDest);
         }

diff --git a/src/test/shell/bazel/bazel_symlink_test.sh b/src/test/shell/bazel/bazel_symlink_test.sh
@@ -381,11 +381,11 @@ java_binary(
     srcs = ["MakeSymlink.java"],
     main_class = "MakeSymlink",
 )
-a(name="a", link_target="somewhere/over/the/rainbow")
+a(name="a", link_target="../somewhere/../over/the/rainbow/..")
 EOF
 
   bazel --windows_enable_symlinks build --experimental_allow_unresolved_symlinks //a:a || fail "build failed"
-  assert_contains "input link is somewhere/over/the/rainbow" bazel-bin/a/a.file
+  assert_contains "input link is ../somewhere/../over/the/rainbow/.." bazel-bin/a/a.file
 }
 
 function test_dangling_symlink_created_from_symlink_action() {