Major Description
There are several places in the FileVault code where an input stream is passed as argument to a method and closed within it (even though this is not explicitly mentioned in the javadocs). I think this is bad for the following reasons:
- it is unexpected for the caller that the given input stream is closed (i.e. this is also never the case for methods taking an input stream in org.apache.commons.io.IOUtils nor in the JRE itself (https://docs.oracle.com/javase/7/docs/api/java/util/Properties.html#load(java.io.InputStream)))
- closing the input stream might actually have an undesired effect if the input stream may contain additional input which is supposed to be consumed afterwards
- closing often happens in a way that there may be leaks in case of exceptions.
- the behaviour is not documented
Examples for such behaviour are at
- https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L104
- https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java#L364
If I am not incorrect the behaviour is also not consistent as e.g. in https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L136 the input stream is not closed.
Update: As it is too dangerous to change the semantics right now it should rather be consistently closed in all these methods in FileVault and clearly documented in the javadocs (although this would still deviate from the semantics of such methods in JRE and most other libraries).