Force paths to be relative when archives are decompressed

--
MOS_MIGRATED_REVID=123350350

3 files changed, 66 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java
index 77716dc4f2..46650d6150 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPath.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.common.base.Optional;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
@@ -27,14 +28,25 @@ public final class StripPrefixedPath {
   private final boolean found;
   private final boolean skip;
 
+  /**
+   * If a prefix is given, it will be removed from the entry's path. This also turns absolute paths
+   * into relative paths (e.g., /usr/bin/bash will become usr/bin/bash, same as unzip's default
+   * behavior) and normalizes the paths (foo/../bar////baz will become bar/baz). Note that this
+   * could cause collisions, if a zip file had one entry for bin/some-binary and another entry for
+   * /bin/some-binary.
+   *
+   * Note that the prefix is stripped to move the files up one level, so if you have an entry
+   * "foo/../bar" and a prefix of "foo", the result will be "bar" not "../bar".
+   */
   public static StripPrefixedPath maybeDeprefix(String entry, Optional<String> prefix) {
-    boolean found = false;
-    PathFragment entryPath = new PathFragment(entry);
+    Preconditions.checkNotNull(entry);
+    PathFragment entryPath = relativize(entry);
     if (!prefix.isPresent()) {
       return new StripPrefixedPath(entryPath, false, false);
     }
 
-    PathFragment prefixPath = new PathFragment(prefix.get());
+    PathFragment prefixPath = relativize(prefix.get());
+    boolean found = false;
     boolean skip = false;
     if (entryPath.startsWith(prefixPath)) {
       found = true;
@@ -48,6 +60,18 @@ public final class StripPrefixedPath {
     return new StripPrefixedPath(entryPath, found, skip);
   }
 
+  /**
+   * Normalize the path and, if it is absolute, make it relative (e.g., /foo/bar becomes foo/bar).
+   */
+  private static PathFragment relativize(String path) {
+    PathFragment entryPath = new PathFragment(path).normalize();
+    if (entryPath.isAbsolute()) {
+      entryPath = new PathFragment(entryPath.getSafePathString().substring(
+          entryPath.windowsVolume().length() + 1));
+    }
+    return entryPath;
+  }
+
   private StripPrefixedPath(PathFragment pathFragment, boolean found, boolean skip) {
     this.pathFragment = pathFragment;
     this.found = found;
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
index f99926642c..6ee8c565fa 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
@@ -12,6 +12,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:collect",
         "//src/main/java/com/google/devtools/build/lib:events",
         "//src/main/java/com/google/devtools/build/lib:inmemoryfs",
+        "//src/main/java/com/google/devtools/build/lib:os_util",
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
         "//src/main/java/com/google/devtools/build/lib:syntax",
         "//src/main/java/com/google/devtools/build/lib:vfs",
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
index 2998460a76..f3ab3f1b8d 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/StripPrefixedPathTest.java
@@ -14,11 +14,13 @@
 
 package com.google.devtools.build.lib.bazel.repository;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.base.Optional;
+import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import org.junit.Test;
@@ -46,4 +48,40 @@ public class StripPrefixedPathTest {
     result = StripPrefixedPath.maybeDeprefix("foof/bar", Optional.of("foo"));
     assertFalse(result.foundPrefix());
   }
+
+  @Test
+  public void testAbsolute() {
+    StripPrefixedPath result = StripPrefixedPath.maybeDeprefix(
+        "/foo/bar", Optional.<String>absent());
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("foo/bar"));
+
+    result = StripPrefixedPath.maybeDeprefix("///foo/bar/baz", Optional.<String>absent());
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("foo/bar/baz"));
+
+    result = StripPrefixedPath.maybeDeprefix("/foo/bar/baz", Optional.of("/foo"));
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("bar/baz"));
+  }
+
+  @Test
+  public void testWindowsAbsolute() {
+    if (OS.getCurrent() != OS.WINDOWS) {
+      return;
+    }
+    StripPrefixedPath result = StripPrefixedPath.maybeDeprefix(
+        "c:/foo/bar", Optional.<String>absent());
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("foo/bar"));
+  }
+
+  @Test
+  public void testNormalize() {
+    StripPrefixedPath result = StripPrefixedPath.maybeDeprefix(
+        "../bar", Optional.<String>absent());
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("../bar"));
+
+    result = StripPrefixedPath.maybeDeprefix("foo/../baz", Optional.<String>absent());
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("baz"));
+
+    result = StripPrefixedPath.maybeDeprefix("foo/../baz", Optional.of("foo"));
+    assertThat(result.getPathFragment()).isEqualTo(new PathFragment("baz"));
+  }
 }