RFR: 8319531: FileServerHandler::discardRequestBody could be improved

[Michael McMahon]

On Fri, 10 Nov 2023 15:44:23 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java line 115:
> 
>> 113:         byte[] skipBuffer = new byte[size];
>> 114:         while (remaining > 0) {
>> 115:             nr = readImpl(skipBuffer, 0, (int)Math.min(size, remaining));
> 
> I wonder if we should import lines 135-137 just before line 115:
> 
> Suggestion:
> 
>             if (server.isFinishing()) {
>                 break;
>             }
>             nr = readImpl(skipBuffer, 0, (int)Math.min(size, remaining));
> 
> 
> and then implement drain simply as:
> 
> 
>     public boolean drain (long l) throws IOException {
>         while (l > 0) {
>             long skip = skip(l);
>             if (skip <= 0) break; // might return 0 if isFinishing or EOF
>             l -= skip;
>         }
>         return eof;
>     }

Yeah, it seems better that drain() would call the skip() that's been implemented.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16616#discussion_r1389685437