Skip to content

Commit 1ff1f1d

Browse files
committed
Merge branch 'develop' into feature/fs-notify
# Conflicts: # src/main/java/org/cryptomator/cryptofs/dir/C9rConflictResolver.java # src/test/java/org/cryptomator/cryptofs/dir/C9rConflictResolverTest.java
2 parents fb5b302 + 72764a0 commit 1ff1f1d

File tree

13 files changed

+263
-56
lines changed

13 files changed

+263
-56
lines changed

pom.xml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
<!-- test dependencies -->
2929
<junit.jupiter.version>5.11.3</junit.jupiter.version>
30+
<jmh.version>1.37</jmh.version>
3031
<mockito.version>5.14.2</mockito.version>
3132
<hamcrest.version>3.0</hamcrest.version>
3233
<jimfs.version>1.3.0</jimfs.version>
@@ -123,6 +124,18 @@
123124
<version>${junit.jupiter.version}</version>
124125
<scope>test</scope>
125126
</dependency>
127+
<dependency>
128+
<groupId>org.openjdk.jmh</groupId>
129+
<artifactId>jmh-core</artifactId>
130+
<version>${jmh.version}</version>
131+
<scope>test</scope>
132+
</dependency>
133+
<dependency>
134+
<groupId>org.openjdk.jmh</groupId>
135+
<artifactId>jmh-generator-annprocess</artifactId>
136+
<version>${jmh.version}</version>
137+
<scope>provided</scope> <!-- only required during compilation -->
138+
</dependency>
126139
<dependency>
127140
<groupId>org.mockito</groupId>
128141
<artifactId>mockito-core</artifactId>
@@ -163,6 +176,11 @@
163176
<artifactId>dagger-compiler</artifactId>
164177
<version>${dagger.version}</version>
165178
</path>
179+
<path>
180+
<groupId>org.openjdk.jmh</groupId>
181+
<artifactId>jmh-generator-annprocess</artifactId>
182+
<version>${jmh.version}</version>
183+
</path>
166184
</annotationProcessorPaths>
167185
</configuration>
168186
</plugin>

src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import org.cryptomator.cryptofs.common.BackupHelper;
44
import org.cryptomator.cryptofs.common.Constants;
5-
import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker;
65
import org.cryptomator.cryptolib.api.Cryptor;
76
import org.cryptomator.cryptolib.api.CryptorProvider;
87
import org.cryptomator.cryptolib.api.Masterkey;
@@ -20,8 +19,6 @@
2019
import java.nio.file.NoSuchFileException;
2120
import java.nio.file.Path;
2221
import java.security.SecureRandom;
23-
import java.util.EnumSet;
24-
import java.util.Set;
2522
import java.util.concurrent.ConcurrentHashMap;
2623
import java.util.concurrent.ConcurrentMap;
2724

src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public DirectoryIdLoader() {
2424
}
2525

2626
@Override
27-
public String load(Path dirFilePath) throws UncheckedIOException {
27+
public String load(Path dirFilePath) throws IOException {
2828
try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ);
2929
InputStream in = Channels.newInputStream(ch)) {
3030
long size = ch.size();
@@ -40,8 +40,6 @@ public String load(Path dirFilePath) throws UncheckedIOException {
4040
}
4141
} catch (NoSuchFileException e) {
4242
return UUID.randomUUID().toString();
43-
} catch (IOException e) {
44-
throw new UncheckedIOException(e);
4543
}
4644
}
4745

src/main/java/org/cryptomator/cryptofs/DirectoryIdProvider.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,19 @@
1111
import com.github.benmanes.caffeine.cache.Cache;
1212
import com.github.benmanes.caffeine.cache.Caffeine;
1313
import com.github.benmanes.caffeine.cache.LoadingCache;
14+
import org.slf4j.Logger;
15+
import org.slf4j.LoggerFactory;
1416

1517
import javax.inject.Inject;
1618
import java.io.IOException;
1719
import java.io.UncheckedIOException;
1820
import java.nio.file.Path;
21+
import java.util.concurrent.CompletionException;
1922

2023
@CryptoFileSystemScoped
2124
class DirectoryIdProvider {
2225

26+
private static final Logger LOG = LoggerFactory.getLogger(DirectoryIdProvider.class);
2327
private static final int MAX_CACHE_SIZE = 5000;
2428

2529
private final LoadingCache<Path, String> ids;
@@ -32,8 +36,9 @@ public DirectoryIdProvider(DirectoryIdLoader directoryIdLoader) {
3236
public String load(Path dirFilePath) throws IOException {
3337
try {
3438
return ids.get(dirFilePath);
35-
} catch (UncheckedIOException e) {
36-
throw new IOException("Failed to load contents of directory file at path " + dirFilePath, e);
39+
} catch (CompletionException e) {
40+
LOG.warn("Failed to load directory id from {}", dirFilePath, e.getCause());
41+
throw (IOException) e.getCause();
3742
}
3843
}
3944

src/main/java/org/cryptomator/cryptofs/Symlinks.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ private void assertIsSymlink(CryptoPath cleartextPath, Path ciphertextSymlinkFil
9090

9191
/**
9292
* Gets the target of a symlink. Recursive, if the target is a symlink itself.
93+
*
9394
* @param cleartextPath A cleartext path. Might be a symlink, otherwise this method is no-op.
9495
* @return The resolved cleartext path. Might be the same as <code>cleartextPath</code> if it wasn't a symlink in the first place.
9596
* @throws IOException

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

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -85,44 +85,65 @@ Stream<Node> resolveConflict(Node conflicting, Path canonicalPath) throws IOExce
8585
resolved.extractedCiphertext = conflicting.extractedCiphertext;
8686
return Stream.of(resolved);
8787
} else {
88-
return Stream.of(renameConflictingFile(canonicalPath, conflictingPath, conflicting.cleartextName));
88+
return renameConflictingFile(canonicalPath, conflicting);
8989
}
9090
}
9191

9292
/**
9393
* Resolves a conflict by renaming the conflicting file.
9494
*
9595
* @param canonicalPath The path to the original (conflict-free) file.
96-
* @param conflictingPath The path to the potentially conflicting file.
97-
* @param cleartext The cleartext name of the conflicting file.
98-
* @return The newly created Node after renaming the conflicting file.
99-
* @throws IOException
96+
* @param conflicting The conflicting file.
97+
* @return The newly created Node if rename succeeded or an empty stream otherwise.
98+
* @throws IOException If an unexpected I/O exception occurs during rename
10099
*/
101-
private Node renameConflictingFile(Path canonicalPath, Path conflictingPath, String cleartext) throws IOException {
100+
private Stream<Node> renameConflictingFile(Path canonicalPath, Node conflicting) throws IOException {
102101
assert Files.exists(canonicalPath);
103-
final int beginOfFileExtension = cleartext.lastIndexOf('.');
104-
final String fileExtension = (beginOfFileExtension > 0) ? cleartext.substring(beginOfFileExtension) : "";
105-
final String basename = (beginOfFileExtension > 0) ? cleartext.substring(0, beginOfFileExtension) : cleartext;
106-
final String lengthRestrictedBasename = basename.substring(0, Math.min(basename.length(), maxCleartextFileNameLength - fileExtension.length() - 5)); // 5 chars for conflict suffix " (42)"
107-
String alternativeCleartext;
108-
String alternativeCiphertext;
109-
String alternativeCiphertextName;
110-
Path alternativePath;
111-
int i = 1;
112-
do {
113-
alternativeCleartext = lengthRestrictedBasename + " (" + i++ + ")" + fileExtension;
102+
assert conflicting.fullCiphertextFileName.endsWith(Constants.CRYPTOMATOR_FILE_SUFFIX);
103+
assert conflicting.fullCiphertextFileName.contains(conflicting.extractedCiphertext);
104+
105+
final String cleartext = conflicting.cleartextName;
106+
final int beginOfCleartextExt = cleartext.lastIndexOf('.');
107+
final String cleartextFileExt = (beginOfCleartextExt > 0) ? cleartext.substring(beginOfCleartextExt) : "";
108+
final String cleartextBasename = (beginOfCleartextExt > 0) ? cleartext.substring(0, beginOfCleartextExt) : cleartext;
109+
110+
// let's assume that some the sync conflict string is added at the end of the file name, but before .c9r:
111+
final int endOfCiphertext = conflicting.fullCiphertextFileName.indexOf(conflicting.extractedCiphertext) + conflicting.extractedCiphertext.length();
112+
final String originalConflictSuffix = conflicting.fullCiphertextFileName.substring(endOfCiphertext, conflicting.fullCiphertextFileName.length() - Constants.CRYPTOMATOR_FILE_SUFFIX.length());
113+
114+
// split available maxCleartextFileNameLength between basename, conflict suffix, and file extension:
115+
final int netCleartext = maxCleartextFileNameLength - cleartextFileExt.length(); // file extension must be preserved
116+
final String conflictSuffix = originalConflictSuffix.substring(0, Math.min(originalConflictSuffix.length(), netCleartext / 2)); // max 50% of available space
117+
final int conflictSuffixLen = Math.max(4, conflictSuffix.length()); // prefer to use original conflict suffix, but reserver at least 4 chars for numerical fallback: " (9)"
118+
final String lengthRestrictedBasename = cleartextBasename.substring(0, Math.min(cleartextBasename.length(), netCleartext - conflictSuffixLen)); // remaining space for basename
119+
120+
// attempt to use original conflict suffix:
121+
String alternativeCleartext = lengthRestrictedBasename + conflictSuffix + cleartextFileExt;
122+
String alternativeCiphertext = cryptor.fileNameCryptor().encryptFilename(BaseEncoding.base64Url(), alternativeCleartext, dirId);
123+
String alternativeCiphertextName = alternativeCiphertext + Constants.CRYPTOMATOR_FILE_SUFFIX;
124+
Path alternativePath = canonicalPath.resolveSibling(alternativeCiphertextName);
125+
126+
// fallback to number conflic suffix, if file with alternative path already exists:
127+
for (int i = 1; i < 10 && Files.exists(alternativePath); i++) {
128+
alternativeCleartext = lengthRestrictedBasename + " (" + i + ")" + cleartextFileExt;
114129
alternativeCiphertext = cryptor.fileNameCryptor().encryptFilename(BaseEncoding.base64Url(), alternativeCleartext, dirId);
115130
alternativeCiphertextName = alternativeCiphertext + Constants.CRYPTOMATOR_FILE_SUFFIX;
116131
alternativePath = canonicalPath.resolveSibling(alternativeCiphertextName);
117-
} while (Files.exists(alternativePath));
132+
}
133+
118134
assert alternativeCiphertextName.length() <= maxC9rFileNameLength;
119-
LOG.info("Moving conflicting file {} to {}", conflictingPath, alternativePath);
120-
Files.move(conflictingPath, alternativePath, StandardCopyOption.ATOMIC_MOVE);
135+
if (Files.exists(alternativePath)) {
136+
LOG.warn("Failed finding alternative name for {}. Keeping original name.", conflicting.ciphertextPath);
137+
return Stream.empty();
138+
}
139+
140+
Files.move(conflicting.ciphertextPath, alternativePath, StandardCopyOption.ATOMIC_MOVE);
141+
LOG.info("Renamed conflicting file {} to {}...", conflicting.ciphertextPath, alternativePath);
121142
Node node = new Node(alternativePath);
122143
node.cleartextName = alternativeCleartext;
123144
node.extractedCiphertext = alternativeCiphertext;
124-
eventConsumer.accept(new ConflictResolvedEvent(cleartextPath.resolve(cleartext), canonicalPath, cleartextPath.resolve(alternativeCleartext), alternativePath));
125-
return node;
145+
eventConsumer.accept(new ConflictResolvedEvent(cleartextPath.resolve(cleartext), conflicting.ciphertextPath, cleartextPath.resolve(alternativeCleartext), alternativePath));
146+
return Stream.of(node);
126147
}
127148

128149

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
class C9rDecryptor {
2121

2222
// visible for testing:
23-
static final Pattern BASE64_PATTERN = Pattern.compile("([a-zA-Z0-9-_]{4})*[a-zA-Z0-9-_]{20}[a-zA-Z0-9-_=]{4}");
23+
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}==)");
2424
private static final CharMatcher DELIM_MATCHER = CharMatcher.anyOf("_-");
2525

2626
private final Cryptor cryptor;
@@ -36,11 +36,7 @@ public Stream<Node> process(Node node) {
3636
String basename = StringUtils.removeEnd(node.fullCiphertextFileName, Constants.CRYPTOMATOR_FILE_SUFFIX);
3737
Matcher matcher = BASE64_PATTERN.matcher(basename);
3838
Optional<Node> match = extractCiphertext(node, matcher, 0, basename.length());
39-
if (match.isPresent()) {
40-
return Stream.of(match.get());
41-
} else {
42-
return Stream.empty();
43-
}
39+
return match.stream();
4440
}
4541

4642
private Optional<Node> extractCiphertext(Node node, Matcher matcher, int start, int end) {

src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFileModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public AtomicReference<Instant> provideLastModifiedDate(@OriginalOpenFilePath Pa
4646
@OpenFileSize
4747
public AtomicLong provideFileSize() {
4848
// will be initialized when first creating a FileChannel. See OpenCryptoFile#size()
49-
return new AtomicLong(-1l);
49+
return new AtomicLong(-1L);
5050
}
5151

5252
private Optional<BasicFileAttributes> readBasicAttributes(Path path) {

src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,10 @@ public void testIOExceptionWhenExistingFileIsEmpty() throws IOException {
8484
when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel);
8585
when(channel.size()).thenReturn(0l);
8686

87-
UncheckedIOException e = Assertions.assertThrows(UncheckedIOException.class, () -> {
87+
var ioException = Assertions.assertThrows(IOException.class, () -> {
8888
inTest.load(dirFilePath);
8989
});
90-
MatcherAssert.assertThat(e.getCause().getMessage(), containsString("Invalid, empty directory file"));
90+
MatcherAssert.assertThat(ioException.getMessage(), containsString("Invalid, empty directory file"));
9191
}
9292

9393
@Test
@@ -96,10 +96,10 @@ public void testIOExceptionWhenExistingFileIsTooLarge() throws IOException {
9696
when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel);
9797
when(channel.size()).thenReturn((long) Integer.MAX_VALUE);
9898

99-
UncheckedIOException e = Assertions.assertThrows(UncheckedIOException.class, () -> {
99+
var ioException = Assertions.assertThrows(IOException.class, () -> {
100100
inTest.load(dirFilePath);
101101
});
102-
MatcherAssert.assertThat(e.getCause().getMessage(), containsString("Unexpectedly large directory file"));
102+
MatcherAssert.assertThat(ioException.getMessage(), containsString("Unexpectedly large directory file"));
103103
}
104104

105105
}

src/test/java/org/cryptomator/cryptofs/DirectoryIdProviderTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,14 @@ public void testLoadInvokesLoader() throws IOException {
3939
}
4040

4141
@Test
42-
public void testIOExceptionFromLoaderIsWrappedAndRethrown() {
42+
public void testIOExceptionFromLoaderIsRethrown() throws IOException {
4343
IOException originalIoException = new IOException();
44-
when(loader.load(aPath)).thenThrow(new UncheckedIOException(originalIoException));
44+
when(loader.load(aPath)).thenThrow(originalIoException);
4545

4646
IOException e = Assertions.assertThrows(IOException.class, () -> {
4747
inTest.load(aPath);
4848
});
49-
Assertions.assertTrue(e.getCause() instanceof UncheckedIOException);
50-
Assertions.assertEquals(originalIoException, e.getCause().getCause());
49+
Assertions.assertSame(originalIoException, e);
5150
}
5251

5352
@Test

0 commit comments

Comments
 (0)