commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [compress] close() and archives/streams in an invalid state
Date Wed, 17 Aug 2011 03:16:15 GMT
On 17 August 2011 03:42, Stefan Bodewig <bodewig@apache.org> wrote:
> On 2011-08-15, sebb wrote:
>
>> On 15 August 2011 09:56, Stefan Bodewig <bodewig@apache.org> wrote:
>>> Hi,
>
>>> while working on the Zip64 stuff I deliberately created some invalid
>>> archives to test I get the expected exceptions.  While doing so I
>>> realized I couldn't close the underlying stream because
>>> ZipArchiveOutputStream's close would throw an exception as finish()
>>> failed before ever closing the wrapped stream.  The calling code may not
>>> even have a handle to the underlying stream if it used the File-arg
>>> constructor and so in the end a broken archive leaks the file
>>> descriptor.
>
>>> Then I went looking through the other implementations and found we are
>>> consistent here as far as archivers and bzip2 go.  All of them will not
>>> close the underlying stream if the archive/stream is invalid.  I'm not
>>> sure what gzip does as it just delegates to close in the Java classlib's
>>> GZipOutputStream.
>
>>> I wonder whether we should rather change the behavior to always close
>>> the underlying stream or whether we should add a new method - I called
>>> in destroy in ZipArchiveOutputStream - that does so.
>
>>> Or maybe we just document it for 1.3, add destroy as a non-common method
>>> where needed (like in ZIP, in all other cases the user code should have
>>> access to the underlying stream) - and revisit this in the 2.x
>>> timeframe, even though I can't see what could be broken by always
>>> closing the stream in a finally block.
>
>>> Does anybody see a good reason why the close behavior should stay the
>>> way it is right now?
>
>> For output, it seems sensible to close the underlying stream on
>> failure, as the content is likely to be garbage.
>
> Yes, this is the case I was talking about.
>
>> For input, there might be a use case for leaving the stream open, in
>> case some kind of recovery is possible.
>
> All our implementations - except for the new dump code, that doesn't
> properly handle close ATM - try to close the input stream regardless of
> whether there has been a problem with the archive or not (they don't
> even check).  This is what I'd expect it to do as well.
>
> Some of the guard against closing System.in, but not all.
>
>> It would be useful to have a way of determining the input file pointer
>> at the point of failure.
>
> stream.getBytesRead() or are you thinking about anything else?

That would be fine.

> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message