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

[Jaikiran Pai]

On Thu, 25 Apr 2024 12:45:28 GMT, robert engels <duke at openjdk.org> wrote:

>> 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)
>
> Isn’t consistency in the code base more important. I like the change but it seems there should be a single PR that changes all of the test cases to this format.

We have been using these newer constructs whenever a new test gets added, but we don't update all tests in one go for such changes. If/when old tests are updated for some bug fix relevant to that test, depending on the complexity we either decide to let them stay as-is or update them to use these newer constructs.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1579417278