Fix bind+connect in both TCP and UDP

This has been known to be broken for a while. Now it works: you can bind
and you'll retain the port (and the file descriptor) for the connect
call. Incidentally, in fixing the binding for more than one IP for the
hostname (with event loop), this commit fixes the setSocketDescriptor
XFAIL.

[ChangeLog][QtNetwork] Fixed a bug that caused both QTcpSocket and
QUdpSocket to close the socket and lose any bound ports before
connecting. Now bind()/setSocketDescriptor() followed by connect() will
retain the original file descriptor.

Task-number: QTBUG-26538
Change-Id: I691caed7e8fd16a9cf687b5995afbf3006bf453a
Reviewed-by: Richard J. Moore <rich@kde.org>
This commit is contained in:
Thiago Macieira 2014-12-23 13:37:01 -02:00
parent 29051bce39
commit 9fb68a90af
10 changed files with 266 additions and 46 deletions

View File

@ -689,6 +689,7 @@ void qt_qhostinfo_clear_cache()
}
}
#ifdef QT_BUILD_INTERNAL
void Q_AUTOTEST_EXPORT qt_qhostinfo_enable_cache(bool e)
{
QAbstractHostInfoLookupManager* manager = theHostInfoLookupManager();
@ -697,6 +698,16 @@ void Q_AUTOTEST_EXPORT qt_qhostinfo_enable_cache(bool e)
}
}
void qt_qhostinfo_cache_inject(const QString &hostname, const QHostInfo &resolution)
{
QAbstractHostInfoLookupManager* manager = theHostInfoLookupManager();
if (!manager || !manager->cache.isEnabled())
return;
manager->cache.put(hostname, resolution);
}
#endif
// cache for 60 seconds
// cache 128 items
QHostInfoCache::QHostInfoCache() : max_age(60), enabled(true), cache(128)

View File

@ -117,6 +117,7 @@ public:
QHostInfo Q_NETWORK_EXPORT qt_qhostinfo_lookup(const QString &name, QObject *receiver, const char *member, bool *valid, int *id);
void Q_AUTOTEST_EXPORT qt_qhostinfo_clear_cache();
void Q_AUTOTEST_EXPORT qt_qhostinfo_enable_cache(bool e);
void Q_AUTOTEST_EXPORT qt_qhostinfo_cache_inject(const QString &hostname, const QHostInfo &resolution);
class QHostInfoCache
{

View File

@ -976,7 +976,7 @@ void QAbstractSocketPrivate::startConnectingByName(const QString &host)
connectTimeElapsed = 0;
if (initSocketLayer(QAbstractSocket::UnknownNetworkLayerProtocol)) {
if (cachedSocketDescriptor != -1 || initSocketLayer(QAbstractSocket::UnknownNetworkLayerProtocol)) {
if (socketEngine->connectToHostByName(host, port) ||
socketEngine->state() == QAbstractSocket::ConnectingState) {
cachedSocketDescriptor = socketEngine->socketDescriptor();
@ -1117,7 +1117,7 @@ void QAbstractSocketPrivate::_q_connectToNextAddress()
host.toString().toLatin1().constData(), port, addresses.count());
#endif
if (!initSocketLayer(host.protocol())) {
if (cachedSocketDescriptor == -1 && !initSocketLayer(host.protocol())) {
// hope that the next address is better
#if defined(QABSTRACTSOCKET_DEBUG)
qDebug("QAbstractSocketPrivate::_q_connectToNextAddress(), failed to initialize sock layer");
@ -1134,9 +1134,6 @@ void QAbstractSocketPrivate::_q_connectToNextAddress()
return;
}
// cache the socket descriptor even if we're not fully connected yet
cachedSocketDescriptor = socketEngine->socketDescriptor();
// Check that we're in delayed connection state. If not, try
// the next address
if (socketEngine->state() != QAbstractSocket::ConnectingState) {
@ -1481,54 +1478,60 @@ void QAbstractSocket::setPauseMode(PauseModes pauseMode)
bool QAbstractSocket::bind(const QHostAddress &address, quint16 port, BindMode mode)
{
Q_D(QAbstractSocket);
return d->bind(address, port, mode);
}
bool QAbstractSocketPrivate::bind(const QHostAddress &address, quint16 port, QAbstractSocket::BindMode mode)
{
Q_Q(QAbstractSocket);
// now check if the socket engine is initialized and to the right type
if (!d->socketEngine || !d->socketEngine->isValid()) {
if (!socketEngine || !socketEngine->isValid()) {
QHostAddress nullAddress;
d->resolveProxy(nullAddress.toString(), port);
resolveProxy(nullAddress.toString(), port);
QAbstractSocket::NetworkLayerProtocol protocol = address.protocol();
if (protocol == QAbstractSocket::UnknownNetworkLayerProtocol)
protocol = nullAddress.protocol();
if (!d->initSocketLayer(protocol))
if (!initSocketLayer(protocol))
return false;
}
if (mode != DefaultForPlatform) {
if (mode != QAbstractSocket::DefaultForPlatform) {
#ifdef Q_OS_UNIX
if ((mode & ShareAddress) || (mode & ReuseAddressHint))
d->socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 1);
if ((mode & QAbstractSocket::ShareAddress) || (mode & QAbstractSocket::ReuseAddressHint))
socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 1);
else
d->socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 0);
socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 0);
#endif
#ifdef Q_OS_WIN
if (mode & ReuseAddressHint)
d->socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 1);
if (mode & QAbstractSocket::ReuseAddressHint)
socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 1);
else
d->socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 0);
if (mode & DontShareAddress)
d->socketEngine->setOption(QAbstractSocketEngine::BindExclusively, 1);
socketEngine->setOption(QAbstractSocketEngine::AddressReusable, 0);
if (mode & QAbstractSocket::DontShareAddress)
socketEngine->setOption(QAbstractSocketEngine::BindExclusively, 1);
else
d->socketEngine->setOption(QAbstractSocketEngine::BindExclusively, 0);
socketEngine->setOption(QAbstractSocketEngine::BindExclusively, 0);
#endif
}
bool result = d->socketEngine->bind(address, port);
d->cachedSocketDescriptor = d->socketEngine->socketDescriptor();
bool result = socketEngine->bind(address, port);
cachedSocketDescriptor = socketEngine->socketDescriptor();
if (!result) {
d->socketError = d->socketEngine->error();
setErrorString(d->socketEngine->errorString());
emit error(d->socketError);
socketError = socketEngine->error();
q->setErrorString(socketEngine->errorString());
emit q->error(socketError);
return false;
}
d->state = BoundState;
d->localAddress = d->socketEngine->localAddress();
d->localPort = d->socketEngine->localPort();
state = QAbstractSocket::BoundState;
localAddress = socketEngine->localAddress();
localPort = socketEngine->localPort();
emit stateChanged(d->state);
d->socketEngine->setReadNotificationEnabled(true);
emit q->stateChanged(state);
socketEngine->setReadNotificationEnabled(true);
return true;
}
@ -1605,14 +1608,16 @@ void QAbstractSocket::connectToHost(const QString &hostName, quint16 port,
d->preferredNetworkLayerProtocol = protocol;
d->hostName = hostName;
d->port = port;
d->state = UnconnectedState;
d->buffer.clear();
d->writeBuffer.clear();
d->abortCalled = false;
d->pendingClose = false;
d->localPort = 0;
if (d->state != BoundState) {
d->state = UnconnectedState;
d->localPort = 0;
d->localAddress.clear();
}
d->peerPort = 0;
d->localAddress.clear();
d->peerAddress.clear();
d->peerName = hostName;
if (d->hostLookupId != -1) {

View File

@ -135,9 +135,11 @@ public:
PauseModes pauseMode() const;
void setPauseMode(PauseModes pauseMode);
// ### Qt6: make the first one virtual
bool bind(const QHostAddress &address, quint16 port = 0, BindMode mode = DefaultForPlatform);
bool bind(quint16 port = 0, BindMode mode = DefaultForPlatform);
// ### Qt6: de-virtualize connectToHost(QHostAddress) overload
virtual void connectToHost(const QString &hostName, quint16 port, OpenMode mode = ReadWrite, NetworkLayerProtocol protocol = AnyIPProtocol);
virtual void connectToHost(const QHostAddress &address, quint16 port, OpenMode mode = ReadWrite);
virtual void disconnectFromHost();

View File

@ -78,6 +78,8 @@ public:
}
#endif
virtual bool bind(const QHostAddress &address, quint16 port, QAbstractSocket::BindMode mode);
bool canReadNotification();
bool canWriteNotification();
void canCloseNotification();

View File

@ -136,6 +136,12 @@ QT_BEGIN_NAMESPACE
" not in "#state1" or "#state2); \
return (returnValue); \
} } while (0)
#define Q_CHECK_STATES3(function, state1, state2, state3, returnValue) do { \
if (d->socketState != (state1) && d->socketState != (state2) && d->socketState != (state3)) { \
qWarning(""#function" was called" \
" not in "#state1" or "#state2); \
return (returnValue); \
} } while (0)
#define Q_CHECK_TYPE(function, type, returnValue) do { \
if (d->socketType != (type)) { \
qWarning(#function" was called by a" \
@ -495,7 +501,7 @@ bool QNativeSocketEngine::connectToHost(const QHostAddress &address, quint16 por
if (!d->checkProxy(address))
return false;
Q_CHECK_STATES(QNativeSocketEngine::connectToHost(),
Q_CHECK_STATES3(QNativeSocketEngine::connectToHost(), QAbstractSocket::BoundState,
QAbstractSocket::UnconnectedState, QAbstractSocket::ConnectingState, false);
d->peerAddress = address;
@ -961,7 +967,7 @@ bool QNativeSocketEngine::waitForWrite(int msecs, bool *timedOut)
QNativeSocketEnginePrivate::TimeOutErrorString);
d->hasSetSocketError = false; // A timeout error is temporary in waitFor functions
return false;
} else if (state() == QAbstractSocket::ConnectingState) {
} else if (state() == QAbstractSocket::ConnectingState || (state() == QAbstractSocket::BoundState && d->socketDescriptor != -1)) {
connectToHost(d->peerAddress, d->peerPort);
}

View File

@ -2404,6 +2404,29 @@ bool QSslSocketPrivate::isPaused() const
return paused;
}
bool QSslSocketPrivate::bind(const QHostAddress &address, quint16 port, QAbstractSocket::BindMode mode)
{
// this function is called from QAbstractSocket::bind
if (!initialized)
init();
initialized = false;
#ifdef QSSLSOCKET_DEBUG
qCDebug(lcSsl) << "QSslSocket::bind(" << address << ',' << port << ',' << mode << ')';
#endif
if (!plainSocket) {
#ifdef QSSLSOCKET_DEBUG
qCDebug(lcSsl) << "\tcreating internal plain socket";
#endif
createPlainSocket(QIODevice::ReadWrite);
}
bool ret = plainSocket->bind(address, port, mode);
localPort = plainSocket->localPort();
localAddress = plainSocket->localAddress();
cachedSocketDescriptor = plainSocket->socketDescriptor();
return ret;
}
/*!
\internal
*/

View File

@ -172,6 +172,7 @@ public:
static void checkSettingSslContext(QSslSocket*, QSharedPointer<QSslContext>);
static QSharedPointer<QSslContext> sslContext(QSslSocket *socket);
bool isPaused() const;
bool bind(const QHostAddress &address, quint16, QAbstractSocket::BindMode) Q_DECL_OVERRIDE;
void _q_connectedSlot();
void _q_hostFoundSlot();
void _q_disconnectedSlot();

View File

@ -73,6 +73,7 @@
// RVCT compiles also unused inline methods
# include <QNetworkProxy>
#include <time.h>
#ifdef Q_OS_LINUX
#include <stdio.h>
#include <stdlib.h>
@ -123,6 +124,8 @@ private slots:
void constructing();
void bind_data();
void bind();
void bindThenResolveHost_data();
void bindThenResolveHost();
void setInvalidSocketDescriptor();
#ifndef Q_OS_WINRT
void setSocketDescriptor();
@ -245,6 +248,9 @@ private:
int earlyBytesWrittenCount;
int earlyReadyReadCount;
QString stressTestDir;
QString firstFailName;
QHostInfo firstFailInfo;
};
enum ProxyTests {
@ -297,7 +303,10 @@ public:
};
tst_QTcpSocket::tst_QTcpSocket()
: firstFailName("qt-test-server-first-fail")
{
qsrand(time(NULL));
tmpSocket = 0;
//This code relates to the socketsConstructedBeforeEventLoop test case
@ -308,6 +317,8 @@ tst_QTcpSocket::tst_QTcpSocket()
connect(earlyConstructedSockets->endPoints[0], SIGNAL(readyRead()), this, SLOT(earlySocketReadyRead()));
connect(earlyConstructedSockets->endPoints[1], SIGNAL(bytesWritten(qint64)), this, SLOT(earlySocketBytesSent(qint64)));
earlyConstructedSockets->endPoints[1]->write("hello work");
firstFailInfo.setAddresses(QList<QHostAddress>() << QHostAddress("224.0.0.0") << QtNetworkSettings::serverIP());
}
tst_QTcpSocket::~tst_QTcpSocket()
@ -389,6 +400,7 @@ void tst_QTcpSocket::init()
}
qt_qhostinfo_clear_cache();
qt_qhostinfo_cache_inject(firstFailName, firstFailInfo);
}
QTcpSocket *tst_QTcpSocket::newSocket() const
@ -486,9 +498,12 @@ void tst_QTcpSocket::constructing()
void tst_QTcpSocket::bind_data()
{
QTest::addColumn<QString>("stringAddr");
QTest::addColumn<int>("port");
QTest::addColumn<bool>("successExpected");
QTest::addColumn<QString>("stringExpectedLocalAddress");
bool testIpv6 = false;
// iterate all interfaces, add all addresses on them as test data
QList<QNetworkInterface> interfaces = QNetworkInterface::allInterfaces();
foreach (const QNetworkInterface &netinterface, interfaces) {
@ -501,10 +516,26 @@ void tst_QTcpSocket::bind_data()
continue; // link-local bind will fail, at least on Linux, so skip it.
QString ip(entry.ip().toString());
QTest::newRow(ip.toLatin1().constData()) << ip << true << ip;
QTest::newRow(ip.toLatin1().constData()) << ip << 0 << true << ip;
if (!testIpv6 && entry.ip().protocol() == QAbstractSocket::IPv6Protocol)
testIpv6 = true;
}
}
// test binding to localhost
QTest::newRow("0.0.0.0") << "0.0.0.0" << 0 << true << "0.0.0.0";
if (testIpv6)
QTest::newRow("[::]") << "::" << 0 << true << "::";
// and binding with a port number...
// Since we want to test that we got the port number we asked for, we need a random port number.
// We use random in case a previous run of the test left the port lingering open.
// -1 indicates "random port"
QTest::newRow("0.0.0.0:randomport") << "0.0.0.0" << -1 << true << "0.0.0.0";
if (testIpv6)
QTest::newRow("[::]:randomport") << "::" << -1 << true << "::";
// additionally, try bind to known-bad addresses, and make sure this doesn't work
// these ranges are guaranteed to be reserved for 'documentation purposes',
// and thus, should be unused in the real world. Not that I'm assuming the
@ -513,8 +544,16 @@ void tst_QTcpSocket::bind_data()
knownBad << "198.51.100.1";
knownBad << "2001:0DB8::1";
foreach (const QString &badAddress, knownBad) {
QTest::newRow(badAddress.toLatin1().constData()) << badAddress << false << QString();
QTest::newRow(badAddress.toLatin1().constData()) << badAddress << 0 << false << QString();
}
#ifdef Q_OS_UNIX
// try to bind to a privileged ports
// we should fail if we're not root (unless the ports are in use!)
QTest::newRow("127.0.0.1:1") << "127.0.0.1" << 1 << !geteuid() << (geteuid() ? QString() : "127.0.0.1");
if (testIpv6)
QTest::newRow("[::]:1") << "::" << 1 << !geteuid() << (geteuid() ? QString() : "::");
#endif
}
void tst_QTcpSocket::bind()
@ -523,23 +562,124 @@ void tst_QTcpSocket::bind()
if (setProxy)
return; // QTBUG-22964 for proxies, QTBUG-29972 for QSKIP
QFETCH(QString, stringAddr);
QFETCH(int, port);
QFETCH(bool, successExpected);
QFETCH(QString, stringExpectedLocalAddress);
QHostAddress addr(stringAddr);
QHostAddress expectedLocalAddress(stringExpectedLocalAddress);
QTcpSocket dummySocket; // used only to "use up" a file descriptor
dummySocket.bind();
QTcpSocket *socket = newSocket();
qDebug() << "Binding " << addr;
quint16 boundPort;
qintptr fd;
if (successExpected) {
QVERIFY2(socket->bind(addr), qPrintable(socket->errorString()));
bool randomPort = port == -1;
int attemptsLeft = 5; // only used with randomPort
do {
if (randomPort) {
// try to get a random port number
// we do this to ensure we're not trying to bind to the same port as we've just used in
// a previous run - race condition with the OS actually freeing the port
Q_STATIC_ASSERT(RAND_MAX > 1024);
port = qrand() & USHRT_MAX;
if (port < 1024)
continue;
}
bool bindSuccess = socket->bind(addr, port);
if (!bindSuccess && randomPort && socket->error() == QTcpSocket::AddressInUseError) {
// we may have been unlucky and hit an already open port, so try another
--attemptsLeft;
continue;
}
QVERIFY2(bindSuccess, qPrintable(socket->errorString() + ", tried port " + QString::number(port)));
break;
} while (randomPort && attemptsLeft);
QCOMPARE(socket->state(), QAbstractSocket::BoundState);
boundPort = socket->localPort();
if (port)
QCOMPARE(int(boundPort), port);
fd = socket->socketDescriptor();
QVERIFY(fd != INVALID_SOCKET);
} else {
QVERIFY(!socket->bind(addr));
QVERIFY(!socket->bind(addr, port));
QCOMPARE(socket->localPort(), quint16(0));
}
QCOMPARE(socket->localAddress(), expectedLocalAddress);
if (successExpected) {
// try to use the socket and expect it to remain working
QTcpServer server;
QVERIFY(server.listen(addr));
// free up the file descriptor
dummySocket.close();
QHostAddress remoteAddr = addr;
if (addr == QHostAddress::AnyIPv4)
remoteAddr = QHostAddress::LocalHost;
else if (addr == QHostAddress::AnyIPv6)
remoteAddr = QHostAddress::LocalHostIPv6;
socket->connectToHost(remoteAddr, server.serverPort());
QVERIFY2(socket->waitForConnected(2000), socket->errorString().toLocal8Bit());
QVERIFY(server.waitForNewConnection(2000));
QTcpSocket *acceptedSocket = server.nextPendingConnection();
QCOMPARE(socket->localPort(), boundPort);
QCOMPARE(acceptedSocket->peerPort(), boundPort);
QCOMPARE(socket->localAddress(), remoteAddr);
QCOMPARE(socket->socketDescriptor(), fd);
}
delete socket;
}
//----------------------------------------------------------------------------------
void tst_QTcpSocket::bindThenResolveHost_data()
{
QTest::addColumn<QString>("hostName");
QTest::newRow("ip-literal") << QtNetworkSettings::serverIP().toString();
QTest::newRow("name") << QtNetworkSettings::serverName();
QTest::newRow("first-fail") << firstFailName;
}
// similar to the previous test, but we'll connect to a host name that needs resolving
void tst_QTcpSocket::bindThenResolveHost()
{
QFETCH_GLOBAL(bool, setProxy);
if (setProxy)
return; // doesn't make sense to test binding locally with proxies
QFETCH(QString, hostName);
QTcpSocket dummySocket; // used only to "use up" a file descriptor
dummySocket.bind();
QTcpSocket *socket = newSocket();
QVERIFY2(socket->bind(QHostAddress(QHostAddress::AnyIPv4), 0), socket->errorString().toLocal8Bit());
QCOMPARE(socket->state(), QAbstractSocket::BoundState);
quint16 boundPort = socket->localPort();
qintptr fd = socket->socketDescriptor();
QVERIFY(fd != INVALID_SOCKET);
dummySocket.close();
socket->connectToHost(hostName, 80);
QVERIFY2(socket->waitForConnected(), "Network timeout");
QCOMPARE(socket->localPort(), boundPort);
QCOMPARE(socket->socketDescriptor(), fd);
delete socket;
}
@ -592,13 +732,10 @@ void tst_QTcpSocket::setSocketDescriptor()
QCOMPARE(socket->socketDescriptor(), (qintptr)sock);
qt_qhostinfo_clear_cache(); //avoid the HostLookupState being skipped due to address being in cache from previous test.
socket->connectToHost(QtNetworkSettings::serverName(), 143);
socket->connectToHost(QtNetworkSettings::serverName(), 80);
QCOMPARE(socket->state(), QTcpSocket::HostLookupState);
QCOMPARE(socket->socketDescriptor(), (qintptr)sock);
QVERIFY(socket->waitForConnected(10000));
// skip this, it has been broken for years, see task 260735
// if somebody complains, consider fixing it, but it might break existing applications.
QEXPECT_FAIL("", "bug has been around for years, will not fix without need", Continue);
QCOMPARE(socket->socketDescriptor(), (qintptr)sock);
delete socket;
#ifdef Q_OS_WIN
@ -615,8 +752,8 @@ void tst_QTcpSocket::socketDescriptor()
QCOMPARE(socket->socketDescriptor(), (qintptr)-1);
socket->connectToHost(QtNetworkSettings::serverName(), 143);
QVERIFY((socket->state() == QAbstractSocket::HostLookupState && socket->socketDescriptor() == -1) ||
(socket->state() == QAbstractSocket::ConnectingState && socket->socketDescriptor() != -1));
QVERIFY(socket->state() == QAbstractSocket::HostLookupState ||
socket->state() == QAbstractSocket::ConnectingState);
QVERIFY(socket->waitForConnected(10000));
QVERIFY(socket->state() == QAbstractSocket::ConnectedState);
QVERIFY(socket->socketDescriptor() != -1);

View File

@ -85,7 +85,8 @@ private slots:
void dualStack();
void dualStackAutoBinding();
void dualStackNoIPv4onV6only();
void readLine();
void connectToHost();
void bindAndConnectToHost();
void pendingDatagramSize();
void writeDatagram();
void performance();
@ -621,7 +622,7 @@ void tst_QUdpSocket::empty_connectedSlot()
//----------------------------------------------------------------------------------
void tst_QUdpSocket::readLine()
void tst_QUdpSocket::connectToHost()
{
QUdpSocket socket1;
QUdpSocket socket2;
@ -629,6 +630,7 @@ void tst_QUdpSocket::readLine()
socket1.setProperty("_q_networksession", QVariant::fromValue(networkSession));
socket2.setProperty("_q_networksession", QVariant::fromValue(networkSession));
#endif
QVERIFY2(socket1.bind(), socket1.errorString().toLatin1().constData());
socket2.connectToHost(makeNonAny(socket1.localAddress()), socket1.localPort());
@ -637,6 +639,36 @@ void tst_QUdpSocket::readLine()
//----------------------------------------------------------------------------------
void tst_QUdpSocket::bindAndConnectToHost()
{
QUdpSocket socket1;
QUdpSocket socket2;
QUdpSocket dummysocket;
#ifdef FORCE_SESSION
socket1.setProperty("_q_networksession", QVariant::fromValue(networkSession));
socket2.setProperty("_q_networksession", QVariant::fromValue(networkSession));
dummysocket.setProperty("_q_networksession", QVariant::fromValue(networkSession));
#endif
// we use the dummy socket to use up a file descriptor
dummysocket.bind();
QVERIFY2(socket2.bind(), socket2.errorString().toLatin1());
quint16 boundPort = socket2.localPort();
qintptr fd = socket2.socketDescriptor();
QVERIFY2(socket1.bind(), socket1.errorString().toLatin1().constData());
dummysocket.close();
socket2.connectToHost(makeNonAny(socket1.localAddress()), socket1.localPort());
QVERIFY(socket2.waitForConnected(5000));
QCOMPARE(socket2.localPort(), boundPort);
QCOMPARE(socket2.socketDescriptor(), fd);
}
//----------------------------------------------------------------------------------
void tst_QUdpSocket::pendingDatagramSize()
{
QUdpSocket server;