On Tue, 14 Jun 2022 15:20:28 GMT, Conor Cleary <ccle...@openjdk.org> wrote:
> **Issue**
> A failure in this test was observed whereby an unexpected connection from a
> client that was not created by the test caused the test to unsucessfully
> complete.
>
> **Solution**
> When a connection is accepted by the ServerSocket, some verification of the
> Request URI and of the Request Path is conducted. If the client connection is
> found to be invalid or external to the test, the connection is closed. The
> ServerSocket will continue to accept new connections until the correct
> parameters are met. Once a valid connection is accepted, the test behaves
> exactly as it previously did.
Thanks for the fix Conor! I have suggested some changes that should make the
fix easier to review and keep it closer to the original.
test/jdk/java/net/httpclient/ServerCloseTest.java line 302:
> 300:
> 301: StringTokenizer tokenizer = new
> StringTokenizer(requestLine);
> 302: tokenizer.nextToken(); // Skip method token as not
> used
Since we were asserting here before, maybe we could directly close the
clientConnection here if `method` is not GET or POST.
Something like:
if (!"GET".equals(method) && !POST.equals(method)) {
System.out.println(now() + getName() + ": Unexpected method: " + method);
clientConnection.close();
continue;
}
test/jdk/java/net/httpclient/ServerCloseTest.java line 304:
> 302: tokenizer.nextToken(); // Skip method token as not
> used
> 303: String path = tokenizer.nextToken();
> 304:
We should check `path` immediately before trying to build the URI. If it
doesn't contain what we expect, then close the clientConnection and continue as
suggested above.
test/jdk/java/net/httpclient/ServerCloseTest.java line 313:
> 311: System.err.printf("Bad target address: \"%s\" in
> \"%s\"%n",
> 312: path, requestLine);
> 313: validURI = false;
maybe just leave that as before?
test/jdk/java/net/httpclient/ServerCloseTest.java line 317:
> 315:
> 316: // Proceed if URI is well-formed and the request
> path is as expected
> 317: if (validURI && path.contains("/dummy/x")) {
Since we will have already checked `path`, `method` and `uri` here just
unconditionally add the clientConnection to the list and proceed as before.
-------------
Changes requested by dfuchs (Reviewer).
PR: https://git.openjdk.org/jdk/pull/9155
