Use Conflict Suffixes from Ciphertext Files

pom.xml

@@ -27,6 +27,7 @@
 
 		<!-- test dependencies -->
 		<junit.jupiter.version>5.11.3</junit.jupiter.version>
+		<jmh.version>1.37</jmh.version>
 		<mockito.version>5.14.2</mockito.version>
 		<hamcrest.version>3.0</hamcrest.version>
 		<jimfs.version>1.3.0</jimfs.version>
@@ -123,6 +124,18 @@
 			<version>${junit.jupiter.version}</version>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.openjdk.jmh</groupId>
+			<artifactId>jmh-core</artifactId>
+			<version>${jmh.version}</version>
+			<scope>test</scope>
+		</dependency>
+		<dependency>
+			<groupId>org.openjdk.jmh</groupId>
+			<artifactId>jmh-generator-annprocess</artifactId>
+			<version>${jmh.version}</version>
+			<scope>provided</scope> <!-- only required during compilation -->
+		</dependency>
 		<dependency>
 			<groupId>org.mockito</groupId>
 			<artifactId>mockito-core</artifactId>
@@ -163,6 +176,11 @@
 							<artifactId>dagger-compiler</artifactId>
 							<version>${dagger.version}</version>
 						</path>
+						<path>
+							<groupId>org.openjdk.jmh</groupId>
+							<artifactId>jmh-generator-annprocess</artifactId>
+							<version>${jmh.version}</version>
+						</path>
 					</annotationProcessorPaths>
 				</configuration>
 			</plugin>

src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java

@@ -61,7 +61,7 @@ public Stream<Node> process(Node node) {
 				Path canonicalPath = node.ciphertextPath.resolveSibling(canonicalCiphertextFileName);
 				return resolveConflict(node, canonicalPath);
 			} catch (IOException e) {
-				LOG.error("Failed to resolve conflict for " + node.ciphertextPath, e);
+				LOG.error("Failed to resolve conflict for {}", node.ciphertextPath, e);
 				return Stream.empty();
 			}
 		}
@@ -75,39 +75,52 @@ private Stream<Node> resolveConflict(Node conflicting, Path canonicalPath) throw
 			resolved.extractedCiphertext = conflicting.extractedCiphertext;
 			return Stream.of(resolved);
 		} else {
-			return Stream.of(renameConflictingFile(canonicalPath, conflictingPath, conflicting.cleartextName));
+			return Stream.of(renameConflictingFile(canonicalPath, conflicting));
 		}
 	}
 
 	/**
 	 * Resolves a conflict by renaming the conflicting file.
 	 *
 	 * @param canonicalPath The path to the original (conflict-free) file.
-	 * @param conflictingPath The path to the potentially conflicting file.
-	 * @param cleartext The cleartext name of the conflicting file.
+	 * @param conflicting The conflicting file.
 	 * @return The newly created Node after renaming the conflicting file.
 	 * @throws IOException
 	 */
-	private Node renameConflictingFile(Path canonicalPath, Path conflictingPath, String cleartext) throws IOException {
+	private Node renameConflictingFile(Path canonicalPath, Node conflicting) throws IOException {
 		assert Files.exists(canonicalPath);
-		final int beginOfFileExtension = cleartext.lastIndexOf('.');
-		final String fileExtension = (beginOfFileExtension > 0) ? cleartext.substring(beginOfFileExtension) : "";
-		final String basename = (beginOfFileExtension > 0) ? cleartext.substring(0, beginOfFileExtension) : cleartext;
-		final String lengthRestrictedBasename = basename.substring(0, Math.min(basename.length(), maxCleartextFileNameLength - fileExtension.length() - 5)); // 5 chars for conflict suffix " (42)"
-		String alternativeCleartext;
-		String alternativeCiphertext;
-		String alternativeCiphertextName;
-		Path alternativePath;
-		int i = 1;
-		do {
-			alternativeCleartext = lengthRestrictedBasename + " (" + i++ + ")" + fileExtension;
+		assert conflicting.fullCiphertextFileName.endsWith(Constants.CRYPTOMATOR_FILE_SUFFIX);
+		assert conflicting.fullCiphertextFileName.contains(conflicting.extractedCiphertext);
+
+		final String cleartext = conflicting.cleartextName;
+		final int beginOfCleartextExt = cleartext.lastIndexOf('.');
+		final String cleartextFileExt = (beginOfCleartextExt > 0) ? cleartext.substring(beginOfCleartextExt) : "";
+		final String cleartextBasename = (beginOfCleartextExt > 0) ? cleartext.substring(0, beginOfCleartextExt) : cleartext;
+
+		// let's assume that some the sync conflict string is added at the end of the file name, but before .c9r:
+		final int endOfCiphertext = conflicting.fullCiphertextFileName.indexOf(conflicting.extractedCiphertext) + conflicting.extractedCiphertext.length();
+		final String originalConflictSuffix = conflicting.fullCiphertextFileName.substring(endOfCiphertext, conflicting.fullCiphertextFileName.length() - Constants.CRYPTOMATOR_FILE_SUFFIX.length());
+
+		// split available maxCleartextFileNameLength between basename, conflict suffix, and file extension:
+		final int netCleartext = maxCleartextFileNameLength - cleartextFileExt.length(); // file extension must be preserved
+		final String conflictSuffix = originalConflictSuffix.substring(0, Math.min(originalConflictSuffix.length(), netCleartext / 2)); // max 50% of available space
+		final int conflictSuffixLen = Math.max(5, conflictSuffix.length()); // prefer to use original conflict suffix, but reserver at least 5 chars for numerical fallback: " (42)"
+		final String lengthRestrictedBasename = cleartextBasename.substring(0, Math.min(cleartextBasename.length(), netCleartext - conflictSuffixLen)); // remaining space for basename
+
+		String alternativeCleartext = lengthRestrictedBasename + conflictSuffix + cleartextFileExt;
+		String alternativeCiphertext = cryptor.fileNameCryptor().encryptFilename(BaseEncoding.base64Url(), alternativeCleartext, dirId);
+		String alternativeCiphertextName = alternativeCiphertext + Constants.CRYPTOMATOR_FILE_SUFFIX;
+		Path alternativePath = canonicalPath.resolveSibling(alternativeCiphertextName);
+		for (int i = 1; Files.exists(alternativePath); i++) {
+			alternativeCleartext = lengthRestrictedBasename + " (" + i + ")" + cleartextFileExt;
 			alternativeCiphertext = cryptor.fileNameCryptor().encryptFilename(BaseEncoding.base64Url(), alternativeCleartext, dirId);
 			alternativeCiphertextName = alternativeCiphertext + Constants.CRYPTOMATOR_FILE_SUFFIX;
 			alternativePath = canonicalPath.resolveSibling(alternativeCiphertextName);
-		} while (Files.exists(alternativePath));
+		}
+
 		assert alternativeCiphertextName.length() <= maxC9rFileNameLength;
-		LOG.info("Moving conflicting file {} to {}", conflictingPath, alternativePath);
-		Files.move(conflictingPath, alternativePath, StandardCopyOption.ATOMIC_MOVE);
+		LOG.info("Moving conflicting file {} to {}", conflicting.ciphertextPath, alternativePath);
+		Files.move(conflicting.ciphertextPath, alternativePath, StandardCopyOption.ATOMIC_MOVE);
 		Node node = new Node(alternativePath);
 		node.cleartextName = alternativeCleartext;
 		node.extractedCiphertext = alternativeCiphertext;

src/main/java/org/cryptomator/cryptofs/dir/C9rDecryptor.java

@@ -20,7 +20,7 @@
 class C9rDecryptor {
 
 	// visible for testing:
-	static final Pattern BASE64_PATTERN = Pattern.compile("([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_]{20}[a-zA-Z0-9-_=]{4}");
+	static final Pattern BASE64_PATTERN = Pattern.compile("[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*(?:[a-zA-Z0-9-_]{4}|[a-zA-Z0-9-_]{3}=|[a-zA-Z0-9-_]{2}==)");
 	private static final CharMatcher DELIM_MATCHER = CharMatcher.anyOf("_-");
 
 	private final Cryptor cryptor;
@@ -36,11 +36,7 @@ public Stream<Node> process(Node node) {
 		String basename = StringUtils.removeEnd(node.fullCiphertextFileName, Constants.CRYPTOMATOR_FILE_SUFFIX);
 		Matcher matcher = BASE64_PATTERN.matcher(basename);
 		Optional<Node> match = extractCiphertext(node, matcher, 0, basename.length());
-		if (match.isPresent()) {
-			return Stream.of(match.get());
-		} else {
-			return Stream.empty();
-		}
+		return match.stream();
 	}
 
 	private Optional<Node> extractCiphertext(Node node, Matcher matcher, int start, int end) {

src/test/java/org/cryptomator/cryptofs/dir/Base64UrlRegexTest.java

@@ -0,0 +1,136 @@
+package org.cryptomator.cryptofs.dir;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Level;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Needs to be compiled via maven as the JMH annotation processor needs to do stuff...
+ */
+public class Base64UrlRegexTest {
+
+	private static final String[] TEST_INPUTS = { //
+			"aaaaBBBBccccDDDDeeeeFFFF",
+			"aaaaBBBBccccDDDDeeeeFFF=",
+			"aaaaBBBBccccDDDDeeeeFF==",
+			"aaaaBBBBcc0123456789_-==",
+			"aaaaBBBBccccDDDDeeeeFFFFggggHH==",
+			"-3h6-FSsYhMCJHSAV9cjJ89F7R73b0zIB4vvO01b7uWq28fWioRagWpMv-w0MA-2ORjbShuv", //
+			"iJYw7QpVjKTDv_b6H5jLkauVrnPcGbV9DnIG426EBzjlYmRuJDX5cjFJLmTDA7EOEmo5rAHT3Jc=", //
+			"PnBpirtqhCKm9hE1341rxkqdASfyYJqNHROxR1xJWDH6TGbeqqXn_sr2Rk5zzVpSbufkiqZH9a==", //
+			"S8cmirjkHBHbIJZXExbFyyTOA8r6TvTPddK_sdQZpcE3RCMDI0mo9w2f53DSkqT0xRf1xcrmxvU=" //
+	};
+
+	private static final String[] TEST_INVALID_INPUTS = { //
+			"aaaaBBBBccccDDDDeeee", // too short
+			"aaaaBBBBccccDDDDeeeeFFF", // unpadded
+			"====BBBBccccDDDDeeeeFFFF", // padding not at end
+			"????BBBBccccDDDDeeeeFFFF", // invalid chars
+			"conflict aaaaBBBBccccDDDDeeeeFFFF", // only a partial match
+			"aaaaBBBBccccDDDDeeeeFFFF conflict", // only a partial match
+			"-3h6-FSsYhMCJHSAV9cjJ89F7R73b0zIB4vvO01b7uWq28fWioRagWpMv-w0MA-2ORjbShu", // not multiple of four
+			"=iJYw7QpVjKTDv_b6H5jLkauVrnPcGbV9DnIG426EBzjlYmRuJDX5cjFJLmTDA7EOEmo5rAHT3J=", // padding in wrong position
+			"PnBp.irtqhCKm9hE1341rxkqdASfyYJqNHROxR1xJWDH6TGbeqqXn_sr2Rk5zzVpSbufkiqZH9a==", // invalid character
+	};
+
+	@ParameterizedTest
+	@ValueSource(strings = {
+			"([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_]{20}[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*(?:[a-zA-Z0-9-_]{4}|[a-zA-Z0-9-_]{3}=|[a-zA-Z0-9-_]{2}==)", // most strict
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_=]{4})+", // most permissive
+	})
+	public void testValidBase64Pattern(String patternString) {
+		Pattern pattern = Pattern.compile(patternString);
+		for (String input : TEST_INPUTS) {
+			Matcher matcher = pattern.matcher(input);
+			Assertions.assertTrue(matcher.matches(), "pattern does not match " + input);
+		}
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {
+			"([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_]{20}[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*(?:[a-zA-Z0-9-_]{4}|[a-zA-Z0-9-_]{3}=|[a-zA-Z0-9-_]{2}==)", // most strict
+			"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_=]{4})+", // most permissive
+	})
+	public void testInvalidInputs(String patternString) {
+		Pattern pattern = Pattern.compile(patternString);
+		for (String input : TEST_INVALID_INPUTS) {
+			Matcher matcher = pattern.matcher(input);
+			Assertions.assertFalse(matcher.matches(), "pattern matches " + input);
+		}
+	}
+
+	@Test
+	@Disabled // only run manually
+	public void runBenchmarks() throws RunnerException {
+		// Taken from http://stackoverflow.com/a/30486197/4014509:
+		Options opt = new OptionsBuilder().include(RegexBenchmark.class.getSimpleName()).build();
+		new Runner(opt).run();
+	}
+
+	@State(Scope.Thread)
+	@Fork(3)
+	@Warmup(iterations = 2, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+	@Measurement(iterations = 3, time = 500, timeUnit = TimeUnit.MILLISECONDS)
+	@BenchmarkMode(value = {Mode.AverageTime})
+	@OutputTimeUnit(TimeUnit.MICROSECONDS)
+	public static class RegexBenchmark {
+
+		// Base64 regex pattern
+		@Param({
+				"([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_]{20}[a-zA-Z0-9-_=]{4}",
+				"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+				"[a-zA-Z0-9-_]{20}([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+				"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_=]{4}",
+				"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_]{4})*(?:[a-zA-Z0-9-_]{4}|[a-zA-Z0-9-_]{3}=|[a-zA-Z0-9-_]{2}==)", // most strict
+				"[a-zA-Z0-9-_]{20}(?:[a-zA-Z0-9-_=]{4})+", // most permissive
+		})
+		private String patternString;
+
+		private Pattern pattern;
+
+		@Setup(Level.Trial)
+		public void compilePattern() {
+			pattern = Pattern.compile(patternString);
+		}
+
+		@Benchmark
+		public void testPattern(Blackhole bh) {
+			for (String input : TEST_INPUTS) {
+				Matcher matcher = pattern.matcher(input);
+				bh.consume(matcher.matches());
+			}
+		}
+
+	}
+}

src/test/java/org/cryptomator/cryptofs/dir/C9rConflictResolverTest.java

@@ -31,7 +31,7 @@ public void setup() {
 		fileNameCryptor = Mockito.mock(FileNameCryptor.class);
 		vaultConfig = Mockito.mock(VaultConfig.class);
 		Mockito.when(cryptor.fileNameCryptor()).thenReturn(fileNameCryptor);
-		Mockito.when(vaultConfig.getShorteningThreshold()).thenReturn(44); // results in max cleartext size = 14
+		Mockito.when(vaultConfig.getShorteningThreshold()).thenReturn(84); // results in max cleartext size = 44
 		conflictResolver = new C9rConflictResolver(cryptor, "foo", vaultConfig);
 	}
 
@@ -60,10 +60,10 @@ public void testResolveHiddenNode(String filename) {
 
 	@Test
 	public void testResolveConflictingFileByChoosingNewName(@TempDir Path dir) throws IOException {
-		Files.createFile(dir.resolve("foo (1).c9r"));
+		Files.createFile(dir.resolve("foo (Created by Alice).c9r"));
 		Files.createFile(dir.resolve("foo.c9r"));
 		Mockito.when(fileNameCryptor.encryptFilename(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn("baz");
-		Node unresolved = new Node(dir.resolve("foo (1).c9r"));
+		Node unresolved = new Node(dir.resolve("foo (Created by Alice).c9r"));
 		unresolved.cleartextName = "bar.txt";
 		unresolved.extractedCiphertext = "foo";
 
@@ -72,26 +72,46 @@ public void testResolveConflictingFileByChoosingNewName(@TempDir Path dir) throw
 
 		Assertions.assertNotEquals(unresolved, resolved);
 		Assertions.assertEquals("baz.c9r", resolved.fullCiphertextFileName);
+		Assertions.assertEquals("bar (Created by Alice).txt", resolved.cleartextName);
+		Assertions.assertTrue(Files.exists(resolved.ciphertextPath));
+		Assertions.assertFalse(Files.exists(unresolved.ciphertextPath));
+	}
+
+	@Test
+	public void testResolveConflictingFileByAddingNumericSuffix(@TempDir Path dir) throws IOException {
+		Files.createFile(dir.resolve("foo (Created by Alice).c9r"));
+		Files.createFile(dir.resolve("foo.c9r"));
+		Files.createFile(dir.resolve("baz.c9r")); // resolved name already occupied, try cux next!
+		Mockito.when(fileNameCryptor.encryptFilename(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn("baz").thenReturn("qux");
+		Node unresolved = new Node(dir.resolve("foo (Created by Alice).c9r"));
+		unresolved.cleartextName = "bar.txt";
+		unresolved.extractedCiphertext = "foo";
+
+		Stream<Node> result = conflictResolver.process(unresolved);
+		Node resolved = result.findAny().get();
+
+		Assertions.assertNotEquals(unresolved, resolved);
+		Assertions.assertEquals("qux.c9r", resolved.fullCiphertextFileName);
 		Assertions.assertEquals("bar (1).txt", resolved.cleartextName);
 		Assertions.assertTrue(Files.exists(resolved.ciphertextPath));
 		Assertions.assertFalse(Files.exists(unresolved.ciphertextPath));
 	}
 
 	@Test
 	public void testResolveConflictingFileByChoosingNewLengthLimitedName(@TempDir Path dir) throws IOException {
-		Files.createFile(dir.resolve("foo (1).c9r"));
+		Files.createFile(dir.resolve("foo (Created by Alice on 2024-01-31).c9r"));
 		Files.createFile(dir.resolve("foo.c9r"));
 		Mockito.when(fileNameCryptor.encryptFilename(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn("baz");
-		Node unresolved = new Node(dir.resolve("foo (1).c9r"));
-		unresolved.cleartextName = "hello world.txt";
+		Node unresolved = new Node(dir.resolve("foo (Created by Alice on 2024-01-31).c9r"));
+		unresolved.cleartextName = "this is a rather long file name.txt";
 		unresolved.extractedCiphertext = "foo";
 
 		Stream<Node> result = conflictResolver.process(unresolved);
 		Node resolved = result.findAny().get();
 
 		Assertions.assertNotEquals(unresolved, resolved);
 		Assertions.assertEquals("baz.c9r", resolved.fullCiphertextFileName);
-		Assertions.assertEquals("hello (1).txt", resolved.cleartextName);
+		Assertions.assertEquals("this is a rather lon (Created by Alice o.txt", resolved.cleartextName);
 		Assertions.assertTrue(Files.exists(resolved.ciphertextPath));
 		Assertions.assertFalse(Files.exists(unresolved.ciphertextPath));
 	}

src/test/java/org/cryptomator/cryptofs/dir/C9rDecryptorTest.java

@@ -34,9 +34,7 @@ public void setup() {
 			"aaaaBBBBccccDDDDeeeeFFFF",
 			"aaaaBBBBccccDDDDeeeeFFF=",
 			"aaaaBBBBccccDDDDeeeeFF==",
-			"aaaaBBBBccccDDDDeeeeF===",
-			"aaaaBBBBccccDDDDeeee====",
-			"aaaaBBBB0123456789-_====",
+			"aaaaBBBBcc0123456789_-==",
 			"aaaaBBBBccccDDDDeeeeFFFFggggHH==",
 	})
 	public void testValidBase64Pattern(String input) {
@@ -47,7 +45,9 @@ public void testValidBase64Pattern(String input) {
 	@ValueSource(strings = {
 			"aaaaBBBBccccDDDDeeee", // too short
 			"aaaaBBBBccccDDDDeeeeFFF", // unpadded
-			"====BBBBccccDDDDeeeeFFFF", // padding not at end
+			"aaaaBBBBccccDDDDeeeeF===", // too much padding
+			"aaaaBBBBccccDDDDeeee====", // too much padding
+			"==aaBBBBccccDDDDeeeeFFFF", // padding not at end
 			"????BBBBccccDDDDeeeeFFFF", // invalid chars
 			"conflict aaaaBBBBccccDDDDeeeeFFFF", // only a partial match
 			"aaaaBBBBccccDDDDeeeeFFFF conflict", // only a partial match