cryptomator / cryptofs

Java Filesystem Provider with integrated encryption
GNU Affero General Public License v3.0
94 stars 35 forks source link

closes #86 #87

Closed infeo closed 4 years ago

infeo commented 4 years ago

This implements the suggestion mentioned in the issue description of :

https://github.com/cryptomator/cryptofs/issues/86

infeo commented 4 years ago

Instead of doing a "let's make it work somehow" fix, we should do a proper refactoring of the backupMasterkeyFileIfRequired method (making the ifRequired part more intelligent).

I second that. But before change the method: Is the word required correct in the method name? From my perspective "possible" or "desired" would be actual term, since we are always making a backup except the filesystem is unlocked in read-only mode, or am I wrong?

Second, and more severe: This doesn't solve the issue under all circumstances. If there isn't a bkup file in the first place (never created due to readonly), Files.readAllBytes will fail with an IOException.

That is... true. This introduces even new errors 😓 I'll refactor it.

infeo commented 4 years ago

I refactored it into one method.

As written in the commit message, only valid states are now:

Otherwise an IllegalStateException is thrown.

Tests are missing, but if someone points me to the correct place, i'll implement them.

overheadhunter commented 4 years ago

Codacy Here is an overview of what got changed by this pull request:


Complexity increasing per file
==============================
- src/main/java/org/cryptomator/cryptofs/CryptoFileSystemModule.java  1
- src/main/java/org/cryptomator/cryptofs/common/MasterkeyBackupHelper.java  5

See the complete overview on Codacy