Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v2]

2023-12-15 Thread Brian Burkhalter
On Fri, 15 Dec 2023 09:55:36 GMT, Alan Bateman  wrote:

> [...] maybe we need to come up with tests that quickly check the handling at 
> this limit.

@mkarg Do you intend to provide any such test as part of this request?

-

PR Comment: https://git.openjdk.org/jdk/pull/17119#issuecomment-1858308908


Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v2]

2023-12-15 Thread Markus KARG
On Fri, 15 Dec 2023 09:41:38 GMT, Jaikiran Pai  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


Re: RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v2]

2023-12-15 Thread Markus KARG
> Fixes JDK-8322141
> 
> As suggested by @vlsi and documented by @bplb this PR does not return, but 
> only sets the maximum result value.

Markus KARG has updated the pull request incrementally with one additional 
commit since the last revision:

  Keep transferring beyond MAX_VALUE bytes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17119/files
  - new: https://git.openjdk.org/jdk/pull/17119/files/2b175c07..8181dccd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17119=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=17119=00-01

  Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17119.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17119/head:pull/17119

PR: https://git.openjdk.org/jdk/pull/17119