Fail connectSSL() when the handshake times out

[luohaha]

Problem

SecureSocketImpl::connectSSL() is the only one of the four SSL_* call sites in this file that does not carry a deadline. It calls SSL_connect once and passes the result to handleError():

if (performHandshake && _pSocket->getBlocking())
{
    int ret = ::SSL_connect(_pSSL);
    handleError(ret);
    verifyPeerCertificate();
}

handleError() maps SSL_ERROR_WANT_READ to the ERR_SSL_WANT_READ return code rather than throwing, and connectSSL() discards that return value and proceeds to verifyPeerCertificate(). On a blocking socket an expired SO_RCVTIMEO surfaces as SSL_ERROR_WANT_READ — OpenSSL's socket BIO sets its retry flags on EAGAIN — so a handshake that consumed its entire timeout without receiving a ServerHello is reported to the caller as a successful connect().

The failure is then deferred into the first request on that session, which re-enters the handshake from SSL_write and spends a second timeout there. Against a peer that completes the TCP handshake and never answers — the half-open state a NAT leaves behind when it forgets a flow — one request costs connect_timeout + receive_timeout instead of connect_timeout.

sendBytes(), receiveBytes() and completeHandshake() were all given a tsStart.isElapsed(...) deadline; connectSSL() was missed.

Change

Give connectSSL() the same loop shape completeHandshake() already uses. The handshake in connectSSL() runs with the socket timeout temporarily set to the connect timeout by SecureSocketImpl::connect(address, timeout, performHandshake), so getReceiveTimeout() is the right bound here, exactly as in completeHandshake().

Non-blocking sockets are untouched: the branch is already guarded by _pSocket->getBlocking().

Measured

Built from this branch, HTTPSClientSession against a listener that accepts the connection and never writes, 1 s connect timeout and 5 s send/receive timeout:

	one stalled connection
before	6.61 s
after	1.16 s

Both raise Poco::TimeoutException; the difference is that the handshake timeout is now recognised where it happens rather than being paid again by the first request.

Test

HTTPSClientSessionTest::testStalledPeerTimeout uses the test suite's own DialogServer, which accepts the connection and — with no response queued — never writes, so the handshake gets no ServerHello. It asserts Poco::TimeoutException and that the failure arrives inside connection_timeout + receive_timeout.

Verified both ways on Linux with NetSSL-testrunner HTTPSClientSessionTest.testStalledPeerTimeout: it passes with this change and fails without it, on the elapsed-time assertion — unpatched also raises TimeoutException, so the timing is what discriminates.

with:     HTTPSClientSessionTest::testStalledPeerTimeout:  OK (1 tests)
without:  HTTPSClientSessionTest::testStalledPeerTimeout:  FAILURE
          "tsStart.elapsed() < (connectTimeout + requestTimeout).totalMicroseconds()"

Unrelated observation

While measuring the above: Timestamp::isElapsed(interval) is diff >= interval, so isElapsed(0) is always true. In the deadline loops added to sendBytes/receiveBytes/completeHandshake, a socket whose timeout is zero — which usually means no timeout — therefore throws Poco::TimeoutException on the very first retry. Not addressed here, but it may be worth a look.

🤖 Generated with Claude Code

[matejk]

I rebased the branch onto main to resolve a conflict; please pull before further changes.

Please restore the send and receive timeouts in SecureSocketImpl::connect(address, timeout, performHandshake) when connectSSL() throws: with the new TimeoutException, the socket otherwise keeps the connect timeout.

[matejk]

Two blank lines between functions, as elsewhere in the file.

Suggested change

[matejk]

Same here.

Suggested change