On Thu, 17 Oct 2024 16:33:47 GMT, Daniel Fuchs <dfu...@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
