On Wed, 8 Jun 2022 18:29:10 GMT, Conor Cleary <ccle...@openjdk.org> wrote:
> **Issue** > It was observed that when the httpclient sends a POST request with the > `Expect: 100 Continue` header set and the server replies with a response code > `417 Expectation Failed` that the httpclient hangs indefinitely when the > version of Http used is HTTP/2. However, it was also seen that the issue > persisted with HTTP/1_1 with the same usage. > > This was caused by an implementation in ExchangeImpl that resulted in two > calls to readBodyAsync() in this case, where the second call causes the > indefinite hanging (as if there was a respomse body, it has already been > read). > > **Solution** > When ExchangeImpl::expectContinue() detects that a response code 417 is > received, two things occur. Firstly, a flag is set which ensures that the > connection is closed locally in this case. Secondly, the response is returned > to the client as a failed future, A unit test was added to ensure that this > usage of the httpclient does not cause hanging. Good fix! Still some issues with the test. src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java line 255: > 253: > 254: // Called when server returns non 100 response to > 255: // and Expect-Continue Typo? test/jdk/java/net/httpclient/ExpectContinueTest.java line 89: > 87: InetSocketAddress saHang = new > InetSocketAddress(InetAddress.getLoopbackAddress(), 0); > 88: > 89: httpTestServer = > HttpServerAdapters.HttpTestServer.of(HttpServer.create(sa, 0)); You should be able to write: httpTestServer = HttpTestServer.of(HttpServer.create(sa, 0)); here. No need for the `HttpServerAdapters` prefix. test/jdk/java/net/httpclient/ExpectContinueTest.java line 217: > 215: os.write(bytes); > 216: os.flush(); > 217: } maybe you should close the client socket if validRequest == false. You should also implement a close() method to close the server socket and the client socket when the test terminates. Also the test seems to be missing a teardown() method to properly close and stop all the servers. ------------- PR: https://git.openjdk.java.net/jdk/pull/9093
