RFR: 8298588: WebSockets: HandshakeUrlEncodingTest unnecessarily depends on a response body [v2]

Daniel Fuchs dfuchs at openjdk.org
Wed Dec 14 11:41:31 UTC 2022


On Tue, 13 Dec 2022 14:48:22 GMT, Michal Karm Babacek <duke at openjdk.org> wrote:

>> According to [rfc6455](https://www.rfc-editor.org/rfc/rfc6455.html#section-5.5.1), the close frame MAY contain a body, i.e. it is considered [optional](https://www.rfc-editor.org/rfc/rfc2119#section-5). It seems that the contemporary JDK HEAD (tip) does populate the body and thus enables `HandshakeUrlEncodingTest.java` to parse its contents. On the contrary, JDK 11 does not populate the body in the same case. I would like to backport JDK-8245245 all the way to JDK 11, so I would like to change the behavior of this test so as it works across JDK versions.
>
> Michal Karm Babacek has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Makes test independent on JDK-8240666

test/jdk/java/net/httpclient/websocket/HandshakeUrlEncodingTest.java line 127:

> 125:                 assertEquals(rawQuery, expectedRawQuery);
> 126:                 if (wse.getResponse().body() != null &&
> 127:                         (wse.getResponse().body().getClass().equals(String.class))) {

In the case of the mainline there MUST be a body - otherwise that is a bug in the implementation.
I believe the mainline test should therefore asserts that the body is present and equals to what it expects.

There is no requirement that a test in the mainline be the same than in previous releases, especially when the mainline test makes use of a functionality that is not present in those releases.

It should be perfectly fine to have the mainline test checks two things, and the update release test only one thing, because the second thing is not implemented in the update release.

If you modify the mainline test to simply add a check to assert that the response URI is what the text expects, then the diff between the update release test (that only check the response URI) and the mainline test after your fix (that checks both the response URI and the body) should naturally show that the mainline test has one additional test compared to the update release test - which is making use of a functionality only available in the mainline. 

I would even suggest to modify the handler in the update release to not write anything into the response body since the body is discarded by the client implementation in that release.

The reason I'm a bit insistent on keeping the check on the body string is that it's the only thing that can reliably assert that the server has received the correct URI. Conceptually, the URI in the HttpResponse could have be composed by the client implementation itself. Writing the URI in the response body was a convenient way to check what the server had actually received.

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

PR: https://git.openjdk.org/jdk/pull/11486


More information about the net-dev mailing list