Single I/O operation in DirectoryIdLoader

cryptomator · May 30, 2017 · 2387356 · 2387356
1 parent 7e8665d
commit 2387356
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 22 deletions.
diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java
@@ -1,9 +1,12 @@
 package org.cryptomator.cryptofs;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.UUID;
 
 import javax.inject.Inject;
@@ -13,19 +16,29 @@
 @PerFileSystem
 class DirectoryIdLoader extends CacheLoader<Path, String> {
 
+	private static final int MAX_DIR_ID_LENGTH = 1000;
+
 	@Inject
 	public DirectoryIdLoader() {
 	}
 
 	@Override
 	public String load(Path dirFilePath) throws IOException {
-		if (Files.exists(dirFilePath)) {
-			byte[] bytes = Files.readAllBytes(dirFilePath);
-			if (bytes.length == 0) {
+		try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ)) {
+			long size = ch.size();
+			if (size == 0) {
 				throw new IOException("Invalid, empty directory file: " + dirFilePath);
+			} else if (size > MAX_DIR_ID_LENGTH) {
+				throw new IOException("Unexpectedly large directory file: " + dirFilePath);
+			} else {
+				assert size <= MAX_DIR_ID_LENGTH; // thus int
+				ByteBuffer buffer = ByteBuffer.allocate((int) size);
+				int read = ch.read(buffer);
+				assert read == size;
+				buffer.flip();
+				return StandardCharsets.UTF_8.decode(buffer).toString();
 			}
-			return new String(bytes, StandardCharsets.UTF_8);
-		} else {
+		} catch (NoSuchFileException e) {
 			return UUID.randomUUID().toString();
 		}
 	}

diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java
@@ -12,29 +12,32 @@
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
-import java.nio.channels.SeekableByteChannel;
+import java.nio.channels.FileChannel;
+import java.nio.channels.spi.AbstractInterruptibleChannel;
 import java.nio.file.FileSystem;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.spi.FileSystemProvider;
 
-import org.cryptomator.cryptofs.mocks.SeekableByteChannelMock;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
 
 public class DirectoryIdLoaderTest {
 
 	@Rule
 	public ExpectedException thrown = ExpectedException.none();
 
-	private FileSystemProvider provider = mock(FileSystemProvider.class);
-	private FileSystem fileSystem = mock(FileSystem.class);
-	private Path dirFilePath = mock(Path.class);
-	private Path otherDirFilePath = mock(Path.class);
+	private final FileSystemProvider provider = mock(FileSystemProvider.class);
+	private final FileSystem fileSystem = mock(FileSystem.class);
+	private final Path dirFilePath = mock(Path.class);
+	private final Path otherDirFilePath = mock(Path.class);
 
-	private DirectoryIdLoader inTest = new DirectoryIdLoader();
+	private final DirectoryIdLoader inTest = new DirectoryIdLoader();
 
 	@Before
 	public void setup() {
@@ -45,8 +48,8 @@ public void setup() {
 
 	@Test
 	public void testDirectoryIdsForTwoNonExistingFilesDiffer() throws IOException {
-		doThrow(new IOException()).when(provider).checkAccess(dirFilePath);
-		doThrow(new IOException()).when(provider).checkAccess(otherDirFilePath);
+		doThrow(new NoSuchFileException("foo")).when(provider).newFileChannel(eq(dirFilePath), any());
+		doThrow(new NoSuchFileException("bar")).when(provider).newFileChannel(eq(otherDirFilePath), any());
 
 		String first = inTest.load(dirFilePath);
 		String second = inTest.load(otherDirFilePath);
@@ -56,7 +59,7 @@ public void testDirectoryIdsForTwoNonExistingFilesDiffer() throws IOException {
 
 	@Test
 	public void testDirectoryIdForNonExistingFileIsNotEmpty() throws IOException {
-		doThrow(new IOException()).when(provider).checkAccess(dirFilePath);
+		doThrow(new NoSuchFileException("foo")).when(provider).newFileChannel(eq(dirFilePath), any());
 
 		String result = inTest.load(dirFilePath);
 
@@ -65,26 +68,56 @@ public void testDirectoryIdForNonExistingFileIsNotEmpty() throws IOException {
 	}
 
 	@Test
-	public void testDirectoryIdIsReadFromExistingFile() throws IOException {
+	public void testDirectoryIdIsReadFromExistingFile() throws IOException, ReflectiveOperationException {
 		String expectedId = "asdüßT°z¬╚‗";
 		byte[] expectedIdBytes = expectedId.getBytes(UTF_8);
-		SeekableByteChannel channel = new SeekableByteChannelMock(ByteBuffer.wrap(expectedIdBytes));
-		when(provider.newByteChannel(eq(dirFilePath), any())).thenReturn(channel);
+		FileChannel channel = createFileChannelMock();
+		when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel);
+		when(channel.size()).thenReturn((long) expectedIdBytes.length);
+		when(channel.read(any(ByteBuffer.class))).then(invocation -> {
+			ByteBuffer buf = invocation.getArgument(0);
+			buf.put(expectedIdBytes);
+			return expectedIdBytes.length;
+		});
 
 		String result = inTest.load(dirFilePath);
 
 		assertThat(result, is(expectedId));
 	}
 
 	@Test
-	public void testIOExceptionWhenExistingFileIsEmpty() throws IOException {
-		SeekableByteChannel channel = new SeekableByteChannelMock(ByteBuffer.allocate(0));
-		when(provider.newByteChannel(eq(dirFilePath), any())).thenReturn(channel);
+	public void testIOExceptionWhenExistingFileIsEmpty() throws IOException, ReflectiveOperationException {
+		FileChannel channel = createFileChannelMock();
+		when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel);
+		when(channel.size()).thenReturn(0l);
 
 		thrown.expect(IOException.class);
 		thrown.expectMessage("Invalid, empty directory file");
 
 		inTest.load(dirFilePath);
 	}
 
+	@Test
+	public void testIOExceptionWhenExistingFileIsTooLarge() throws IOException, ReflectiveOperationException {
+		FileChannel channel = createFileChannelMock();
+		when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel);
+		when(channel.size()).thenReturn((long) Integer.MAX_VALUE);
+
+		thrown.expect(IOException.class);
+		thrown.expectMessage("Unexpectedly large directory file");
+
+		inTest.load(dirFilePath);
+	}
+
+	private FileChannel createFileChannelMock() throws ReflectiveOperationException {
+		FileChannel channel = Mockito.mock(FileChannel.class);
+		Field channelOpenField = AbstractInterruptibleChannel.class.getDeclaredField("open");
+		channelOpenField.setAccessible(true);
+		channelOpenField.set(channel, true);
+		Field channelCloseLockField = AbstractInterruptibleChannel.class.getDeclaredField("closeLock");
+		channelCloseLockField.setAccessible(true);
+		channelCloseLockField.set(channel, new Object());
+		return channel;
+	}
+
 }