From c1243deab9ac13415074763ac8879fb7eae25e51 Mon Sep 17 00:00:00 2001 From: Andrey Batyiev Date: Mon, 26 Apr 2021 10:37:34 +0300 Subject: [PATCH 1/2] SYNERGY-885 mac client listens on ipv4 only but attempts to connect on ipv6 (#6983) * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix ipv6 port parsing * Fix ipv6 server bind * SYNERGY 885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Add ipv6 scope checker * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Update changelog * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Refactor network address logic * SYNERGY-885-Mac client listens on ipv4 only but attempts to connect on ipv6 * Build fix * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix code smells * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix empty ipv4 hostname * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix build * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Add test for new logic * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Upgrade network adress parser tests * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix sonar code smells in tests * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 * Fix code smells * SYNERGY-885 Mac client listens on ipv4 only but attempts to connect on ipv6 *Fix comment Co-authored-by: user Co-authored-by: Andrii Batyiev --- ChangeLog | 1 + src/gui/src/MainWindow.cpp | 9 +++ src/lib/arch/unix/ArchNetworkBSD.cpp | 20 +++++- src/lib/net/NetworkAddress.cpp | 78 +++++++++++++---------- src/test/unittests/server/ConfigTests.cpp | 54 ++++++++++++++++ 5 files changed, 124 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5ce6afcf7..6654157dc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,7 @@ Bug fixes: - #6975 Fix server stop working - #6976 Fix windows builds - #6979 Manual config error in client mode +- #6983 Fix that mac client listens on ipv4 only Enhancements: - #6954 Move language selection to advanced section diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index f02ca2462..2e4494705 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -838,6 +838,15 @@ QString MainWindow::configFilename() QString MainWindow::address() const { QString i = appConfig().networkInterface(); + // if interface is IPv6 - ensure that ip is in square brackets + if (i.count(':') > 1) { + if(i[0] != '[') { + i.insert(0, '['); + } + if(i[i.size() - 1] != ']') { + i.push_back(']'); + } + } return (!i.isEmpty() ? i : "") + ":" + QString::number(appConfig().port()); } diff --git a/src/lib/arch/unix/ArchNetworkBSD.cpp b/src/lib/arch/unix/ArchNetworkBSD.cpp index 922743432..ff4fed1db 100644 --- a/src/lib/arch/unix/ArchNetworkBSD.cpp +++ b/src/lib/arch/unix/ArchNetworkBSD.cpp @@ -679,14 +679,27 @@ ArchNetworkBSD::nameToAddr(const std::string& name) char ipstr[INET6_ADDRSTRLEN]; struct addrinfo hints; struct addrinfo *p; + struct in6_addr serveraddr; int ret; memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; + hints.ai_flags = AI_NUMERICSERV; + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_STREAM; + + if (inet_pton(AF_INET, name.c_str(), &serveraddr) == 1) { + hints.ai_family = AF_INET; + hints.ai_flags |= AI_NUMERICHOST; + } + else if (inet_pton(AF_INET6, name.c_str(), &serveraddr) == 1) { + hints.ai_family = AF_INET6; + hints.ai_flags |= AI_NUMERICHOST; + } // done with static buffer ARCH->lockMutex(m_mutex); - if ((ret = getaddrinfo(name.c_str(), NULL, &hints, &p)) != 0) { + ret = getaddrinfo(name.c_str(), nullptr, &hints, &p); + if (ret != 0) { ARCH->unlockMutex(m_mutex); delete addr; throwNameError(ret); @@ -697,6 +710,7 @@ ArchNetworkBSD::nameToAddr(const std::string& name) } else { addr->m_len = (socklen_t)sizeof(struct sockaddr_in6); } + memcpy(&addr->m_addr, p->ai_addr, addr->m_len); freeaddrinfo(p); ARCH->unlockMutex(m_mutex); @@ -992,4 +1006,4 @@ ArchNetworkBSD::throwNameError(int err) default: throw XArchNetworkName(s_msg[4]); } -} \ No newline at end of file +} diff --git a/src/lib/net/NetworkAddress.cpp b/src/lib/net/NetworkAddress.cpp index 66654ec4d..5bbcbc403 100644 --- a/src/lib/net/NetworkAddress.cpp +++ b/src/lib/net/NetworkAddress.cpp @@ -22,6 +22,7 @@ #include "arch/Arch.h" #include "arch/XArch.h" +#include #include // @@ -62,46 +63,53 @@ NetworkAddress::NetworkAddress(const String& hostname, int port) : m_hostname(hostname), m_port(port) { - // check for port suffix - String::size_type i = m_hostname.rfind(':'); - if (i != String::npos && i + 1 < m_hostname.size()) { - // found a colon. see if it looks like an IPv6 address. - bool colonNotation = false; - bool dotNotation = false; - bool doubleColon = false; - for (String::size_type j = 0; j < i; ++j) { - if (m_hostname[j] == ':') { - colonNotation = true; - dotNotation = false; - if (m_hostname[j + 1] == ':') { - doubleColon = true; - } - } - else if (m_hostname[j] == '.' && colonNotation) { - dotNotation = true; - } + //detect internet protocol version with colom count + auto isColomPredicate = [](char c){return c == ':';}; + auto colomCount = std::count_if(m_hostname.begin(), m_hostname.end(), isColomPredicate); + + if(colomCount == 1) { + //ipv4 with port part + auto hostIt = m_hostname.find(':'); + try { + m_port = std::stoi(m_hostname.substr(hostIt + 1)); + } catch(...) { + throw XSocketAddress(XSocketAddress::kBadPort, m_hostname, m_port); } - // port suffix is ambiguous with IPv6 notation if there's - // a double colon and the end of the address is not in dot - // notation. in that case we assume it's not a port suffix. - // the user can replace the double colon with zeros to - // disambiguate. - if ((!doubleColon || dotNotation) && !colonNotation) { - // parse port from hostname - char* end; - const char* chostname = m_hostname.c_str(); - long suffixPort = strtol(chostname + i + 1, &end, 10); - if (end == chostname + i + 1 || *end != '\0') { - throw XSocketAddress(XSocketAddress::kBadPort, - m_hostname, m_port); + auto endHostnameIt = static_cast(hostIt); + m_hostname = m_hostname.substr(0, endHostnameIt > 0 ? endHostnameIt : 0); + } + else if (colomCount > 1) { + //ipv6 part + if (m_hostname[0] == '[') { + //ipv6 with port part + String portDelimeter = "]:"; + auto hostIt = m_hostname.find(portDelimeter); + + //bad syntax of ipv6 with port + if (hostIt == String::npos) { + throw XSocketAddress(XSocketAddress::kUnknown, m_hostname, m_port); } - // trim port from hostname - m_hostname.erase(i); + auto portSuffix = m_hostname.substr(hostIt + portDelimeter.size()); + //port is implied but omitted + if (portSuffix.empty()) { + throw XSocketAddress(XSocketAddress::kBadPort, m_hostname, m_port); + } + try { + m_port = std::stoi(portSuffix); + } catch(...) { + //port is not a number + throw XSocketAddress(XSocketAddress::kBadPort, m_hostname, m_port); + } - // save port - m_port = static_cast(suffixPort); + auto endHostnameIt = static_cast(hostIt) - 1; + m_hostname = m_hostname.substr(1, endHostnameIt > 0 ? endHostnameIt : 0); + } + + // ensure that ipv6 link-local adress ended with scope id + if (m_hostname.rfind("fe80:", 0) == 0 && m_hostname.find('%') == String::npos) { + throw XSocketAddress(XSocketAddress::kUnknown, m_hostname, m_port); } } diff --git a/src/test/unittests/server/ConfigTests.cpp b/src/test/unittests/server/ConfigTests.cpp index d661cbc5b..d38a89c19 100644 --- a/src/test/unittests/server/ConfigTests.cpp +++ b/src/test/unittests/server/ConfigTests.cpp @@ -16,6 +16,7 @@ */ #include "lib/server/Config.h" +#include "net/XSocket.h" #include "test/global/gtest.h" class OnlySystemFilter: public InputFilter::Condition { @@ -84,6 +85,59 @@ TEST(ServerConfigTests, serverconfig_will_deem_equal_configs_with_same_cell_name EXPECT_TRUE(b == a); } +TEST(NetworkAddress, hostname_valid_parsing) +{ + const int validPort = 24900; + const String portStr = std::to_string(validPort); + + //list of test cases. 1 param - hostname for parsing, 2 param - port, 3 param - expected hostname + const std::initializer_list> validTestCases = { + std::make_tuple(String("127.0.0.1"), validPort, "127.0.0.1"), + std::make_tuple(String("127.0.0.1:") + portStr, 0, "127.0.0.1"), + std::make_tuple(String(":") + portStr, 0, ""), + std::make_tuple(String("localhost"), validPort, "localhost"), + std::make_tuple(String("localhost:") + portStr, 0, "localhost"), + std::make_tuple(String(""), validPort, ""), + std::make_tuple(String("[::1]:") + portStr, 0, "::1"), + std::make_tuple(String("[fe80::a156:9f36:793:7bfb%14]:") + portStr, 0, "fe80::a156:9f36:793:7bfb%14"), + std::make_tuple(String("::1"), validPort, "::1"), + std::make_tuple(String("fe80::a156:9f36:793:7bfb%14"), validPort, "fe80::a156:9f36:793:7bfb%14"), + std::make_tuple(String("fe80:0000:0000:0000:a156:9f36:793:7bfb%14"), validPort, "fe80:0000:0000:0000:a156:9f36:793:7bfb%14"), + }; + + for (const auto &caseParams : validTestCases) { + NetworkAddress addr(std::get<0>(caseParams), std::get<1>(caseParams)); + addr.resolve(); + + EXPECT_TRUE(addr.getHostname() == std::get<2>(caseParams)); + EXPECT_TRUE(addr.getPort() == validPort); + EXPECT_TRUE(addr.getAddress() != nullptr); + } + + //list of non valid hostnames + const std::initializer_list nonValidTestCases = { + ":nonValidPort", + ":", + "[::1]:", + "[::1]:nonValidPort", + "fe80::1", + "[::1]:-1", + "[::1]:65536" + }; + + for (const auto &caseParam : nonValidTestCases) { + bool flag = false; + try { + NetworkAddress addr(caseParam, validPort); + } catch (const XSocketAddress&) { + + flag = true; + } + + EXPECT_TRUE(flag); + } +} + TEST(ServerConfigTests, serverconfig_will_deem_different_configs_with_same_cell_names_different_options) { Config a(nullptr); From 7c365c4dac15cd401182f49dd014464ed2c7d729 Mon Sep 17 00:00:00 2001 From: SerhiiGadzhilov <71632867+SerhiiGadzhilov@users.noreply.github.com> Date: Wed, 28 Apr 2021 10:06:56 +0300 Subject: [PATCH 2/2] SYNERGY-694 Setup client configuration issues (#6987) * SYNERGY-694 Fix compilation for old platforms * SYNERGY-694 Add server screen if it hasn't been added. * SYNERGY-694 Fix issue with repeat popups * Update ChangeLog --- ChangeLog | 2 +- src/gui/src/MainWindow.cpp | 5 ++--- src/gui/src/ServerConfigDialog.cpp | 12 +++++------- src/gui/src/ServerConnection.cpp | 6 ++++-- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6654157dc..09f29fd59 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,7 +19,7 @@ Enhancements: - #6973 Update synergy UI. Main window - #6977 Update synergy UI. Configure server - #6978 Update synergy UI. Settings window -- #6981 Update synergy UI. Setup client configuration +- #6981 | #6987 Update synergy UI. Setup client configuration - #6984 Update synergy UI. Client error messages - #6962 | #6965 Add macOS 10.13 builder =========== diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 2e4494705..29568f159 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -461,8 +461,7 @@ void MainWindow::checkConnected(const QString& line) m_pLabelClientState->updateClientState(line); } - if (line.contains("connected to server") || - line.contains("accepted client connection")) + if (line.contains("connected to server") || line.contains("has connected")) { setSynergyState(synergyConnected); @@ -1062,7 +1061,7 @@ QString MainWindow::getIPAddresses() for (const auto& address : addresses) { if (address.protocol() == QAbstractSocket::IPv4Protocol && address != QHostAddress(QHostAddress::LocalHost) && - !address.isLinkLocal()) { + !address.isInSubnet(QHostAddress::parseSubnet("169.254.0.0/16"))) { // usually 192.168.x.x is a useful ip for the user, so indicate // this by making it bold. diff --git a/src/gui/src/ServerConfigDialog.cpp b/src/gui/src/ServerConfigDialog.cpp index bf004dfd4..266838b7b 100644 --- a/src/gui/src/ServerConfigDialog.cpp +++ b/src/gui/src/ServerConfigDialog.cpp @@ -70,18 +70,16 @@ ServerConfigDialog::ServerConfigDialog(QWidget* parent, ServerConfig& config) : m_pScreenSetupView->setModel(&m_ScreenSetupModel); - if (serverConfig().numScreens() == 0) { + auto& screens = serverConfig().screens(); + auto server = std::find_if(screens.begin(), screens.end(), [this](const Screen& screen){ return (screen.name() == serverConfig().getServerName());}); + + if (server == screens.end()) { Screen serverScreen(serverConfig().getServerName()); serverScreen.markAsServer(); model().screen(serverConfig().numColumns() / 2, serverConfig().numRows() / 2) = serverScreen; } else { - for (auto& screen : serverConfig().screens()) { - if (screen.name() == serverConfig().getServerName()) { - screen.markAsServer(); - break; - } - } + server->markAsServer(); } m_pButtonAddComputer->setEnabled(!model().isFull()); diff --git a/src/gui/src/ServerConnection.cpp b/src/gui/src/ServerConnection.cpp index fba9b62c4..30ad1d361 100644 --- a/src/gui/src/ServerConnection.cpp +++ b/src/gui/src/ServerConnection.cpp @@ -61,6 +61,8 @@ void ServerConnection::addClient(const QString& clientName) { if (!m_parent.serverConfig().isFull() && checkMainWindow()) { + m_parent.stopSynergy(); + QMessageBox message(&m_parent); message.addButton(QObject::tr("Ignore"), QMessageBox::RejectRole); message.addButton(QObject::tr("Accept and configure"), QMessageBox::AcceptRole); @@ -74,6 +76,8 @@ void ServerConnection::addClient(const QString& clientName) { m_ignoredClients.append(clientName); } + + m_parent.startSynergy(); } } @@ -84,6 +88,4 @@ void ServerConnection::configureClient(const QString& clientName) ServerConfigDialog dlg(&m_parent, config); dlg.exec(); - - m_parent.restartSynergy(); }