ffang commented on PR #3640: URL: https://github.com/apache/cxf/pull/3640#issuecomment-6505872618
> **Possible follow-up, as a separate JIRA:** also set `connectionComplete` in `contentLength()`, before it waits. For HTTP/1.1 the HttpClient only asks for the length after it has connected and completed the TLS handshake, so the trust decision could go ahead and the request would succeed. The ordering for HTTP/2 needs checking before relying on this. Thanks @coheigea, good catch. I reproduced it: with HTTPS, a trust decider and chunking disallowed, the last commit made the call spin forever, where before it failed after the connection timeout. While checking, I found that a refused connection with a trust decider spins forever too, even with chunking allowed, because `subscribe()` never runs, so `connectionComplete` is never set. I've done both in this PR, in https://github.com/apache/cxf/pull/3640/commits/6c88b674d4: - **The loop in `getHttpsURLConnectionInfo()`** now stops once the request has failed, and rethrows the failure. - **`HttpClientSizedBodyPublisher.contentLength()`** sets `connectionComplete` before it waits for the body. So the trust decision goes ahead, the body gets written, and the request succeeds instead of failing after the connection timeout. Since the follow-up you suggested is part of this PR, a separate JIRA isn't needed. I checked the HTTP/2 ordering: the HttpClient asks for the content length only once the connection is set up, for HTTP/2 as well as HTTP/1.1, so for HTTPS the TLS handshake is already done. The new HTTP/2 test below confirms it. New tests: - **`HTTPSConduitTest.testHttpsTrustWithoutChunking`:** a trusted peer succeeds and an untrusted one fails, without chunking. - **`HTTPSConduitTest.testHttpsTrustConnectionRefused`:** with a trust decider and nothing listening on the port, the call fails rather than hangs, with chunking allowed or not. - **`JettyClientServerHttp2Test.testHttpsTrustWithoutChunkingHttp2`:** the same trusted case with HTTP/2 forced. The server echoes the protocol it received, and the test asserts `HTTP/2.0`. | Code | `testHttpsTrustWithoutChunking` (HTTP/1.1) | `testHttpsTrustWithoutChunkingHttp2` | |---|---|---| | Previous commit | hangs | hangs | | Only the loop change | fails after the connection timeout | fails after the connection timeout | | Both changes | passes | passes | `testHttpsTrustConnectionRefused` hangs on the previous commit and passes with the loop change. Freeman -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
