RFR: 8328919: Add BodyHandlers / BodySubscribers methods to handle excessive server input [v13]

[Jaikiran Pai]

On Wed, 22 Jan 2025 09:45:16 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Adds `limiting()` factory methods to `HttpResponse.Body{Handlers,Subscribers}` to handle excessive server input in `HttpClient`. I would appreciate your input whether `discardExcess` should be kept or dropped. I plan to file a CSR once there is an agreement on the PR.
>
> Volkan Yazici has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Verify `limiting()` doesn't affect header parsing

test/jdk/java/net/httpclient/HttpResponseLimitingTest.java line 179:

> 177:         for (var pair : new ServerClientPair[]{HTTP1, HTTPS1, HTTP2, HTTPS2}) {
> 178:             pair.client.close();
> 179:             pair.server.stop();

I think we should consider catching any exception that might happen here when closing each client/server and log a message and continue closing the next pair, especially since this is not a `othervm` test - we wouldn't want the sockets to continue to be open after this test completes.

test/jdk/java/net/httpclient/HttpResponseLimitingTest.java line 198:

> 196:         try (InputStream responseBodyStream = response.body()) {
> 197:             byte[] responseBodyBuffer = new byte[RESPONSE_BODY.length];
> 198:             int responseBodyBufferLength = responseBodyStream.read(responseBodyBuffer);

I think a better way would be to read the `responseBodyStream` till EOF and keep accumulating the read data into the byte array. After reaching EOF, we then compare the accumulated byte array for the expected size and content. In the current form, if there's a short read (i.e. this `read()` call reads lesser bytes than the `responseBodyBuffer.length`, even if there's additional data available), then this test will fail.

test/jdk/java/net/httpclient/HttpResponseLimitingTest.java line 230:

> 228:             var exception = assertThrows(
> 229:                     IOException.class,
> 230:                     () -> responseBodyStream.read(new byte[RESPONSE_BODY.length]));

Here too, I would suggest reading till EOF and expecting IOException at some point while doing so, for the same reason I noted in a review comment above.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1925427812
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1925436156
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1925444519