Accept valid relative symlinks in TreeArtifacts.
--
MOS_MIGRATED_REVID=137072310
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index d21c908..cbb3976 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -77,6 +77,14 @@
/** Timestamp builder tests for TreeArtifacts. */
@RunWith(JUnit4.class)
public class TreeArtifactBuildTest extends TimestampBuilderTestCase {
+
+ private static final Predicate<Event> IS_ERROR_EVENT = new Predicate<Event>() {
+ @Override
+ public boolean apply(Event event) {
+ return event.getKind().equals(EventKind.ERROR);
+ }
+ };
+
// Common Artifacts, TreeFileArtifact, and Buttons. These aren't all used in all tests, but
// they're used often enough that we can save ourselves a lot of copy-pasted code by creating them
// in setUp().
@@ -145,8 +153,7 @@
Action testAction = new TestAction(
TestAction.NO_EFFECT, ImmutableList.of(outOne), ImmutableList.of(normalOutput)) {
@Override
- public void execute(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
+ public void execute(ActionExecutionContext actionExecutionContext) {
try {
// Check the file cache for input TreeFileArtifacts.
ActionInputFileCache fileCache = actionExecutionContext.getActionInputFileCache();
@@ -388,18 +395,10 @@
reporter.removeHandler(failFastHandler);
reporter.addHandler(storingEventHandler);
- Predicate<Event> isErrorEvent = new Predicate<Event>() {
- @Override
- public boolean apply(Event event) {
- return event.getKind().equals(EventKind.ERROR);
- }
- };
-
TreeArtifactTestAction failureOne = new TreeArtifactTestAction(
Runnables.doNothing(), outOneFileOne, outOneFileTwo) {
@Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
+ public void executeTestBehavior(ActionExecutionContext actionExecutionContext) {
try {
writeFile(outOneFileOne, "one");
writeFile(outOneFileTwo, "two");
@@ -418,7 +417,7 @@
} catch (BuildFailedException e) {
//not all outputs were created
List<Event> errors = ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), isErrorEvent));
+ Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage()).contains("not present on disk");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
@@ -427,8 +426,7 @@
TreeArtifactTestAction failureTwo = new TreeArtifactTestAction(
Runnables.doNothing(), outTwoFileOne, outTwoFileTwo) {
@Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
+ public void executeTestBehavior(ActionExecutionContext actionExecutionContext) {
try {
writeFile(outTwoFileOne, "one");
writeFile(outTwoFileTwo, "two");
@@ -449,7 +447,7 @@
fail(); // Should have thrown
} catch (BuildFailedException e) {
List<Event> errors = ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), isErrorEvent));
+ Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage()).contains("not present on disk");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
@@ -476,8 +474,7 @@
TreeArtifactTestAction action = new TreeArtifactTestAction(out) {
@Override
- public void execute(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
+ public void execute(ActionExecutionContext actionExecutionContext) {
try {
writeFile(out.getPath().getChild("one"), "one");
writeFile(out.getPath().getChild("two"), "two");
@@ -502,6 +499,148 @@
checkFilePermissions(out.getPath().getChild("three").getChild("four"));
}
+ @Test
+ public void testValidRelativeSymlinkAccepted() throws Exception {
+ final Artifact out = createTreeArtifact("output");
+
+ TreeArtifactTestAction action = new TreeArtifactTestAction(out) {
+ @Override
+ public void execute(ActionExecutionContext actionExecutionContext) {
+ try {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"),
+ "../one");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+
+ registerAction(action);
+
+ buildArtifact(action.getSoleOutput());
+ }
+
+ @Test
+ public void testInvalidSymlinkRejected() throws Exception {
+ // Failure expected
+ StoredEventHandler storingEventHandler = new StoredEventHandler();
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(storingEventHandler);
+
+ final Artifact out = createTreeArtifact("output");
+
+ TreeArtifactTestAction action = new TreeArtifactTestAction(out) {
+ @Override
+ public void execute(ActionExecutionContext actionExecutionContext) {
+ try {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"),
+ "../invalid");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+
+ registerAction(action);
+
+ try {
+ buildArtifact(action.getSoleOutput());
+ fail(); // Should have thrown
+ } catch (BuildFailedException e) {
+ List<Event> errors = ImmutableList.copyOf(
+ Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains(
+ "Failed to resolve relative path links/link");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
+ }
+ }
+
+ @Test
+ public void testAbsoluteSymlinkRejected() throws Exception {
+ // Failure expected
+ StoredEventHandler storingEventHandler = new StoredEventHandler();
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(storingEventHandler);
+
+ final Artifact out = createTreeArtifact("output");
+
+ TreeArtifactTestAction action = new TreeArtifactTestAction(out) {
+ @Override
+ public void execute(ActionExecutionContext actionExecutionContext) {
+ try {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"),
+ "/random/pointer");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+
+ registerAction(action);
+
+ try {
+ buildArtifact(action.getSoleOutput());
+ fail(); // Should have thrown
+ } catch (BuildFailedException e) {
+ List<Event> errors = ImmutableList.copyOf(
+ Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains(
+ "A TreeArtifact may not contain absolute symlinks");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
+ }
+ }
+
+ @Test
+ public void testRelativeSymlinkTraversingOutsideOfTreeArtifactRejected() throws Exception {
+ // Failure expected
+ StoredEventHandler storingEventHandler = new StoredEventHandler();
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(storingEventHandler);
+
+ final Artifact out = createTreeArtifact("output");
+
+ TreeArtifactTestAction action = new TreeArtifactTestAction(out) {
+ @Override
+ public void execute(ActionExecutionContext actionExecutionContext) {
+ try {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"),
+ "../../output/random/pointer");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+
+ registerAction(action);
+
+ try {
+ buildArtifact(action.getSoleOutput());
+ fail(); // Should have thrown
+ } catch (BuildFailedException e) {
+ List<Event> errors = ImmutableList.copyOf(
+ Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains(
+ "A TreeArtifact may not contain relative symlinks whose target paths traverse "
+ + "outside of the TreeArtifact");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
+ }
+ }
+
// This is more a smoke test than anything, because it turns out that:
// 1) there is no easy way to turn fast digests on/off for these test cases, and
// 2) injectDigest() doesn't really complain if you inject bad digests or digests
