RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream

[Eirik Bjorsnos]

On Thu, 14 Dec 2023 20:15:39 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

> `GZIPInputStream`, when looking for a concatenated stream, relies on what the underlying `InputStream` says is how many bytes are `available()`. But this is inappropriate because `InputStream.available()` is just an estimate and is allowed (for example) to always return zero.
> 
> The fix is to ignore what's `available()` and just proceed and see what happens. If fewer bytes are available than required, the attempt to extend to another stream is canceled just as it was before, e.g., when the next stream header couldn't be read.

The test could benefit from a conversion to JUnit. Not sure I love final local variables, I see the split assignment inside the try/catch makes it useful, but perhaps if you rewrite countBytes as suggested, final will be less useful.

test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 54:

> 52:         try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(gz32))) {
> 53:             count1 = countBytes(in);
> 54:         }

Consider rewriting countBytes to take the ByteArrayInputStream/ZeroAvailableInputStream as a parameter, so you could just do:

 ```suggestion
        long count1 = countBytes(new ByteArrayInputStream(gz32));

test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 56:

> 54:         }
> 55: 
> 56:         // (a) Read it from a stream where available() always returns zero

Suggestion:

        // (b) Read it from a stream where available() always returns zero

test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 64:

> 62:         // They should be the same
> 63:         if (count2 != count1)
> 64:             throw new AssertionError(count2 + " != " + count1);

Consider converting the test to JUnit, this would allow:
Suggestion:

        asssertEquals(count1, count2);

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

PR Review: https://git.openjdk.org/jdk/pull/17113#pullrequestreview-1782746089
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427283057
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427283554
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427285307