commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <>
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 <> wrote:
> On 2011-08-15, sebb wrote:
>> On 15 August 2011 09:56, Stefan Bodewig <> 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, 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:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message