On 2011-08-15, sebb wrote: > On 15 August 2011 09:56, Stefan Bodewig <bode...@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? Stefan --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
