HttpResponse.BodyHandlers.ofFile may lose data on response with small chunk fragments

[Chris Hegarty]

Thanks Dawid,

I've added 11 to the affects version, and raised the following PR:

https://github.com/openjdk/jdk/pull/11722

-Chris.

On 19/12/2022 11:38, Dawid Weiss wrote:
> 
> Thanks Chris.
> 
> Not sure whether this matters but the same bug/code goes back all the 
> way to JDK 11... (the issue lists JDK 17+ as affected):
> 
> https://github.com/openjdk/jdk11u/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java#L207-L215 
> <https://github.com/openjdk/jdk11u/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java#L207-L215>
> 
> Dawid
> 
> On Mon, Dec 19, 2022 at 10:23 AM Chris Hegarty <chegar999 at gmail.com 
> <mailto:chegar999 at gmail.com>> wrote:
> 
>     Hi Dawid,
> 
>     Thanks for raising this issue. I filed the following JIRA to track it:
>     https://bugs.openjdk.org/browse/JDK-8299015
>     <https://bugs.openjdk.org/browse/JDK-8299015>
> 
>     -Chris.
> 
>     On 17/12/2022 18:45, Dawid Weiss wrote:
>      > Hello,
>      >
>      > I've discovered a bug in the implementation of PathSubscriber in how
>      > byte buffers are flushed/ written to the file channel. Specifically,
>      > this method in PathSubscriber [2]:
>      >
>      > @Override
>      > public void onNext(List<ByteBuffer> items) {
>      >      try {
>      >          out.write(items.toArray(Utils.EMPTY_BB_ARRAY));
>      > ...
>      >
>      > the implementation assumes out.write(ByteBuffer[]) always writes the
>      > content of all buffers - this isn't the case [1] and the method can
>      > write any number of bytes from input buffers.
>      >
>      > The manifestation of this bug is that HttpClient can fail to
>     write all
>      > the data in a heavily chunked http response (!).
>      >
>      > The longer story is that I discovered the above while attempting to
>      > debug data inconsistency errors in a HttpClient fetching chunked
>      > responses. I'm on Windows and the problem is fairly easy to
>     reproduce.
>      > Wireshark (packet interceptor) shows full data being transmitted,
>      > HttpClient fails to write all of it to disk with:
>      >
>      > httpClient.send(request,
>     HttpResponse.BodyHandlers.ofFile(outputFile));
>      >
>      > The bug is fairly evident - I'm not sure how it slipped
>     undetected for
>      > so long. The non-exhaustive FileChannel.write(ByteBuffer[]) behavior
>      > can be demonstrated by this code:
>      >
>      > var file = Files.createTempFile("tmp", "tmp");
>      > try (var out = FileChannel.open(file, StandardOpenOption.WRITE,
>      > StandardOpenOption.CREATE)) {
>      >    // Create a lot of smallish buffers.
>      >    ByteBuffer[] buffers = new ByteBuffer[100];
>      >    for (int i = 0; i < buffers.length; i++) {
>      >      buffers[i] = ByteBuffer.wrap(new byte[100]);
>      >    }
>      >
>      >    System.out.println(
>      >        "Data in buffers: " +
>      > Arrays.stream(buffers).mapToLong(Buffer::remaining).sum());
>      >    long len = out.write(buffers);
>      >    System.out.println("Written: " + len);
>      >    System.out.println(
>      >        "Data in buffers: " +
>      > Arrays.stream(buffers).mapToLong(Buffer::remaining).sum());
>      > } finally {
>      >    Files.delete(file);
>      > }
>      >
>      > This prints the following for me on Windows:
>      >
>      > Data in buffers: 10000
>      > Written: 1600
>      > Data in buffers: 8400
>      >
>      > Clearly, PathSubscriber should have a loop to exhaust all buffers.
>      > Seems like a trivial fix, so I don't include the patch (and I can't
>      > file a proper Jira issue because I have no access to Jira).
>      >
>      > Dawid
>      >
>      > [1]
>     https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/channels/FileChannel.html#write(java.nio.ByteBuffer%5B%5D)
>     <https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/channels/FileChannel.html#write(java.nio.ByteBuffer%5B%5D)>
>      >
>      > [2]
>     https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java#L285
>     <https://github.com/openjdk/jdk/blob/master/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java#L285>
>