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

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

> 58:         InetAddress loopback = InetAddress.getLoopbackAddress();
> 59:         InetSocketAddress addr = new InetSocketAddress (loopback, 0);
> 60:         HttpServer server = HttpServer.create (addr, 0);

Given the change we are doing, I think it would be good to even include a test (within this same test class) which tests the `HttpsServer`.

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

> 72:             var uri = URIBuilder.newBuilder().scheme("http").loopback().port(server.getAddress().getPort()).path("/test").build();
> 73:             var response = client.send(HttpRequest.newBuilder(uri).build(), HttpResponse.BodyHandlers.ofString());
> 74:             if(!response.body().equals("hello")) throw new IllegalStateException("incorrect body");

It helps during debugging if the response body which failed the checked would either be logged or included as part of the exception that's being thrown. Perhaps:


throw new IllegalStateException("unexpected response body: " + response.body());

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

> 77:             var uri = URIBuilder.newBuilder().scheme("http").loopback().port(server.getAddress().getPort()).path("/chunked").build();
> 78:             var response = client.send(HttpRequest.newBuilder(uri).build(), HttpResponse.BodyHandlers.ofString());
> 79:             if(!response.body().equals("hello")) throw new IllegalStateException("incorrect body");

Same applies here, it would be better to include the failing response body in the exception message.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579386619
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579389263
PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579390252