Rollback of commit bdfd58a8ca2ed5735d6aaa5b238fb0f689515724.

--
MOS_MIGRATED_REVID=125160288

2 files changed, 16 insertions, 16 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
index 317051b9d4..f9c31ca044 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
@@ -150,26 +150,25 @@ public class ArtifactFactoryTest {
   }
 
   @Test
-  public void testResolveArtifactWithUpLevel() throws Exception {
+  public void testResolveArtifactWithUpLevelFailsCleanly() throws Exception {
     // We need a package in the root directory to make every exec path (even one with up-level
     // references) be in a package.
     Map<PackageIdentifier, Root> packageRoots = ImmutableMap.of(
-        PackageIdentifier.create("@workspace", new PathFragment("")), clientRoot,
-        PackageIdentifier.create("@repo", new PathFragment("dir")), clientRoot);
+        PackageIdentifier.createInMainRepo(new PathFragment("")), clientRoot);
     artifactFactory.setPackageRoots(packageRoots);
-    PathFragment topLevel = new PathFragment("../workspace/foo");
-    PathFragment subdir = new PathFragment("../repo/dir/foo");
-    Artifact topLevelArtifact = artifactFactory.resolveSourceArtifact(topLevel);
-    assertThat(topLevelArtifact).isNotNull();
-    Artifact subdirArtifact = artifactFactory.resolveSourceArtifact(subdir);
-    assertThat(subdirArtifact).isNotNull();
+    PathFragment outsideWorkspace = new PathFragment("../foo");
+    PathFragment insideWorkspace =
+        new PathFragment("../" + clientRoot.getPath().getBaseName() + "/foo");
+    assertNull(artifactFactory.resolveSourceArtifact(outsideWorkspace));
+    assertNull("Up-level-containing paths that descend into the right workspace aren't allowed",
+            artifactFactory.resolveSourceArtifact(insideWorkspace));
     MockPackageRootResolver packageRootResolver = new MockPackageRootResolver();
     packageRootResolver.setPackageRoots(packageRoots);
     Map<PathFragment, Artifact> result = new HashMap<>();
-    result.put(topLevel, topLevelArtifact);
-    result.put(subdir, subdirArtifact);
+    result.put(insideWorkspace, null);
+    result.put(outsideWorkspace, null);
     assertThat(
-        artifactFactory.resolveSourceArtifacts(ImmutableList.of(topLevel, subdir),
+        artifactFactory.resolveSourceArtifacts(ImmutableList.of(insideWorkspace, outsideWorkspace),
             packageRootResolver).entrySet()).containsExactlyElementsIn(result.entrySet());
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
index 676eab402a..e43ddd1b28 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
@@ -57,12 +57,13 @@ public class ArtifactTest {
   }
 
   @Test
-  public void testConstruction_UplevelRootDir() throws IOException {
+  public void testConstruction_badRootDir() throws IOException {
     Path f1 = scratch.file("/exec/dir/file.ext");
     Path bogusDir = scratch.file("/exec/dir/bogus");
-    Artifact artifact = new Artifact(f1, Root.asDerivedRoot(bogusDir), f1.relativeTo(execDir));
-    assertThat(artifact.getExecPath()).isEqualTo(new PathFragment("dir/file.ext"));
-    assertThat(artifact.getRootRelativePath()).isEqualTo(new PathFragment("../file.ext"));
+    try {
+      new Artifact(f1, Root.asDerivedRoot(bogusDir), f1.relativeTo(execDir));
+      fail("Expected IllegalArgumentException constructing artifact with a bad root dir");
+    } catch (IllegalArgumentException expected) {}
   }
 
   @Test