RFR: 8193682: Infinite loop in ZipOutputStream.close() [v4]

[Lance Andersen]

On Tue, 26 Oct 2021 06:30:39 GMT, Ravi Reddy <rreddy at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review this fix for Infinite loop in ZipOutputStream.close().
>> The main issue here is when ever there is an exception during close operations on GZip we are not setting the deflator to a finished state which is leading to an infinite loop when we try writing on the same GZip instance( since we use while(!def.finished()) inside the write operation).
>> 
>> Thanks,
>> Ravi
>
> Ravi Reddy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8193682 : Infinite loop in ZipOutputStream.close()

I think overall this looks good.  Thank you for continuing to work on this.

I do believe it would be worth adding a CSR just to document the behavior realizing that it was always left undefined in the past

Please also add a comment to each test describing its intent

test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 41:

> 39: public class GZipLoopTest {
> 40:     private static final int FINISH_NUM = 512;
> 41:     private static OutputStream outStream = new OutputStream() {

Please add a comment describing the intent of outstream and for FINISH_NUM.  You might also consider a different name vs FINISH_NUM.  Perhaps the comment will clarify this

test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 57:

> 55:     @Test
> 56:     public void testGZipClose() throws IOException {
> 57:         rand.nextBytes(b);

You could possibly consider using a BeforeTest or BeforeMethod if you choose to reduce redundancy.  No biggie otherwise.

test/jdk/java/util/zip/GZIP/GZipLoopTest.java line 65:

> 63:         } catch (IOException e) {
> 64:             //expected
> 65:         }

For the above if an IOException is expected, should this be tested via assertThrows()?

-------------

PR: https://git.openjdk.java.net/jdk/pull/5522