Improved error logging when encountering malformed filenames during m…

…igration, fixes #76

src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java

@@ -44,7 +44,7 @@ class FilePathMigration {
 	private final String oldCanonicalName;
 
 	/**
-	 * @param oldPath The actual file path before migration
+	 * @param oldPath          The actual file path before migration
 	 * @param oldCanonicalName The inflated old filename without any conflicting pre- or suffixes but including the file type prefix
 	 */
 	FilePathMigration(Path oldPath, String oldCanonicalName) {
@@ -57,7 +57,7 @@ class FilePathMigration {
 	 * Starts a migration of the given file.
 	 *
 	 * @param vaultRoot Path to the vault's base directory (parent of <code>d/</code> and <code>m/</code>).
-	 * @param oldPath Path of an existing file inside the <code>d/</code> directory of a vault. May be a normal file, directory file or symlink as well as conflicting copies.
+	 * @param oldPath   Path of an existing file inside the <code>d/</code> directory of a vault. May be a normal file, directory file or symlink as well as conflicting copies.
 	 * @return A new instance of FileNameMigration
 	 * @throws IOException Non-recoverable I/O error, such as {@link UninflatableFileException}s
 	 */
@@ -89,7 +89,7 @@ public static Optional<FilePathMigration> parse(Path vaultRoot, Path oldPath) th
 	/**
 	 * Resolves the canonical name of a deflated file represented by the given <code>longFileName</code>.
 	 *
-	 * @param vaultRoot Path to the vault's base directory (parent of <code>d/</code> and <code>m/</code>).
+	 * @param vaultRoot    Path to the vault's base directory (parent of <code>d/</code> and <code>m/</code>).
 	 * @param longFileName Canonical name of the {@value #OLD_SHORTENED_FILENAME_SUFFIX} file.
 	 * @return The inflated filename
 	 * @throws UninflatableFileException If the file could not be inflated due to missing or malformed metadata.
@@ -151,7 +151,7 @@ public Path migrate() throws IOException {
 	 * @return The path after successful migration of {@link #oldPath} if migration is successful for the given attemptSuffix
 	 */
 	// visible for testing
-	Path getTargetPath(String attemptSuffix) {
+	Path getTargetPath(String attemptSuffix) throws InvalidOldFilenameException {
 		final String canonicalInflatedName = getNewInflatedName();
 		final String canonicalDeflatedName = getNewDeflatedName();
 		final boolean isShortened = !canonicalInflatedName.equals(canonicalDeflatedName);
@@ -177,7 +177,7 @@ Path getTargetPath(String attemptSuffix) {
 			}
 		}
 	}
-	
+
 	public Path getOldPath() {
 		return oldPath;
 	}
@@ -202,19 +202,34 @@ String getOldCanonicalNameWithoutTypePrefix() {
 	}
 
 	/**
-	 * @return BASE64-encode(BASE32-decode({@link #getOldCanonicalNameWithoutTypePrefix oldCanonicalNameWithoutPrefix})) + {@value #NEW_REGULAR_SUFFIX}
+	 * @return BASE64-encode({@link #getDecodedCiphertext oldDecodedCiphertext}) + {@value #NEW_REGULAR_SUFFIX}
+	 * @throws InvalidOldFilenameException if failing to base32-decode the old filename
 	 */
 	// visible for testing
-	String getNewInflatedName() {
-		byte[] decoded = BASE32.decode(getOldCanonicalNameWithoutTypePrefix());
+	String getNewInflatedName() throws InvalidOldFilenameException {
+		byte[] decoded = getDecodedCiphertext();
 		return BASE64.encode(decoded) + NEW_REGULAR_SUFFIX;
 	}
 
+	/**
+	 * @return BASE32-decode({@link #getOldCanonicalNameWithoutTypePrefix oldCanonicalNameWithoutPrefix})
+	 * @throws InvalidOldFilenameException if failing to base32-decode the old filename
+	 */
+	// visible for testing
+	byte[] getDecodedCiphertext() throws InvalidOldFilenameException {
+		String encodedCiphertext = getOldCanonicalNameWithoutTypePrefix();
+		try {
+			return BASE32.decode(encodedCiphertext);
+		} catch (IllegalArgumentException e) {
+			throw new InvalidOldFilenameException("Can't base32-decode '" + encodedCiphertext + "' in file " + oldPath.toString(), e);
+		}
+	}
+
 	/**
 	 * @return {@link #getNewInflatedName() newInflatedName} if it is shorter than {@link #SHORTENING_THRESHOLD}, else BASE64(SHA1(newInflatedName)) + ".c9s"
 	 */
 	// visible for testing
-	String getNewDeflatedName() {
+	String getNewDeflatedName() throws InvalidOldFilenameException {
 		String inflatedName = getNewInflatedName();
 		if (inflatedName.length() > SHORTENING_THRESHOLD) {
 			byte[] longFileNameBytes = inflatedName.getBytes(UTF_8);

src/main/java/org/cryptomator/cryptofs/migration/v7/InvalidOldFilenameException.java

@@ -0,0 +1,10 @@
+package org.cryptomator.cryptofs.migration.v7;
+
+import java.io.IOException;
+
+public class InvalidOldFilenameException extends IOException {
+
+	public InvalidOldFilenameException(String message, Throwable cause) {
+		super(message, cause);
+	}
+}

src/test/java/org/cryptomator/cryptofs/migration/v7/FilePathMigrationTest.java

@@ -15,6 +15,9 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.converter.ArgumentConversionException;
+import org.junit.jupiter.params.converter.ConvertWith;
+import org.junit.jupiter.params.converter.SimpleArgumentConverter;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mockito;
@@ -25,6 +28,7 @@
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Base64;
 import java.util.Optional;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -44,14 +48,39 @@ public void testGetOldCanonicalNameWithoutTypePrefix(String oldCanonicalName, St
 
 		Assertions.assertEquals(expectedResult, migration.getOldCanonicalNameWithoutTypePrefix());
 	}
+
+	@ParameterizedTest(name = "getDecodedCiphertext() expected to be {1} for {0}")
+	@CsvSource({
+			"ORSXG5A=,dGVzdA==",
+			"0ORSXG5A=,dGVzdA==",
+			"1SORSXG5A=,dGVzdA==",
+	})
+	public void testGetDecodedCiphertext(String oldCanonicalName, @ConvertWith(ByteArrayArgumentConverter.class) byte[] expected) throws InvalidOldFilenameException {
+		FilePathMigration migration = new FilePathMigration(oldPath, oldCanonicalName);
+		Assertions.assertArrayEquals(expected, migration.getDecodedCiphertext());
+	}
+
+	@ParameterizedTest(name = "getDecodedCiphertext() throws InvalidOldFilenameException for {0}")
+	@ValueSource(strings = {
+		"ORSXG5=A",
+		"ORSX=G5A"
+	})
+	public void testMalformedGetDecodedCiphertext(String oldCanonicalName) {
+		FilePathMigration migration = new FilePathMigration(oldPath, oldCanonicalName);
+
+		InvalidOldFilenameException e = Assertions.assertThrows(InvalidOldFilenameException.class, () -> {
+			migration.getDecodedCiphertext();
+		});
+		Assertions.assertTrue(e.getMessage().contains(oldPath.toString()));
+	}
 
 	@ParameterizedTest(name = "getNewInflatedName() expected to be {1} for {0}")
 	@CsvSource({
 			"ORSXG5A=,dGVzdA==.c9r",
 			"0ORSXG5A=,dGVzdA==.c9r",
 			"1SORSXG5A=,dGVzdA==.c9r",
 	})
-	public void testGetNewInflatedName(String oldCanonicalName, String expectedResult) {
+	public void testGetNewInflatedName(String oldCanonicalName, String expectedResult) throws InvalidOldFilenameException {
 		FilePathMigration migration = new FilePathMigration(oldPath, oldCanonicalName);
 
 		Assertions.assertEquals(expectedResult, migration.getNewInflatedName());
@@ -63,7 +92,7 @@ public void testGetNewInflatedName(String oldCanonicalName, String expectedResul
 			"ORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSQ====,dGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRlc3QgdGVzdCB0ZXN0IHRl.c9r",
 			"ORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG===,30xtS3YjsiMJRwu1oAVc_0S2aAU=.c9s",
 	})
-	public void testGetNewDeflatedName(String oldCanonicalName, String expectedResult) {
+	public void testGetNewDeflatedName(String oldCanonicalName, String expectedResult) throws InvalidOldFilenameException {
 		FilePathMigration migration = new FilePathMigration(oldPath, oldCanonicalName);
 
 		Assertions.assertEquals(expectedResult, migration.getNewDeflatedName());
@@ -104,7 +133,7 @@ public void testIsSymlink(String oldCanonicalName, boolean expectedResult) {
 			"ORSXG5A=,'_1',dGVzdA==_1.c9r",
 			"ORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG5BAORSXG===,'_123',30xtS3YjsiMJRwu1oAVc_0S2aAU=_123.c9s/contents.c9r",
 	})
-	public void testGetTargetPath(String oldCanonicalName, String attemptSuffix, String expected) {
+	public void testGetTargetPath(String oldCanonicalName, String attemptSuffix, String expected) throws InvalidOldFilenameException {
 		Path old = Paths.get("/tmp/foo");
 		FilePathMigration migration = new FilePathMigration(old, oldCanonicalName);
 
@@ -325,4 +354,12 @@ public void testMigrateShortened(String oldPathStr, String metadataFilePath, Str
 
 	}
 
+	public static class ByteArrayArgumentConverter extends SimpleArgumentConverter {
+
+		@Override
+		protected Object convert(Object source, Class<?> targetType) throws ArgumentConversionException {
+			Assertions.assertEquals(byte[].class, targetType, "Can only convert to byte[]");
+			return Base64.getUrlDecoder().decode(String.valueOf(source));
+		}
+	}
 }