SecureSocketImpl::reset shouldn't close socket

[gelldur]

I'm working on top of the poco-1.12.6 branch and I want to implement HTTPSClientSession setSource to work.
Changes for this: #2078 (Not sure why not still merged)

Everyting seems to work as I use cherry pick of this commit: d0312c6

I only encounter bug when I close session by my intention or by server sending me close connection header.

After reconnecting I "lose" source IP and fall back to default gateway. I found out that it is caused by SecureSocketImpl::connect(const SocketAddress& address, const Poco::Timespan& timeout, bool performHandshake)

poco/NetSSL_OpenSSL/src/SecureSocketImpl.cpp

Line 134 in 93eee24

if (_pSSL) reset();

Caused by:

if (_pSSL) reset(); 

which calls: close();
Maybe I don't understand something, but I believe reset shouldn't call close() as it can close just freshly created socket.
Where SecureSocketImpl::reset is called:

• dtor (we can call close() there)
• SecureSocketImpl::connectNB(const SocketAddress& address)
• SecureSocketImpl::connect(const SocketAddress& address, const Poco::Timespan& timeout, bool performHandshake)
• SecureSocketImpl::connect(const SocketAddress& address, bool performHandshake)

So in other cases than dtor socket state will be managed by other classes like HTTPClientSession so no need to explicitly close socket here again.

I would propose:

SecureSocketImpl::~SecureSocketImpl()
{
	try
	{
		close(); // <---- close in dtor as there may be some dependency on this call
		reset();
	}
	catch (...)
	{
		poco_unexpected();
	}
}

void SecureSocketImpl::reset()
{
        // <----------- remove close() as not needed
	if (_pSSL)
	{
		SSL_set_ex_data(_pSSL, SSLManager::instance().socketIndex(), nullptr);
		SSL_free(_pSSL);
		_pSSL = 0;
	}
}

---

This is how I close HTTPSClientSession:
Poco::Net::SecureStreamSocket(rawSession.socket()).close();

Not sure is it correct way but calling HTTPSClientSession::abort or HTTPSClientSession::reset do not sound right.
Maybe I'm doing something wrong ?
Also I tested calling abort/reset (or both) but I still have same issue with source IP.
I want to close gracefully socket so server ACK shutdown.

---

This is how call stack looks like:

HTTPClientSession::reconnect()
    -> HTTPSession::connect(const SocketAddress& targetAddress, const SocketAddress& sourceAddress)
        -> _socket.bind <----- as we closed socket now we recreate it
        -> HTTPSession::connect(const SocketAddress& address)
            -> _socket.connect(address, _connectionTimeout); (this is calling: SecureSocketImpl instance)
                -> SecureSocketImpl::connect(const SocketAddress& address, bool performHandshake)
                    -> reset() <----------------- issue here as we again destroy and create socket
                    -> SocketImpl::connect(const SocketAddress& address, const Poco::Timespan& timeout)

---

Example

From this test I expect to get same public IP, without above "fix" I receive 2 different public IP's

		{
			net::Connection connectionTest{"ifconfig.me"};
			connectionTest.setSourceIP("172.31.11.130");

			{
				net::ApiCall call;
				call.request.setMethod("GET");
				connectionTest.executeCall(call);
				llog::debug() << connectionTest << "\n" << call;
			}

			connectionTest.closeSession();

			{
				net::ApiCall call;
				call.request.setMethod("GET");
				connectionTest.executeCall(call);
				llog::debug() << connectionTest << "\n" << call;
			}
		}

[aleks-f]

#2078 was not merged because it was part of the 2.0 effort, which never happened (too much work, not enough contributors) and was left orphaned (that PR is to the old develop branch). You are welcome to create a new PR to dev branch and if it passes CI, we'll include it in 1.14

same for the changes proposed here - send a PR with a testcase and we will merge it, if ok. otherwise, you'll have to wait until someone gets to it.