Skip to content

SecureSocketImpl::reset shouldn't close socket #4415

@gelldur

Description

@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)

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;
			}
		}

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

Relationships

None yet

Development

No branches or pull requests

Issue actions