search

cryptomator / cryptofs

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

Calling the size() of a broken CyphertextFile throws IllegalArgumentException #29

Closed infeo closed 6 years ago

infeo commented 6 years ago

The caused highlevel-error is described here: https://github.com/cryptomator/cryptomator/issues/673

Getting the size of an invalid cyphertext file throws an (undocumented) IllegalArgumentException.

We should either document this behavoiur und handle it in the corresponding nio-adapter libraries or should change it e.g. such that a negative size is returned (or something else).

The problem causing code is in CrypoBasicFileAttribute: https://github.com/cryptomator/cryptofs/blob/04cb671690ae87c5baf03a57a53d1d224a1d404f/src/main/java/org/cryptomator/cryptofs/CryptoBasicFileAttributes.java#L58

overheadhunter commented 6 years ago

Since we explicitly introduced the IllegalArgumentException to find bugs easier, we should document it.

Issue is related to #22.

overheadhunter commented 6 years ago

Reopening since we've reconsidered:

The IllegalArgumentException thrown by Cryptors.cleartextSize should be handled by CryptoFS within the size() method itself.

https://github.com/cryptomator/cryptofs/blob/1db02cc353c0e549f1b2cdf60f688cacaf4bc886/src/main/java/org/cryptomator/cryptofs/CryptoBasicFileAttributes.java#L48-L60

In the case of a invalid ciphertext a size of 0 should be returned. The [contract of this method](https://docs.oracle.com/javase/10/docs/api/java/nio/file/attribute/BasicFileAttributes.html#size()) states:

Returns the size of the file (in bytes). The size may differ from the actual size on the file system due to compression, support for sparse files, or other reasons. The size of files that are not regular files is implementation specific and therefore unspecified.

Even is a ciphertext file is broken, it is still reported as a "regular" file (not a dir, not a symlink, ...). Therefore the returned value must be a valid number of bytes, i.e. a positive integer.

A file is then reported as an "empty" file in dir listings. When attempting to access a malformed file, we can still throw an IOException. This seems to be the most correct way to handle broken files.

infeo commented 6 years ago

closed with commit https://github.com/cryptomator/cryptofs/commit/02fa4a0e72910dd959659eae5535bcabf880d75c