RFR: 8352502: Response message is null if expect 100 assertion fails with non 100

[Daniel Fuchs]

On Thu, 26 Jun 2025 10:48:56 GMT, Darragh Clarke <dclarke at openjdk.org> wrote:

> Currently if a request has set Expect-Continue and receives a non 100 response the `responseMessage` wouldn't be set.
> 
> This PR sets `responseMessage`, it also updates `getResponseMessage` to check if the message has already been set. This should match the way that `responseCode` is currently handled.
> 
> I also added a test to cover some possible responses.

test/jdk/java/net/httpclient/ExpectContinueResponseMessageTest.java line 98:

> 96:             while (!control.stop) {
> 97:                 try {
> 98:                     Socket socket = control.serverSocket.accept();

So this socket will not be closed... I understand closing it here might cause a reset...
Is that the reason for the catch clause on the client side?

test/jdk/java/net/httpclient/ExpectContinueResponseMessageTest.java line 124:

> 122:                     //send a wrong response
> 123:                     outputStream.write(control.statusLine.getBytes());
> 124:                     outputStream.flush();

If you added Content-Length: 0 the the status line (which should be better called response BTW) then you should be able to at least call `socket.shutdownOutput()` here.

test/jdk/java/net/httpclient/ExpectContinueResponseMessageTest.java line 149:

> 147:         String body = "Testing: " + expectedCode;
> 148:         Control control = this.control;
> 149:         control.statusLine = statusLine + "\r\n\r\n";

Shouldn't you add Content-Length: 0 here?

test/jdk/java/net/httpclient/ExpectContinueResponseMessageTest.java line 159:

> 157:         } catch (Exception ex) {
> 158:             // swallow the exception
> 159:         }

@DarraghClarke - I am curious, is this catch clause necessary? If it is maybe you could improve the comment here to explain why it is necessary.

test/jdk/java/net/httpclient/ExpectContinueResponseMessageTest.java line 167:

> 165:         assertTrue(expectedMessage.equals(responseMessage),
> 166:                 String.format("Expected Response Message  %s, instead received %s",
> 167:                         expectedMessage, responseMessage));

maybe the connection should be explicitely closed at the end.
maybe the accepted socket should be added to Control and closed here too?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25999#discussion_r2168831749
PR Review Comment: https://git.openjdk.org/jdk/pull/25999#discussion_r2168849333
PR Review Comment: https://git.openjdk.org/jdk/pull/25999#discussion_r2168833905
PR Review Comment: https://git.openjdk.org/jdk/pull/25999#discussion_r2168825892
PR Review Comment: https://git.openjdk.org/jdk/pull/25999#discussion_r2168844935