On Mon, 13 Nov 2023 15:25:25 GMT, Darragh Clarke <[email protected]> wrote:
>> **Problem** >> `discardRequestBody` calls `InputStream::readAllBytes` to read and discard >> all the request body bytes. This is somewhat wasteful as they can instead be >> skipped. >> >> **Changes** >> - Updated `FileServerHandler::discardRequestBody` to use `InputStream::skip`. >> - Created skip in `LeftOverInputStream` based on >> [InputStream](https://github.com/openjdk/jdk/blob/c9657cad124d2be10b8d6006d0ca9a038b1c5945/src/java.base/share/classes/java/io/InputStream.java#L540). >> This gets used by `FixedLengthInputStream` and `ChunkedInputStream` which >> previously had been using the skip implementation from `FilteredInputStream` >> which would have caused blocking. >> >> - Also made a minor change to `LeftOverInputStream::Drain` to change bufSize. >> >> >> I ran test tiers 1-3 and all tests are passing with these changes > > Darragh Clarke has updated the pull request incrementally with one additional > commit since the last revision: > > updated drain to use the new skip method src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java line 162: > 160: private static void discardRequestBody(HttpExchange exchange) throws > IOException { > 161: try (InputStream is = exchange.getRequestBody()) { > 162: is.skip(Integer.MAX_VALUE); Hello Darragh, since the goal is to read all bytes of the InputStream and then close it, I wonder if we should instead do: try (InputStream is = exchange.getRequestBody()) { boolean eof = false; while (!eof) { is.skip(Long.MAX_VALUE); // check if stream has reached EOF final int nextByte = is.read(); if (nextByte < 0) { eof = true; } } } (We could have just called `while ( !eof) { eof = is.drain(Long.MAX_VALUE); }`, but drain() is a method on an internal implementation of the `InputStream`.) In its current form (even before the changes proposed in the PR), the use of `readAllBytes()` doesn't guarantee that the entire stream is read. That can then mean that we might close the (socket) input stream before reading the entire body. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16616#discussion_r1392147516
