[ 
https://issues.apache.org/jira/browse/JCRVLT-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16897809#comment-16897809
 ] 

Konrad Windszus commented on JCRVLT-349:
----------------------------------------

The current state is really inconsistent:
# Filter: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java#L330
 -> InputStream is not(!) closed, as InputStream is first copied to byte array 
(for some reason?) and only the new InputStream based on this byte array is 
closed
# Config: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/AbstractVaultFsConfig.java#L76
 -> InputStream is closed. Still InputStream is first copied to another 
ByteArrayInputStream (which is not closed) for some reason....
# Settings: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/AbstractConfig.java#L90
 -> InputStream is closed. Directly acting on the input stream instead of 
copying it first.
# Properties: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L169
 -> InputStream is not(!) closed, as only {{Properties.loadFromXml(...)}} is 
called which does not close it
# Privileges: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L177
 -> InputStream is not(!) closed as the underlying 
{{PrivilegeDefinitionReader}} does not do it
# CND: 
https://github.com/apache/jackrabbit-filevault/blob/6df76ba4a45316a84ec1cd10636296d191a82260/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultMetaInf.java#L135
 -> InputStream is not(!) closed as the underlying CNDReader does not do it

[~tripod] Is default behaviour of closing what is/was really intended? Can you 
also shed some light on why for cases 1 and 2 the InputStream is first copied 
before being consumed?




> Clarify closing behaviour of input streams in methods getting it as argument
> ----------------------------------------------------------------------------
>
>                 Key: JCRVLT-349
>                 URL: https://issues.apache.org/jira/browse/JCRVLT-349
>             Project: Jackrabbit FileVault
>          Issue Type: Wish
>          Components: vlt
>    Affects Versions: 3.2.8
>            Reporter: Konrad Windszus
>            Assignee: Konrad Windszus
>            Priority: Major
>
> 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).



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Reply via email to