Windows: handle errors correctly when connecting to unreachable host

The previous code handled only some error codes, in a very inefficient
way, for some code paths. This change standardizes error handling using
a helper function that maps winsock WSAE* codes to Qt error codes.

The test for connecting to unreachable hosts or ports is now more
generic, and enabled on Windows, where it passes in local tests,
but dependency on network configuration still makes it fragile,
so ignoring some failures without completely skipping the test.

[ChangeLog][Network][Windows] Correctly emit errors when trying to
reach unreachable hosts or services

Change-Id: Icaca3e6fef88621d683f6d6fa3016212847de4ea
Fixes: QTBUG-42567
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Volker Hilsheimer 2019-07-02 14:28:58 +02:00
parent 2d597c0e1a
commit cc32226e64
2 changed files with 84 additions and 70 deletions

View File

@ -623,6 +623,53 @@ bool QNativeSocketEnginePrivate::fetchConnectionParameters()
} }
static void setErrorFromWSAError(int error, QNativeSocketEnginePrivate *d)
{
Q_ASSERT(d);
switch (error) {
case WSAEISCONN:
d->socketState = QAbstractSocket::ConnectedState;
break;
case WSAEHOSTUNREACH:
d->setError(QAbstractSocket::NetworkError, QNativeSocketEnginePrivate::HostUnreachableErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEADDRNOTAVAIL:
d->setError(QAbstractSocket::NetworkError, QNativeSocketEnginePrivate::AddressNotAvailableErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEINPROGRESS:
d->setError(QAbstractSocket::UnfinishedSocketOperationError, QNativeSocketEnginePrivate::InvalidSocketErrorString);
d->socketState = QAbstractSocket::ConnectingState;
break;
case WSAEADDRINUSE:
d->setError(QAbstractSocket::NetworkError, QNativeSocketEnginePrivate::AddressInuseErrorString);
break;
case WSAECONNREFUSED:
d->setError(QAbstractSocket::ConnectionRefusedError, QNativeSocketEnginePrivate::ConnectionRefusedErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAETIMEDOUT:
d->setError(QAbstractSocket::NetworkError, QNativeSocketEnginePrivate::ConnectionTimeOutErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEACCES:
d->setError(QAbstractSocket::SocketAccessError, QNativeSocketEnginePrivate::AccessErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAENETUNREACH:
d->setError(QAbstractSocket::NetworkError, QNativeSocketEnginePrivate::NetworkUnreachableErrorString);
d->socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEINVAL:
case WSAEALREADY:
d->setError(QAbstractSocket::UnfinishedSocketOperationError, QNativeSocketEnginePrivate::InvalidSocketErrorString);
break;
default:
break;
}
}
bool QNativeSocketEnginePrivate::nativeConnect(const QHostAddress &address, quint16 port) bool QNativeSocketEnginePrivate::nativeConnect(const QHostAddress &address, quint16 port)
{ {
@ -651,9 +698,6 @@ bool QNativeSocketEnginePrivate::nativeConnect(const QHostAddress &address, quin
case WSANOTINITIALISED: case WSANOTINITIALISED:
//### //###
break; break;
case WSAEISCONN:
socketState = QAbstractSocket::ConnectedState;
break;
case WSAEWOULDBLOCK: { case WSAEWOULDBLOCK: {
// If WSAConnect returns WSAEWOULDBLOCK on the second // If WSAConnect returns WSAEWOULDBLOCK on the second
// connection attempt, we have to check SO_ERROR's // connection attempt, we have to check SO_ERROR's
@ -668,82 +712,33 @@ bool QNativeSocketEnginePrivate::nativeConnect(const QHostAddress &address, quin
do { do {
if (::getsockopt(socketDescriptor, SOL_SOCKET, SO_ERROR, (char *) &value, &valueSize) == 0) { if (::getsockopt(socketDescriptor, SOL_SOCKET, SO_ERROR, (char *) &value, &valueSize) == 0) {
if (value != NOERROR) { if (value != NOERROR) {
WS_ERROR_DEBUG(value);
errorDetected = true;
// MSDN says getsockopt with SO_ERROR clears the error, but it's not actually cleared // MSDN says getsockopt with SO_ERROR clears the error, but it's not actually cleared
// and this can affect all subsequent WSAConnect attempts, so clear it now. // and this can affect all subsequent WSAConnect attempts, so clear it now.
const int val = NO_ERROR; const int val = NO_ERROR;
::setsockopt(socketDescriptor, SOL_SOCKET, SO_ERROR, reinterpret_cast<const char*>(&val), sizeof val); ::setsockopt(socketDescriptor, SOL_SOCKET, SO_ERROR, reinterpret_cast<const char*>(&val), sizeof val);
} } else {
if (value == WSAECONNREFUSED) {
setError(QAbstractSocket::ConnectionRefusedError, ConnectionRefusedErrorString);
socketState = QAbstractSocket::UnconnectedState;
errorDetected = true;
break;
}
if (value == WSAETIMEDOUT) {
setError(QAbstractSocket::NetworkError, ConnectionTimeOutErrorString);
socketState = QAbstractSocket::UnconnectedState;
errorDetected = true;
break;
}
if (value == WSAEHOSTUNREACH) {
setError(QAbstractSocket::NetworkError, HostUnreachableErrorString);
socketState = QAbstractSocket::UnconnectedState;
errorDetected = true;
break;
}
if (value == WSAEADDRNOTAVAIL) {
setError(QAbstractSocket::NetworkError, AddressNotAvailableErrorString);
socketState = QAbstractSocket::UnconnectedState;
errorDetected = true;
break;
}
if (value == NOERROR) {
// When we get WSAEWOULDBLOCK the outcome was not known, so a // When we get WSAEWOULDBLOCK the outcome was not known, so a
// NOERROR might indicate that the result of the operation // NOERROR might indicate that the result of the operation
// is still unknown. We try again to increase the chance that we did // is still unknown. We try again to increase the chance that we did
// get the correct result. // get the correct result.
tryAgain = !tryAgain; tryAgain = !tryAgain;
} }
setErrorFromWSAError(value, this);
} }
tries++; tries++;
} while (tryAgain && (tries < 2)); } while (tryAgain && (tries < 2));
if (errorDetected) if (errorDetected)
break; break;
// fall through to unfinished operation error handling
err = WSAEINPROGRESS;
Q_FALLTHROUGH(); Q_FALLTHROUGH();
} }
case WSAEINPROGRESS:
setError(QAbstractSocket::UnfinishedSocketOperationError, InvalidSocketErrorString);
socketState = QAbstractSocket::ConnectingState;
break;
case WSAEADDRINUSE:
setError(QAbstractSocket::NetworkError, AddressInuseErrorString);
break;
case WSAECONNREFUSED:
setError(QAbstractSocket::ConnectionRefusedError, ConnectionRefusedErrorString);
socketState = QAbstractSocket::UnconnectedState;
break;
case WSAETIMEDOUT:
setError(QAbstractSocket::NetworkError, ConnectionTimeOutErrorString);
break;
case WSAEACCES:
setError(QAbstractSocket::SocketAccessError, AccessErrorString);
socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEHOSTUNREACH:
setError(QAbstractSocket::NetworkError, HostUnreachableErrorString);
socketState = QAbstractSocket::UnconnectedState;
break;
case WSAENETUNREACH:
setError(QAbstractSocket::NetworkError, NetworkUnreachableErrorString);
socketState = QAbstractSocket::UnconnectedState;
break;
case WSAEINVAL:
case WSAEALREADY:
setError(QAbstractSocket::UnfinishedSocketOperationError, InvalidSocketErrorString);
break;
default: default:
setErrorFromWSAError(err, this);
break; break;
} }
if (socketState != QAbstractSocket::ConnectedState) { if (socketState != QAbstractSocket::ConnectedState) {

View File

@ -164,9 +164,8 @@ private slots:
void waitForReadyReadInASlot(); void waitForReadyReadInASlot();
void remoteCloseError(); void remoteCloseError();
void nestedEventLoopInErrorSlot(); void nestedEventLoopInErrorSlot();
#ifndef Q_OS_WIN void connectToHostError_data();
void connectToLocalHostNoService(); void connectToHostError();
#endif
void waitForConnectedInHostLookupSlot(); void waitForConnectedInHostLookupSlot();
void waitForConnectedInHostLookupSlot2(); void waitForConnectedInHostLookupSlot2();
void readyReadSignalsAfterWaitForReadyRead(); void readyReadSignalsAfterWaitForReadyRead();
@ -2040,18 +2039,38 @@ void tst_QTcpSocket::nestedEventLoopInErrorSlot()
} }
//---------------------------------------------------------------------------------- //----------------------------------------------------------------------------------
#ifndef Q_OS_WIN
void tst_QTcpSocket::connectToLocalHostNoService() void tst_QTcpSocket::connectToHostError_data()
{
QTest::addColumn<QString>("host");
QTest::addColumn<int>("port");
QTest::addColumn<QAbstractSocket::SocketError>("expectedError");
QTest::newRow("localhost no service") << QStringLiteral("localhost") << 31415 << QAbstractSocket::ConnectionRefusedError;
QTest::newRow("unreachable") << QStringLiteral("0.0.0.1") << 65000 << QAbstractSocket::NetworkError;
}
void tst_QTcpSocket::connectToHostError()
{ {
// This test was created after we received a report that claimed
// QTcpSocket would crash if trying to connect to "localhost" on a random
// port with no service listening.
QTcpSocket *socket = newSocket(); QTcpSocket *socket = newSocket();
socket->connectToHost("localhost", 31415); // no service running here, one suspects
QAbstractSocket::SocketError error = QAbstractSocket::UnknownSocketError;
QFETCH(QString, host);
QFETCH(int, port);
QFETCH(QAbstractSocket::SocketError, expectedError);
connect(socket, QOverload<QAbstractSocket::SocketError>::of(&QAbstractSocket::error),[&](QAbstractSocket::SocketError socketError){
error = socketError;
});
socket->connectToHost(host, port); // no service running here, one suspects
QTRY_COMPARE(socket->state(), QTcpSocket::UnconnectedState); QTRY_COMPARE(socket->state(), QTcpSocket::UnconnectedState);
if (error != expectedError && error == QAbstractSocket::ConnectionRefusedError)
QEXPECT_FAIL("unreachable", "CI firewall interfers with this test", Continue);
QCOMPARE(error, expectedError);
delete socket; delete socket;
} }
#endif
//---------------------------------------------------------------------------------- //----------------------------------------------------------------------------------
void tst_QTcpSocket::waitForConnectedInHostLookupSlot() void tst_QTcpSocket::waitForConnectedInHostLookupSlot()