On Thu, 14 Dec 2023 20:15:39 GMT, Archie Cobbs <aco...@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
