Commit c7579eb1 authored by Simon Redman's avatar Simon Redman

Fix LanLinkProviderTest on Windows

## Summary
LanLinkProviderTest fails on Windows. This patch fixes that.

I believe the root cause is that we are using a shared UDP socket to listen for identity broadcasts both in the LanLinkProvider and in the test. Apparently this works on Linux, but on Windows the LanLinkProvider picks up its own identity packet and pairs with itself.

This patch gives a parameter to LanLinkProvider to allow it to listen and broadcast on different ports, then uses that ability in the test to make the test pass on Windows.

## Test Plan

### Before:
lanlinkprovider test fails, first because it can't bind its UDP listener socket, and then because Windows seems to handle shared sockets differently than Linux, so the UDP broadcasts were not reaching the test's listener.

### After:
lanlinkprovider test seems to pass reliably both in my Windows VM and in the CI
parent 9453e640
Pipeline #4067 passed with stage
in 9 minutes and 32 seconds
......@@ -45,10 +45,16 @@
#define MIN_VERSION_WITH_SSL_SUPPORT 6
LanLinkProvider::LanLinkProvider(bool testMode)
LanLinkProvider::LanLinkProvider(
bool testMode,
quint16 udpBroadcastPort,
quint16 udpListenPort
)
: m_server(new Server(this))
, m_udpSocket(this)
, m_tcpPort(0)
, m_udpBroadcastPort(udpBroadcastPort)
, m_udpListenPort(udpListenPort)
, m_testMode(testMode)
, m_combineBroadcastsTimer(this)
{
......@@ -86,14 +92,14 @@ void LanLinkProvider::onStart()
{
const QHostAddress bindAddress = m_testMode? QHostAddress::LocalHost : QHostAddress::Any;
bool success = m_udpSocket.bind(bindAddress, UDP_PORT, QUdpSocket::ShareAddress);
bool success = m_udpSocket.bind(bindAddress, m_udpListenPort, QUdpSocket::ShareAddress);
if (!success) {
QAbstractSocket::SocketError sockErr = m_udpSocket.error();
// Refer to https://doc.qt.io/qt-5/qabstractsocket.html#SocketError-enum to decode socket error number
QString errorMessage = QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey(sockErr);
qCritical(KDECONNECT_CORE)
<< QLatin1String("Failed to bind UDP socket on port")
<< UDP_PORT
<< m_udpListenPort
<< QLatin1String("with error")
<< errorMessage;
}
......@@ -160,15 +166,14 @@ void LanLinkProvider::broadcastToNetwork()
QHostAddress sourceAddress = ifaceAddress.ip();
if (sourceAddress.protocol() == QAbstractSocket::IPv4Protocol && sourceAddress != QHostAddress::LocalHost) {
qCDebug(KDECONNECT_CORE()) << "Broadcasting as" << sourceAddress;
sendSocket.bind(sourceAddress, UDP_PORT);
sendSocket.writeDatagram(np.serialize(), destAddress, UDP_PORT);
sendSocket.writeDatagram(np.serialize(), destAddress, m_udpBroadcastPort);
sendSocket.close();
}
}
}
}
#else
m_udpSocket.writeDatagram(np.serialize(), destAddress, UDP_PORT);
m_udpSocket.writeDatagram(np.serialize(), destAddress, m_udpBroadcastPort);
#endif
}
......@@ -237,7 +242,7 @@ void LanLinkProvider::connectError(QAbstractSocket::SocketError socketError)
NetworkPacket np(QLatin1String(""));
NetworkPacket::createIdentityPacket(&np);
np.set(QStringLiteral("tcpPort"), m_tcpPort);
m_udpSocket.writeDatagram(np.serialize(), m_receivedIdentityPackets[socket].sender, UDP_PORT);
m_udpSocket.writeDatagram(np.serialize(), m_receivedIdentityPackets[socket].sender, m_udpBroadcastPort);
//The socket we created didn't work, and we didn't manage
//to create a LanDeviceLink from it, deleting everything.
......@@ -299,7 +304,7 @@ void LanLinkProvider::tcpSocketConnected()
//I think this will never happen, but if it happens the deviceLink
//(or the socket that is now inside it) might not be valid. Delete them.
qCDebug(KDECONNECT_CORE) << "Fallback (2), try reverse connection (send udp packet)";
m_udpSocket.writeDatagram(np2.serialize(), m_receivedIdentityPackets[socket].sender, UDP_PORT);
m_udpSocket.writeDatagram(np2.serialize(), m_receivedIdentityPackets[socket].sender, m_udpBroadcastPort);
}
delete m_receivedIdentityPackets.take(socket).np;
......
......@@ -40,7 +40,16 @@ class KDECONNECTCORE_EXPORT LanLinkProvider
Q_OBJECT
public:
LanLinkProvider(bool testMode = false);
/**
* @param testMode Some special overrides needed while testing
* @param udpBroadcastPort Port which should be used for *sending* identity packets
* @param udpListenPort Port which should be used for *receiving* identity packets
*/
LanLinkProvider(
bool testMode = false,
quint16 udpBroadcastPort = UDP_PORT,
quint16 udpListenPort = UDP_PORT
);
~LanLinkProvider() override;
QString name() override { return QStringLiteral("LanLinkProvider"); }
......@@ -53,6 +62,9 @@ public:
static void configureSslSocket(QSslSocket* socket, const QString& deviceId, bool isDeviceTrusted);
static void configureSocket(QSslSocket* socket);
/**
* This is the default UDP port both for broadcasting and receiving identity packets
*/
const static quint16 UDP_PORT = 1716;
const static quint16 MIN_TCP_PORT = 1716;
const static quint16 MAX_TCP_PORT = 1764;
......@@ -83,6 +95,9 @@ private:
QUdpSocket m_udpSocket;
quint16 m_tcpPort;
quint16 m_udpBroadcastPort;
quint16 m_udpListenPort;
QMap<QString, LanDeviceLink*> m_links;
QMap<QString, LanPairingHandler*> m_pairingHandlers;
......
......@@ -32,6 +32,7 @@
#include <QSslKey>
#include <QUdpSocket>
#include <QtCrypto>
#include <QMetaEnum>
/*
* This class tests the working of LanLinkProvider under different conditions that when identity packet is received over TCP, over UDP and same when the device is paired.
......@@ -42,14 +43,28 @@ class LanLinkProviderTest : public QObject
Q_OBJECT
public:
explicit LanLinkProviderTest()
: m_lanLinkProvider(true) {
: m_lanLinkProvider(true)
, m_server(nullptr)
, m_reader(nullptr)
, m_udpSocket(nullptr)
{
QStandardPaths::setTestModeEnabled(true);
}
public Q_SLOTS:
void initTestCase();
void init();
void cleanup();
private Q_SLOTS:
/**
* Test that the LanLinkProvider will send an identity packet to a non-default port
*/
void testChangedUDPBroadcastPort();
/**
* Test that the LanLinkProvider will receive an identity packet on a non-default port
*/
void testChangedUDPListenPort();
void pairedDeviceTcpPacketReceived();
void pairedDeviceUdpPacketReceived();
......@@ -78,7 +93,7 @@ private:
void removeTrustedDevice();
void setSocketAttributes(QSslSocket* socket);
void testIdentityPacket(QByteArray& identityPacket);
void socketBindErrorFail(const QUdpSocket& socket);
};
void LanLinkProviderTest::initTestCase()
......@@ -90,22 +105,90 @@ void LanLinkProviderTest::initTestCase()
m_privateKey = QCA::KeyGenerator().createRSA(2048);
m_certificate = generateCertificate(m_deviceId, m_privateKey);
m_identityPacket = QStringLiteral("{\"id\":1439365924847,\"type\":\"kdeconnect.identity\",\"body\":{\"deviceId\":\"testdevice\",\"deviceName\":\"Test Device\",\"protocolVersion\":6,\"deviceType\":\"phone\",\"tcpPort\":") + QString::number(TEST_PORT) + QStringLiteral("}}");
}
void LanLinkProviderTest::init()
{
m_lanLinkProvider.onStart();
}
m_identityPacket = QStringLiteral("{\"id\":1439365924847,\"type\":\"kdeconnect.identity\",\"body\":{\"deviceId\":\"testdevice\",\"deviceName\":\"Test Device\",\"protocolVersion\":6,\"deviceType\":\"phone\",\"tcpPort\":") + QString::number(TEST_PORT) + QStringLiteral("}}");
void LanLinkProviderTest::cleanup()
{
m_lanLinkProvider.onStop();
}
void LanLinkProviderTest::testChangedUDPBroadcastPort()
{
quint16 udpListenPort = LanLinkProvider::UDP_PORT;
quint16 udpBroadcastPort = LanLinkProvider::UDP_PORT + 1;
m_lanLinkProvider.onStop();
LanLinkProvider testlanLinkProvider(true, udpBroadcastPort, udpListenPort);
testlanLinkProvider.onStart();
QUdpSocket mUdpServer;
bool bindSuccessful = mUdpServer.bind(QHostAddress::LocalHost, udpBroadcastPort, QUdpSocket::ShareAddress | QUdpSocket::ReuseAddressHint);
if (!bindSuccessful) {
socketBindErrorFail(mUdpServer);
}
QSignalSpy spy(&mUdpServer, SIGNAL(readyRead()));
testlanLinkProvider.onNetworkChange();
QVERIFY2(!spy.isEmpty() || spy.wait(), "Did not receive UDP packet");
}
void LanLinkProviderTest::testChangedUDPListenPort()
{
quint16 udpListenPort = LanLinkProvider::UDP_PORT + 1;
quint16 udpBroadcastPort = LanLinkProvider::UDP_PORT;
m_lanLinkProvider.onStop();
LanLinkProvider testlanLinkProvider(true, udpBroadcastPort, udpListenPort);
testlanLinkProvider.onStart();
m_server = new Server(this);
QUdpSocket testUdpSocket;
m_server->listen(QHostAddress::LocalHost, TEST_PORT);
QSignalSpy spy(m_server, SIGNAL(newConnection()));
// Write an identity packet to udp socket here. We do not broadcast it here.
qint64 bytesWritten = testUdpSocket.writeDatagram(m_identityPacket.toLatin1(), QHostAddress::LocalHost, udpListenPort);
QCOMPARE(bytesWritten, m_identityPacket.size());
// In response to receiving an identity packet, the LanLinkProvider should try to open a TCP connection to us
QVERIFY(!spy.isEmpty() || spy.wait());
QSslSocket* serverSocket = m_server->nextPendingConnection();
QVERIFY2(serverSocket != 0, "Server socket is null");
QVERIFY2(serverSocket->isOpen(), "Server socket already closed");
delete m_server;
}
void LanLinkProviderTest::pairedDeviceTcpPacketReceived()
{
quint16 udpListenPort = LanLinkProvider::UDP_PORT;
quint16 udpBroadcastPort = LanLinkProvider::UDP_PORT + 1;
m_lanLinkProvider.onStop();
LanLinkProvider testlanLinkProvider(true, udpBroadcastPort, udpListenPort);
testlanLinkProvider.onStart();
KdeConnectConfig* kcc = KdeConnectConfig::instance();
addTrustedDevice();
QUdpSocket* mUdpServer = new QUdpSocket;
bool b = mUdpServer->bind(QHostAddress::LocalHost, LanLinkProvider::UDP_PORT, QUdpSocket::ShareAddress);
QVERIFY(b);
bool bindSuccessful = mUdpServer->bind(QHostAddress::LocalHost, udpBroadcastPort, QUdpSocket::ShareAddress);
if (!bindSuccessful) {
socketBindErrorFail(*mUdpServer);
}
QSignalSpy spy(mUdpServer, SIGNAL(readyRead()));
m_lanLinkProvider.onNetworkChange();
testlanLinkProvider.onNetworkChange();
QVERIFY(!spy.isEmpty() || spy.wait());
QByteArray datagram;
......@@ -208,12 +291,21 @@ void LanLinkProviderTest::pairedDeviceUdpPacketReceived()
void LanLinkProviderTest::unpairedDeviceTcpPacketReceived()
{
quint16 udpListenPort = LanLinkProvider::UDP_PORT;
quint16 udpBroadcastPort = LanLinkProvider::UDP_PORT + 1;
m_lanLinkProvider.onStop();
LanLinkProvider testlanLinkProvider(true, udpBroadcastPort, udpListenPort);
testlanLinkProvider.onStart();
QUdpSocket* mUdpServer = new QUdpSocket;
bool b = mUdpServer->bind(QHostAddress::LocalHost, LanLinkProvider::UDP_PORT, QUdpSocket::ShareAddress);
QVERIFY(b);
bool bindSuccessful = mUdpServer->bind(QHostAddress::LocalHost, udpBroadcastPort, QUdpSocket::ShareAddress);
if (!bindSuccessful) {
socketBindErrorFail(*mUdpServer);
}
QSignalSpy spy(mUdpServer, SIGNAL(readyRead()));
m_lanLinkProvider.onNetworkChange();
testlanLinkProvider.onNetworkChange();
QVERIFY(!spy.isEmpty() || spy.wait());
QByteArray datagram;
......@@ -351,6 +443,15 @@ void LanLinkProviderTest::removeTrustedDevice()
kcc->removeTrustedDevice(m_deviceId);
}
void LanLinkProviderTest::socketBindErrorFail(const QUdpSocket& socket)
{
QAbstractSocket::SocketError sockErr = socket.error();
// Refer to https://doc.qt.io/qt-5/qabstractsocket.html#SocketError-enum to decode socket error number
QString errorMessage = QLatin1String("Failed to bind UDP socket with error ");
errorMessage = errorMessage + QMetaEnum::fromType<QAbstractSocket::SocketError>().valueToKey(sockErr);
QFAIL(errorMessage.toLocal8Bit().data());
}
QTEST_GUILESS_MAIN(LanLinkProviderTest)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment