Skip to content

Commit

Permalink
Force source files to be readable before copying them from sandbox.
Browse files Browse the repository at this point in the history
Rules can (and have) accidentally made outputs unreadable, which makes the sandbox copying fail only when moving across devices. Unreadable outputs make no sense, so we just force them to be readable.

RELNOTES: None.
PiperOrigin-RevId: 363668244
  • Loading branch information
larsrc-google authored and katre committed Jul 13, 2021
1 parent 451b296 commit 671e048
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,17 @@ public static MoveResult moveFile(Path from, Path to) throws IOException {
try (InputStream in = from.getInputStream();
OutputStream out = to.getOutputStream()) {
copyLargeBuffer(in, out);
} catch (FileAccessException e1) {
// Rules can accidentally make output non-readable, let's fix that (b/150963503)
if (!from.isReadable()) {
from.setReadable(true);
try (InputStream in = from.getInputStream();
OutputStream out = to.getOutputStream()) {
copyLargeBuffer(in, out);
}
} else {
throw e1;
}
}
to.setLastModifiedTime(stat.getLastModifiedTime()); // Preserve mtime.
if (!from.isWritable()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,26 +364,11 @@ public void testMoveFile() throws IOException {

@Test
public void testMoveFileAcrossDevices() throws Exception {
class MultipleDeviceFS extends InMemoryFileSystem {
MultipleDeviceFS() {
super(DigestHashFunction.SHA256);
}

@Override
public void renameTo(Path source, Path target) throws IOException {
if (!source.startsWith(target.asFragment().subFragment(0, 1))) {
throw new IOException("EXDEV");
}
super.renameTo(source, target);
}
}
FileSystem fs = new MultipleDeviceFS();
Path dev1 = fs.getPath("/fs1");
dev1.createDirectory();
Path dev2 = fs.getPath("/fs2");
dev2.createDirectory();
Path source = dev1.getChild("source");
Path target = dev2.getChild("target");
Path source = fs.getPath("/fs1/source");
source.getParentDirectory().createDirectoryAndParents();
Path target = fs.getPath("/fs2/target");
target.getParentDirectory().createDirectoryAndParents();

FileSystemUtils.writeContent(source, UTF_8, "hello, world");
source.setLastModifiedTime(142);
Expand All @@ -394,12 +379,34 @@ public void renameTo(Path source, Path target) throws IOException {
assertThat(target.getLastModifiedTime()).isEqualTo(142);

source.createSymbolicLink(PathFragment.create("link-target"));

assertThat(FileSystemUtils.moveFile(source, target)).isEqualTo(MoveResult.FILE_COPIED);

assertThat(source.exists(Symlinks.NOFOLLOW)).isFalse();
assertThat(target.isSymbolicLink()).isTrue();
assertThat(target.readSymbolicLink()).isEqualTo(PathFragment.create("link-target"));
}

@Test
public void testMoveFileFixPermissions() throws Exception {
FileSystem fs = new MultipleDeviceFS();
Path source = fs.getPath("/fs1/source");
source.getParentDirectory().createDirectoryAndParents();
Path target = fs.getPath("/fs2/target");
target.getParentDirectory().createDirectoryAndParents();

FileSystemUtils.writeContent(source, UTF_8, "linear-a");
source.setLastModifiedTime(142);
source.setReadable(false);

MoveResult moveResult = moveFile(source, target);

assertThat(moveResult).isEqualTo(MoveResult.FILE_COPIED);
assertThat(source.exists(Symlinks.NOFOLLOW)).isFalse();
assertThat(target.isFile(Symlinks.NOFOLLOW)).isTrue();
assertThat(FileSystemUtils.readContent(target, UTF_8)).isEqualTo("linear-a");
}

@Test
public void testReadContentWithLimit() throws IOException {
createTestDirectoryTree();
Expand Down Expand Up @@ -834,4 +841,18 @@ public void testCreateHardLinkForNonEmptyDirectory_success() throws Exception {
assertThat(fileSystem.stat(linkPath3, false).getNodeId())
.isEqualTo(fileSystem.stat(originalPath3, false).getNodeId());
}

static class MultipleDeviceFS extends InMemoryFileSystem {
MultipleDeviceFS() {
super(DigestHashFunction.SHA256);
}

@Override
public void renameTo(Path source, Path target) throws IOException {
if (!source.startsWith(target.asFragment().subFragment(0, 1))) {
throw new IOException("EXDEV");
}
super.renameTo(source, target);
}
}
}

0 comments on commit 671e048

Please sign in to comment.