RFR: 8342075: HttpClient: improve HTTP/2 flow control checks

[Andrey Turbanov]

On Thu, 17 Oct 2024 16:33:47 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> Please find here a fix that improves flow control in the HTTP/2 implementation.
> 
> The change makes sure that flow control issues are reported to the server as FLOW_CONTROL_ERROR.
> It also clarify how some system properties that allow to initialize flow control windows are handled, by documenting the full range of valid values (when applicable) and explaining what happens if the property points to a value that is out of range.
> 
> Bad flow control values in the SETTINGS frame will also cause a FLOW_CONTROL_ERROR to be reported.

src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 1464:

> 1462:         Log.logError("cancelling exchange on stream {0} due to protocol error: {1}\n", streamid, cause);
> 1463:         // send a RESET frame and close the stream
> 1464:         cancelImpl(cause,code);

Suggestion:

        cancelImpl(cause, code);

test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 116:

> 114:         System.out.printf("%ntesting %s%n", uri);
> 115:         ConcurrentHashMap<String, CompletableFuture<String>>  responseSent = new ConcurrentHashMap<>();
> 116:         ConcurrentHashMap<String, HttpResponse<InputStream>>  responses = new ConcurrentHashMap<>();

Suggestion:

        ConcurrentHashMap<String, CompletableFuture<String>> responseSent = new ConcurrentHashMap<>();
        ConcurrentHashMap<String, HttpResponse<InputStream>> responses = new ConcurrentHashMap<>();

test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 299:

> 297:                 byte[] bytes = is.readAllBytes();
> 298:                 System.out.println("Server " + t.getLocalAddress() + " received:\n"
> 299:                         + t.getRequestURI() + ": " +  new String(bytes, StandardCharsets.UTF_8));

Suggestion:

                        + t.getRequestURI() + ": " + new String(bytes, StandardCharsets.UTF_8));

test/jdk/java/net/httpclient/http2/ConnectionFlowControlTest.java line 345:

> 343:     // A custom Http2TestExchangeImpl that overrides sendResponseHeaders to
> 344:     // allow headers to be sent with a number of CONTINUATION frames.
> 345:     static class  FCHttp2TestExchange extends Http2TestExchangeImpl {

Suggestion:

    static class FCHttp2TestExchange extends Http2TestExchangeImpl {

test/jdk/java/net/httpclient/http2/StreamFlowControlTest.java line 161:

> 159:     {
> 160:         System.out.printf("%ntesting testAsync(%s, %s)%n", uri, sameClient);
> 161:         ConcurrentHashMap<String, CompletableFuture<String>>  responseSent = new ConcurrentHashMap<>();

Suggestion:

        ConcurrentHashMap<String, CompletableFuture<String>> responseSent = new ConcurrentHashMap<>();

test/jdk/java/net/httpclient/http2/StreamFlowControlTest.java line 322:

> 320:     // A custom Http2TestExchangeImpl that overrides sendResponseHeaders to
> 321:     // allow headers to be sent with a number of CONTINUATION frames.
> 322:     static class  FCHttp2TestExchange extends Http2TestExchangeImpl {

Suggestion:

    static class FCHttp2TestExchange extends Http2TestExchangeImpl {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808285394
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808284352
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808284729
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808284916
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808283897
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1808283640