Skip to content

Commit 5adce83

Browse files
infeoinfeo
committed
dedup code
1 parent 582bc77 commit 5adce83

File tree

2 files changed

+44
-66
lines changed

2 files changed

+44
-66
lines changed

‎src/main/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDir.java‎

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88
import org.cryptomator.cryptofs.common.Constants;
99
import org.cryptomator.cryptofs.health.api.DiagnosticResult;
1010
import org.cryptomator.cryptolib.api.AuthenticationFailedException;
11+
import org.cryptomator.cryptolib.api.CryptoException;
1112
import org.cryptomator.cryptolib.api.Cryptor;
1213
import org.cryptomator.cryptolib.api.FileNameCryptor;
1314
import org.cryptomator.cryptolib.api.Masterkey;
14-
import org.cryptomator.cryptolib.common.ByteBuffers;
15-
import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel;
1615
import org.slf4j.Logger;
1716
import org.slf4j.LoggerFactory;
1817

1918
import java.io.IOException;
2019
import java.nio.ByteBuffer;
21-
import java.nio.channels.ByteChannel;
2220
import java.nio.charset.StandardCharsets;
2321
import java.nio.file.FileAlreadyExistsException;
2422
import java.nio.file.Files;
@@ -89,7 +87,7 @@ private void fix(Path pathToVault, VaultConfig config, Cryptor cryptor) throws I
8987
AtomicInteger dirCounter = new AtomicInteger(1);
9088
AtomicInteger symlinkCounter = new AtomicInteger(1);
9189
String longNameSuffix = createClearnameToBeShortened(config.getShorteningThreshold());
92-
Optional<String> dirId = retrieveDirId(orphanedDir, cryptor);
90+
Optional<byte[]> dirId = retrieveDirId(orphanedDir, cryptor);
9391

9492
try (var orphanedContentStream = Files.newDirectoryStream(orphanedDir, this::matchesEncryptedContentPattern)) {
9593
for (Path orphanedResource : orphanedContentStream) {
@@ -180,29 +178,18 @@ CiphertextDirectory prepareStepParent(Path dataDir, Path cipherRecoveryDir, Cryp
180178
}
181179

182180
//visible for testing
183-
Optional<String> retrieveDirId(Path orphanedDir, Cryptor cryptor) {
184-
var dirIdFile = orphanedDir.resolve(Constants.DIR_ID_BACKUP_FILE_NAME);
185-
var dirIdBuffer = ByteBuffer.allocate(36); //a dir id contains at most 36 ascii chars
186-
187-
try (var channel = Files.newByteChannel(dirIdFile, StandardOpenOption.READ); //
188-
var decryptingChannel = createDecryptingReadableByteChannel(channel, cryptor)) {
189-
ByteBuffers.fill(decryptingChannel, dirIdBuffer);
190-
dirIdBuffer.flip();
191-
} catch (IOException e) {
192-
LOG.info("Unable to read {}.", dirIdFile, e);
181+
Optional<byte []> retrieveDirId(Path orphanedDir, Cryptor cryptor) {
182+
try {
183+
byte[] dirId = DirectoryIdBackup.read(cryptor, orphanedDir);
184+
return Optional.of(dirId);
185+
} catch (IOException | CryptoException | IllegalStateException e) {
186+
LOG.info("Unable to retrieve directory id for directory {}", orphanedDir, e);
193187
return Optional.empty();
194188
}
195-
196-
return Optional.of(StandardCharsets.US_ASCII.decode(dirIdBuffer).toString());
197-
}
198-
199-
//exists and visible for testability
200-
DecryptingReadableByteChannel createDecryptingReadableByteChannel(ByteChannel channel, Cryptor cryptor) {
201-
return new DecryptingReadableByteChannel(channel, cryptor, true);
202189
}
203190

204191
//visible for testing
205-
String decryptFileName(Path orphanedResource, boolean isShortened, String dirId, FileNameCryptor cryptor) throws IOException, AuthenticationFailedException {
192+
String decryptFileName(Path orphanedResource, boolean isShortened, byte [] dirId, FileNameCryptor cryptor) throws IOException, AuthenticationFailedException {
206193
final String filenameWithExtension;
207194
if (isShortened) {
208195
filenameWithExtension = Files.readString(orphanedResource.resolve(Constants.INFLATED_FILE_NAME));
@@ -211,7 +198,7 @@ String decryptFileName(Path orphanedResource, boolean isShortened, String dirId,
211198
}
212199

213200
final String filename = filenameWithExtension.substring(0, filenameWithExtension.length() - Constants.CRYPTOMATOR_FILE_SUFFIX.length());
214-
return cryptor.decryptFilename(BaseEncoding.base64Url(), filename, dirId.getBytes(StandardCharsets.UTF_8));
201+
return cryptor.decryptFilename(BaseEncoding.base64Url(), filename, dirId);
215202
}
216203

217204
// visible for testing

‎src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java‎

Lines changed: 34 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,30 @@
66
import org.cryptomator.cryptofs.VaultConfig;
77
import org.cryptomator.cryptofs.common.Constants;
88
import org.cryptomator.cryptolib.api.AuthenticationFailedException;
9+
import org.cryptomator.cryptolib.api.CryptoException;
910
import org.cryptomator.cryptolib.api.Cryptor;
1011
import org.cryptomator.cryptolib.api.FileNameCryptor;
1112
import org.cryptomator.cryptolib.api.Masterkey;
12-
import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel;
1313
import org.junit.jupiter.api.Assertions;
1414
import org.junit.jupiter.api.BeforeEach;
1515
import org.junit.jupiter.api.DisplayName;
1616
import org.junit.jupiter.api.Nested;
1717
import org.junit.jupiter.api.Test;
1818
import org.junit.jupiter.api.io.TempDir;
19+
import org.junit.jupiter.params.ParameterizedTest;
20+
import org.junit.jupiter.params.provider.FieldSource;
1921
import org.mockito.Mockito;
2022

2123
import java.io.IOException;
22-
import java.nio.ByteBuffer;
23-
import java.nio.channels.SeekableByteChannel;
2424
import java.nio.charset.StandardCharsets;
2525
import java.nio.file.FileAlreadyExistsException;
2626
import java.nio.file.Files;
2727
import java.nio.file.Path;
2828
import java.nio.file.StandardOpenOption;
2929
import java.security.MessageDigest;
30+
import java.util.List;
3031
import java.util.Optional;
3132
import java.util.UUID;
32-
import java.util.concurrent.atomic.AtomicInteger;
3333
import java.util.concurrent.atomic.AtomicReference;
3434

3535
public class OrphanContentDirTest {
@@ -227,46 +227,38 @@ class RetrieveDirIdTests {
227227

228228
private OrphanContentDir resultSpy;
229229

230+
static class MyCryptoException extends CryptoException {
231+
232+
}
233+
234+
static List<Throwable> expectedExceptions = List.of(new IOException(), new IllegalStateException(), new MyCryptoException());
235+
230236
@BeforeEach
231237
public void init() {
232238
resultSpy = Mockito.spy(result);
233239
}
234240

235241
@Test
236-
@DisplayName("retrieveDirId extracts directory id of cipher-dir/dirId.c9r")
237-
public void testRetrieveDirIdSuccess() throws IOException {
238-
var dirIdFile = cipherOrphan.resolve(Constants.DIR_ID_BACKUP_FILE_NAME);
239-
var dirId = "random-uuid-with-at-most-36chars";
240-
241-
Files.writeString(dirIdFile, dirId, StandardCharsets.US_ASCII, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
242-
DecryptingReadableByteChannel dirIdReadChannel = Mockito.mock(DecryptingReadableByteChannel.class);
243-
244-
Mockito.doReturn(dirIdReadChannel).when(resultSpy).createDecryptingReadableByteChannel(Mockito.any(), Mockito.eq(cryptor));
245-
AtomicInteger readBytesInMockedChannel = new AtomicInteger(0);
246-
//in every invocation the channel position is updated, simulating a stateful channel
247-
Mockito.doAnswer(invocationOnMock -> {
248-
ByteBuffer buf = invocationOnMock.getArgument(0);
249-
try (SeekableByteChannel channel = Files.newByteChannel(dirIdFile, StandardOpenOption.READ)) {
250-
channel.position(readBytesInMockedChannel.get());
251-
readBytesInMockedChannel.getAndSet(channel.read(buf));
252-
return readBytesInMockedChannel.get();
253-
}
254-
}).when(dirIdReadChannel).read(Mockito.any());
255-
256-
Mockito.when(fileNameCryptor.hashDirectoryId(dirId)).thenReturn("333333");
257-
258-
var maybeDirId = resultSpy.retrieveDirId(cipherOrphan, cryptor);
259-
260-
Assertions.assertTrue(maybeDirId.isPresent());
261-
Assertions.assertEquals(dirId, maybeDirId.get());
242+
@DisplayName("Successful reading dirId from backup file")
243+
public void success() {
244+
var dirId = new byte[]{'f', 'o', 'o'};
245+
try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) {
246+
dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenReturn(dirId);
247+
var result = resultSpy.retrieveDirId(cipherOrphan, cryptor);
248+
Assertions.assertTrue(result.isPresent());
249+
Assertions.assertArrayEquals(dirId, result.get());
250+
}
262251
}
263252

264-
@Test
265-
@DisplayName("retrieveDirId returns an empty optional if cipher-dir/dirId.c9r cannot be read")
266-
public void testRetrieveDirIdIOExceptionReadingFile() throws IOException {
267-
var notExistingResult = resultSpy.retrieveDirId(cipherOrphan, cryptor);
268-
269-
Assertions.assertTrue(notExistingResult.isEmpty());
253+
@ParameterizedTest
254+
@DisplayName("retrieveDirId returns an empty optional on any exception")
255+
@FieldSource("expectedExceptions")
256+
public void testRetrieveDirIdIOExceptionReadingFile(Throwable t) throws IOException {
257+
try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) {
258+
dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenThrow(t);
259+
var notExistingResult = resultSpy.retrieveDirId(cipherOrphan, cryptor);
260+
Assertions.assertTrue(notExistingResult.isEmpty());
261+
}
270262
}
271263

272264
}
@@ -283,7 +275,7 @@ void testRestoreFilenameNormalSuccess() throws IOException {
283275
//by using Mockito.eq() in filename parameter Mockito.verfiy() not necessary
284276
Mockito.when(fileNameCryptor.decryptFilename(Mockito.any(), Mockito.eq("orphan"), Mockito.any())).thenReturn("theTrueName.txt");
285277

286-
String decryptedFile = result.decryptFileName(oldCipherPath, false, "someDirId", fileNameCryptor);
278+
String decryptedFile = result.decryptFileName(oldCipherPath, false, new byte[]{}, fileNameCryptor);
287279

288280
Assertions.assertEquals("theTrueName.txt", decryptedFile);
289281
}
@@ -299,7 +291,7 @@ void testRestoreFilenameShortenedSuccess() throws IOException {
299291
//by using Mockito.eq() in filename parameter Mockito.verfiy() not necessary
300292
Mockito.when(fileNameCryptor.decryptFilename(Mockito.any(), Mockito.eq("OrphanWithLongestName"), Mockito.any())).thenReturn("theRealLongName.txt");
301293

302-
String decryptedFile = result.decryptFileName(oldCipherPath, true, "someDirId", fileNameCryptor);
294+
String decryptedFile = result.decryptFileName(oldCipherPath, true, new byte[]{}, fileNameCryptor);
303295

304296
Assertions.assertEquals("theRealLongName.txt", decryptedFile);
305297
}
@@ -310,7 +302,7 @@ void testRestoreFilenameShortenedIOException() throws IOException {
310302
Path oldCipherPath = cipherOrphan.resolve("hashOfOrphanWithLongestName.c9r");
311303
Files.createDirectory(oldCipherPath);
312304

313-
Assertions.assertThrows(IOException.class, () -> result.decryptFileName(oldCipherPath, true, "someDirId", fileNameCryptor));
305+
Assertions.assertThrows(IOException.class, () -> result.decryptFileName(oldCipherPath, true, new byte[]{}, fileNameCryptor));
314306
}
315307
}
316308

@@ -440,7 +432,7 @@ public void testFixContinuesOnNotRecoverableFilename() throws IOException {
440432
Files.createDirectories(orphan2);
441433
Files.createFile(cipherOrphan.resolve(Constants.DIR_ID_BACKUP_FILE_NAME));
442434

443-
var dirId = Optional.of("trololo-id");
435+
var dirId = Optional.of(new byte[]{'t', 'r', 'o', 'l', 'o', 'l', 'o'});
444436

445437
CiphertextDirectory stepParentDir = new CiphertextDirectory("aaaaaa", dataDir.resolve("22/2222"));
446438

@@ -479,7 +471,7 @@ public void testFixWithDirId() throws IOException {
479471
Files.createDirectories(orphan2);
480472
Files.createFile(cipherOrphan.resolve(Constants.DIR_ID_BACKUP_FILE_NAME));
481473

482-
var dirId = Optional.of("trololo-id");
474+
var dirId = Optional.of(new byte[]{'t', 'r', 'o', 'l', 'o', 'l', 'o'});
483475

484476
CiphertextDirectory stepParentDir = new CiphertextDirectory("aaaaaa", dataDir.resolve("22/2222"));
485477

@@ -522,7 +514,7 @@ public void testFixWithNonCryptomatorFiles() throws IOException {
522514
Files.createFile(unrelated);
523515
Files.createFile(cipherOrphan.resolve(Constants.DIR_ID_BACKUP_FILE_NAME));
524516

525-
var dirId = Optional.of("trololo-id");
517+
var dirId = Optional.of(new byte[]{'t', 'r', 'o', 'l', 'o', 'l', 'o'});
526518

527519
CiphertextDirectory stepParentDir = new CiphertextDirectory("aaaaaa", dataDir.resolve("22/2222"));
528520
Files.createDirectories(stepParentDir.path()); //needs to be created here, otherwise the Files.move(non-crypto-resource, stepparent) will fail
@@ -605,5 +597,4 @@ public void testFixOrphanedRecoveryDir() throws IOException {
605597
Mockito.verify(resultSpy, Mockito.never()).adoptOrphanedResource(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.eq(fileNameCryptor), Mockito.any());
606598
Mockito.verify(resultSpy).prepareRecoveryDir(pathToVault, fileNameCryptor);
607599
}
608-
609600
}

0 commit comments

Comments
 (0)