RFR: 6968351: httpserver clashes with delayed TCP ACKs for low Content-Length [v17]

[Jaikiran Pai]

On Tue, 23 Apr 2024 19:10:48 GMT, robert engels <duke at openjdk.org> wrote:

>> fix bug JDK-B6968351 by avoiding flush after response headers
>
> robert engels has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix broken test cases

I think it might be better to move out the new test from `test/jdk/com/sun/net/httpserver/bugs/TcpNoDelayNotRequired.java` to `test/jdk/com/sun/net/httpserver/TcpNoDelayNotRequired.java`. We haven't been using the `bugs` directory (or the bug number as the file name) for a while now.

test/jdk/com/sun/net/httpserver/bugs/TcpNoDelayNotRequired.java line 99:

> 97:             t.sendResponseHeaders(200,5);
> 98:             t.getResponseBody().write("hello".getBytes(StandardCharsets.ISO_8859_1));
> 99:             t.getResponseBody().close();

I don't have a strong preference, but would you be open to updating this (and the ChunkedHandler) to:


try (InputStream is = t.getRequestBody()) {
    is.readAllBytes();
}
Headers reqHeaders = t.getRequestHeaders();
Headers respHeaders = t.getResponseHeaders();
respHeaders.add("content-type", "text/plain");
t.sendResponseHeaders(200,5);
try (OutputStream os = t.getResponseBody()) {
    os.write("hello".getBytes(StandardCharsets.ISO_8859_1));
}


(I haven't compiled it or done any testing with the proposed snippet)

test/jdk/com/sun/net/httpserver/bugs/TcpNoDelayNotRequired.java line 112:

> 110:             is.close();
> 111:             rmap.add("content-type","text/plain");
> 112:             t.sendResponseHeaders(200,0);

The value 0 and -1 have been a constant confusion in our tests when calling `sendResponseHeaders`. Can you update this line to:


t.sendResponseHeaders(200, 0 /* chunked */);

so that it's immediately obvious what the intent here is.

test/jdk/java/net/Authenticator/B4769350.java line 358:

> 356:         {
> 357:             exchange.getResponseHeaders().add("Proxy-Authenticate", reply);
> 358:             exchange.sendResponseHeaders(407, -1);

Similarly here:


exchange.sendResponseHeaders(407, -1 /* no response body */);

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

PR Comment: https://git.openjdk.org/jdk/pull/18667#issuecomment-2077092241
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579399205
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579400959
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579402251