[ 
https://issues.apache.org/jira/browse/JCRVLT-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Konrad Windszus updated JCRVLT-349:
-----------------------------------
    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).

  was:
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.


> 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
>            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