RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v2]
Markus KARG
duke at openjdk.org
Fri Dec 15 11:54:57 UTC 2023
On Fri, 15 Dec 2023 09:41:38 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Markus KARG has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Keep transferring beyond MAX_VALUE bytes
>
> src/java.base/share/classes/java/io/SequenceInputStream.java line 249:
>
>> 247: transferred = Math.addExact(transferred, in.transferTo(out));
>> 248: } catch (ArithmeticException ignore) {
>> 249: transferred = Long.MAX_VALUE;
>
> Hello Markus, looking at this code, even with this change, I think we still have a bug here. Once the `transferred` becomes `Long.MAX_VALUE`, if there is a `nextStream()` available, then when we continue in this `while` loop, then this check:
>
> if (transferred < Long.MAX_VALUE) {
>
> will prevent it from transfering to the output stream, the next input stream's content and thus ignoring the next stream's content.
>
> I think what we might want here is something like:
>
>
>
> diff --git a/src/java.base/share/classes/java/io/SequenceInputStream.java b/src/java.base/share/classes/java/io/SequenceInputStream.java
> index de3fafc884d..b89d9ca80b0 100644
> --- a/src/java.base/share/classes/java/io/SequenceInputStream.java
> +++ b/src/java.base/share/classes/java/io/SequenceInputStream.java
> @@ -242,11 +242,14 @@ public long transferTo(OutputStream out) throws IOException {
> if (getClass() == SequenceInputStream.class) {
> long transferred = 0;
> while (in != null) {
> + long numTransferred = in.transferTo(out);
> + // increment the total transferred byte count
> + // only if we haven't already reached the Long.MAX_VALUE
> if (transferred < Long.MAX_VALUE) {
> try {
> - transferred = Math.addExact(transferred, in.transferTo(out));
> + transferred = Math.addExact(transferred, numTransferred);
> } catch (ArithmeticException ignore) {
> - return Long.MAX_VALUE;
> + transferred = Long.MAX_VALUE;
> }
> }
> nextStream();
>
> (I haven't tested it)
@jaikiran Good catch! I have added your proposed solution in https://github.com/openjdk/jdk/pull/17119/commits/8181dccd9217aa5d57b2df0888373904510183df.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17119#discussion_r1427890488
More information about the core-libs-dev
mailing list