On 17 August 2011 03:42, Stefan Bodewig <bode...@apache.org> wrote: > 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?
That would be fine. > Stefan > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
