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

[Michael McMahon]

On Mon, 22 Apr 2024 22:07:32 GMT, robert engels <duke at openjdk.org> wrote:

>> Let me add some background:
>> https://www.hyrumslaw.com/
>> https://xkcd.com/1172/
>> https://wiki.openjdk.org/display/csr/Kinds+of+Compatibility
>> 
>> Every time we make an observable behavioral change, we need to decide if we can make it without any additional documentation, or if a release note or a CSR is necessary.
>> 
>> In this case we have 2 failing tests with that change. As you correctly pointed out, these tests are failing because of bugs in their implementation. But then, these tests show that the change is already impacting existing code. With that in mind, a release note will be necessary. This is not for us; this is for the users who may upgrade to the latest version and find that their tests are failing.
>> 
>> The product changes are fine in their current form. Could you fix the failing tests?
>
> I’ll update the PR to fix the tests. I understand the need for backwards compatibility- it’s one of the prime reasons Java is awesome - but in this situation the tests are validating that 1+1=3 and I don’t think anyone wants that. 
> 
> You’re assuming that changing the validation of the test cases means the potential for breaking code - as I’ve stated before I don’t think that’s possible in this case - because the test cases were never verifying code you would see in a production system.

If the client was based on HttpURLConnection in Java and if the code was doing HTTP authentication then it's possible it could happen because HTTPURLConnection closes the connection in between each phase of the authentication. It is unfortunate as well that the server side HttpExchange.sendResponseHeaders method's second parameter is quite counter-intuitive in its meaning. It is a common mistake to provide a parameter value of zero (which means use chunked-encoding) when what is intended is probably -1, (meaning no response content).

In any case, we all still accept that the change is worthwhile, and we just need to flag it in the release notes. We also obviously have to fix the tests so they pass. You just need to make sure the exchanges get closed in all handlers defined in those two tests.

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

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