On Thu, 30 Jun 2022 14:26:23 GMT, Ryan Ernst <d...@openjdk.org> wrote:
>> Since ReplyingServer binds to an open port on the system where tests
>> run, it is possible some other program probes that port while the test
>> is running. If the probe request does not have request params, the
>> request is invalid and a test assertion fails. This commit replaces that
>> assertion with returning a 400 response.
>
> Ryan Ernst has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains two additional commits since
> the last revision:
>
> - Merge branch 'master' into httpclient_test_assert
> - 8287593: ShortResponseBody could be made more resilient to rogue
> connections
>
> Since ReplyingServer binds to an open port on the system where tests
> run, it is possible some other program probes that port while the test
> is running. If the probe request does not have request params, the
> request is invalid and a test assertion fails. This commit replaces that
> assertion with returning a 400 response.
test/jdk/java/net/httpclient/ShortResponseBody.java line 531:
> 529: if (query == null) {
> 530: out.println("Request headers: [" + headers +
> "]");
> 531: writeResponse(s, BAD_REQUEST_RESPONSE,
> BAD_REQUEST_RESPONSE.length());
I'd suggest improving the log message above to say that an unexpected request
has been received, and then add a second line to say "Replying with 400 Bad
Request".
Otherwise this looks reasonable to me. Sending 400 is a good idea.
-------------
PR: https://git.openjdk.org/jdk/pull/9329
