From 6638076b856e7e47b55f3371c3200a814b5f0935 Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Fri, 18 Sep 2026 16:27:24 +0100 Subject: [PATCH] fix(server): send key repeat without language to pre-1.8 clients fixes: #10069 fixes: #9483 fixes: #9101 The 2021 language change put the string into the frozen 1.1 proxy, so every client below 1.8 (Barrier, Input Leap, Synergy 1.14.1 and older) disconnected on the first key repeat. Adds a test pinning the bytes each proxy version sends. --- docs/dev/protocol_reference.md | 3 +- src/lib/deskflow/ProtocolTypes.cpp | 1 + src/lib/deskflow/ProtocolTypes.h | 42 +++- src/lib/server/ClientProxy1_1.cpp | 13 +- src/lib/server/ClientProxy1_8.cpp | 17 +- src/lib/server/ClientProxy1_8.h | 1 + src/unittests/server/CMakeLists.txt | 7 + src/unittests/server/ClientProxyTests.cpp | 225 ++++++++++++++++++++++ src/unittests/server/ClientProxyTests.h | 28 +++ 9 files changed, 320 insertions(+), 17 deletions(-) create mode 100644 src/unittests/server/ClientProxyTests.cpp create mode 100644 src/unittests/server/ClientProxyTests.h diff --git a/docs/dev/protocol_reference.md b/docs/dev/protocol_reference.md index bde8826fe..3914d6bc3 100644 --- a/docs/dev/protocol_reference.md +++ b/docs/dev/protocol_reference.md @@ -158,7 +158,8 @@ This table lists all protocol messages in alphabetical order. For a typical sequ | [**DKDL**](@ref kMsgDKeyDown) | @ref kMsgDKeyDown | Data | Server→Client | Key down with language | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.8+ | | [**DKDN**](@ref kMsgDKeyDown1_1) | @ref kMsgDKeyDown1_1 | Data | Server→Client | Key down | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.1+ | | [**DKDN**](@ref kMsgDKeyDown1_0) | @ref kMsgDKeyDown1_0 | Data | Server→Client | Key down (legacy) | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.0 | -| [**DKRP**](@ref kMsgDKeyRepeat) | @ref kMsgDKeyRepeat | Data | Server→Client | Key repeat | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.1+ | +| [**DKRP**](@ref kMsgDKeyRepeat) | @ref kMsgDKeyRepeat | Data | Server→Client | Key repeat with language | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.8+ | +| [**DKRP**](@ref kMsgDKeyRepeat1_1) | @ref kMsgDKeyRepeat1_1 | Data | Server→Client | Key repeat | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.1+ | | [**DKRP**](@ref kMsgDKeyRepeat1_0) | @ref kMsgDKeyRepeat1_0 | Data | Server→Client | Key repeat (legacy) | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.0 | | [**DKUP**](@ref kMsgDKeyUp) | @ref kMsgDKeyUp | Data | Server→Client | Key up | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.1+ | | [**DKUP**](@ref kMsgDKeyUp1_0) | @ref kMsgDKeyUp1_0 | Data | Server→Client | Key up (legacy) | [MsgSize](#constraint-protocol-max-message-length), [KeyMap](#constraint-keymap) | 1.0 | diff --git a/src/lib/deskflow/ProtocolTypes.cpp b/src/lib/deskflow/ProtocolTypes.cpp index cac481e65..41fd97080 100644 --- a/src/lib/deskflow/ProtocolTypes.cpp +++ b/src/lib/deskflow/ProtocolTypes.cpp @@ -31,6 +31,7 @@ const char *const kMsgDKeyDown1_1 = "DKDN%2i%2i%2i"; const char *const kMsgDKeyDown1_0 = "DKDN%2i%2i"; const char *const kMsgDKeyRepeat = "DKRP%2i%2i%2i%2i%s"; +const char *const kMsgDKeyRepeat1_1 = "DKRP%2i%2i%2i%2i"; const char *const kMsgDKeyRepeat1_0 = "DKRP%2i%2i%2i"; const char *const kMsgDKeyUp = "DKUP%2i%2i%2i"; const char *const kMsgDKeyUp1_0 = "DKUP%2i%2i"; diff --git a/src/lib/deskflow/ProtocolTypes.h b/src/lib/deskflow/ProtocolTypes.h index b27c2214f..56d6221da 100644 --- a/src/lib/deskflow/ProtocolTypes.h +++ b/src/lib/deskflow/ProtocolTypes.h @@ -616,11 +616,43 @@ extern const char *const kMsgDKeyDown1_0; * Sent when a key is held down and auto-repeating. The repeat count * indicates how many repeat events occurred since the last message. * - * @see kMsgDKeyDown - * @since Protocol version 1.1 + * Shares the `DKRP` message code with kMsgDKeyRepeat1_1, so the receiver can + * only tell the two apart by the negotiated protocol version. Only sent to + * clients that replied 1.8 in their hello. + * + * @see kMsgDKeyDown, kMsgDKeyRepeat1_1 + * @since Protocol version 1.8 */ extern const char *const kMsgDKeyRepeat; +/** + * @brief Key auto-repeat event (v1.1 to v1.7) + * + * **Message Code**: `"DKRP"` + * **Direction**: Primary → Secondary + * **Format**: `"DKRP%2i%2i%2i%2i"` + * **Parameters**: + * - `$1`: KeyID (2 bytes) - Virtual key identifier + * - `$2`: KeyModifierMask (2 bytes) - Active modifier keys + * - `$3`: Repeat count (2 bytes) - Number of repeats + * - `$4`: KeyButton (2 bytes) - Physical key code + * + * **Example**: + * + * 'a' key repeating 3 times + * ``` + * "DKRP\x00\x61\x00\x00\x00\x03\x00\x1E" + * ``` + * + * Version without the language code. Used when communicating with + * protocol version 1.1 through 1.7 clients (Synergy 1.4 to 1.14.1, + * Barrier, Input Leap). + * + * @see kMsgDKeyRepeat + * @since Protocol version 1.1 + */ +extern const char *const kMsgDKeyRepeat1_1; + /** * @brief Key auto-repeat event (legacy v1.0) * @@ -632,10 +664,10 @@ extern const char *const kMsgDKeyRepeat; * - `$2`: KeyModifierMask (2 bytes) - Active modifier keys * - `$3`: Repeat count (2 bytes) - Number of repeats * - * Legacy version without KeyButton and language parameters. + * Legacy version without the KeyButton parameter. * - * @deprecated Use kMsgDKeyRepeat for protocol version 1.1+ - * @see kMsgDKeyRepeat + * @deprecated Use kMsgDKeyRepeat1_1 for protocol version 1.1+ + * @see kMsgDKeyRepeat1_1 * @since Protocol version 1.0 */ extern const char *const kMsgDKeyRepeat1_0; diff --git a/src/lib/server/ClientProxy1_1.cpp b/src/lib/server/ClientProxy1_1.cpp index ea512cf1f..96133a73d 100644 --- a/src/lib/server/ClientProxy1_1.cpp +++ b/src/lib/server/ClientProxy1_1.cpp @@ -26,16 +26,13 @@ void ClientProxy1_1::keyDown(KeyID key, KeyModifierMask mask, KeyButton button, ProtocolUtil::writef(getStream(), kMsgDKeyDown1_1, key, mask, button); } -void ClientProxy1_1::keyRepeat( - KeyID key, KeyModifierMask mask, int32_t count, KeyButton button, const std::string &lang -) +void ClientProxy1_1::keyRepeat(KeyID key, KeyModifierMask mask, int32_t count, KeyButton button, const std::string &) { - LOG( - (CLOG_VERBOSE "send key repeat to \"%s\" id=%d, mask=0x%04x, count=%d, " - "button=0x%04x, lang=\"%s\"", - getName().c_str(), key, mask, count, button, lang.c_str()) + LOG_VERBOSE( + "send key repeat to \"%s\" id=%d, mask=0x%04x, count=%d, button=0x%04x", getName().c_str(), key, mask, count, + button ); - ProtocolUtil::writef(getStream(), kMsgDKeyRepeat, key, mask, count, button, &lang); + ProtocolUtil::writef(getStream(), kMsgDKeyRepeat1_1, key, mask, count, button); } void ClientProxy1_1::keyUp(KeyID key, KeyModifierMask mask, KeyButton button) diff --git a/src/lib/server/ClientProxy1_8.cpp b/src/lib/server/ClientProxy1_8.cpp index 14cefa5ac..0d22ecd08 100644 --- a/src/lib/server/ClientProxy1_8.cpp +++ b/src/lib/server/ClientProxy1_8.cpp @@ -32,9 +32,20 @@ void ClientProxy1_8::synchronizeLanguages() const void ClientProxy1_8::keyDown(KeyID key, KeyModifierMask mask, KeyButton button, const std::string &language) { - LOG( - (CLOG_VERBOSE "send key down to \"%s\" id=%d, mask=0x%04x, button=0x%04x, layout=%s", getName().c_str(), key, - mask, button, language.c_str()) + LOG_VERBOSE( + "send key down to \"%s\" id=%d, mask=0x%04x, button=0x%04x, layout=%s", getName().c_str(), key, mask, button, + language.c_str() ); ProtocolUtil::writef(getStream(), kMsgDKeyDown, key, mask, button, &language); } + +void ClientProxy1_8::keyRepeat( + KeyID key, KeyModifierMask mask, int32_t count, KeyButton button, const std::string &language +) +{ + LOG_VERBOSE( + "send key repeat to \"%s\" id=%d, mask=0x%04x, count=%d, button=0x%04x, layout=%s", getName().c_str(), key, mask, + count, button, language.c_str() + ); + ProtocolUtil::writef(getStream(), kMsgDKeyRepeat, key, mask, count, button, &language); +} diff --git a/src/lib/server/ClientProxy1_8.h b/src/lib/server/ClientProxy1_8.h index 44169daaa..9f48438ec 100644 --- a/src/lib/server/ClientProxy1_8.h +++ b/src/lib/server/ClientProxy1_8.h @@ -15,6 +15,7 @@ public: ~ClientProxy1_8() override = default; void keyDown(KeyID, KeyModifierMask, KeyButton, const std::string &) override; + void keyRepeat(KeyID, KeyModifierMask, int32_t count, KeyButton, const std::string &) override; private: void synchronizeLanguages() const; diff --git a/src/unittests/server/CMakeLists.txt b/src/unittests/server/CMakeLists.txt index 864d1462f..f43ed06a7 100644 --- a/src/unittests/server/CMakeLists.txt +++ b/src/unittests/server/CMakeLists.txt @@ -21,3 +21,10 @@ create_test( WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/src/lib/server" ) +create_test( + NAME ClientProxyTests + DEPENDS server + LIBS base arch io mt net platform app ${extra_libs} + SOURCE ClientProxyTests.cpp + WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/src/lib/server" +) diff --git a/src/unittests/server/ClientProxyTests.cpp b/src/unittests/server/ClientProxyTests.cpp new file mode 100644 index 000000000..560ec49b6 --- /dev/null +++ b/src/unittests/server/ClientProxyTests.cpp @@ -0,0 +1,225 @@ +/* + * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2026 Synergy App Ltd + * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception + */ + +#include "ClientProxyTests.h" + +#include "../deskflow/MockEventQueue.h" +#include "deskflow/AppUtil.h" +#include "io/IStream.h" +#include "server/ClientProxy1_0.h" +#include "server/ClientProxy1_1.h" +#include "server/ClientProxy1_6.h" +#include "server/ClientProxy1_7.h" +#include "server/ClientProxy1_8.h" + +#include +#include +#include + +#include +#include + +namespace { + +class TestAppUtil : public AppUtil +{ +public: + int run() override + { + return 0; + } + + std::vector getKeyboardLayoutList() override + { + return {"en"}; + } + + std::string getCurrentLanguageCode() override + { + return "en"; + } +}; + +class CapturingStream : public deskflow::IStream +{ +public: + QByteArray take() + { + auto bytes = m_buffer; + m_buffer.clear(); + return bytes; + } + + void write(const void *buffer, uint32_t n) override + { + m_buffer.append(static_cast(buffer), n); + } + + void close() override + { + } + + uint32_t read(void *, uint32_t) override + { + return 0; + } + + void flush() override + { + } + + void shutdownInput() override + { + } + + void shutdownOutput() override + { + } + + void *getEventTarget() const override + { + return const_cast(this); + } + + bool isReady() const override + { + return false; + } + + uint32_t getSize() const override + { + return 0; + } + +private: + QByteArray m_buffer; +}; + +std::unique_ptr makeProxy(int minor, deskflow::IStream *stream, IEventQueue *events) +{ + // the 1.4 and later constructors assert the server pointer is non-null but only store it + auto *server = reinterpret_cast(0x1); + + std::unique_ptr proxy; + switch (minor) { + case 0: + proxy = std::make_unique("client", stream, events); + break; + case 1: + proxy = std::make_unique("client", stream, events); + break; + case 6: + proxy = std::make_unique("client", stream, server, events); + break; + case 7: + proxy = std::make_unique("client", stream, server, events); + break; + case 8: + proxy = std::make_unique("client", stream, server, events); + break; + default: + break; + } + return proxy; +} + +struct ProxyUnderTest +{ + MockEventQueue events; + CapturingStream *stream = new CapturingStream; + std::unique_ptr proxy; + + explicit ProxyUnderTest(int minor) : proxy(makeProxy(minor, stream, &events)) + { + // drop the query-info and layout-sync messages the constructors send + stream->take(); + } +}; + +const KeyID kKey = 0x61; +const KeyModifierMask kMask = 0; +const KeyButton kButton = 0x1e; +const int32_t kCount = 3; +const std::string kLang = "en"; + +} // namespace + +void ClientProxyTests::initTestCase() +{ + // the 1.8 constructor reads the keyboard layouts through AppUtil::instance() + static TestAppUtil appUtil; +} + +// These formats are frozen because shipped third-party clients parse them byte +// for byte: Synergy 1.4 through 1.14.1 negotiate 1.4 through 1.7, Barrier and +// Input Leap negotiate 1.6, and Synergy 1.14.2 onwards and Deskflow negotiate 1.8. +void ClientProxyTests::keyDown_data() +{ + QTest::addColumn("minor"); + QTest::addColumn("expected"); + + QTest::newRow("1.0") << 0 << "DKDN" + QByteArray::fromHex("0061 0000"); + QTest::newRow("1.1") << 1 << "DKDN" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.6") << 6 << "DKDN" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.7") << 7 << "DKDN" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.8") << 8 << "DKDL" + QByteArray::fromHex("0061 0000 001e 00000002") + "en"; +} + +void ClientProxyTests::keyDown() +{ + QFETCH(int, minor); + QFETCH(QByteArray, expected); + + ProxyUnderTest test(minor); + test.proxy->keyDown(kKey, kMask, kButton, kLang); + QCOMPARE(test.stream->take(), expected); +} + +void ClientProxyTests::keyRepeat_data() +{ + QTest::addColumn("minor"); + QTest::addColumn("expected"); + + QTest::newRow("1.0") << 0 << "DKRP" + QByteArray::fromHex("0061 0000 0003"); + QTest::newRow("1.1") << 1 << "DKRP" + QByteArray::fromHex("0061 0000 0003 001e"); + QTest::newRow("1.6") << 6 << "DKRP" + QByteArray::fromHex("0061 0000 0003 001e"); + QTest::newRow("1.7") << 7 << "DKRP" + QByteArray::fromHex("0061 0000 0003 001e"); + QTest::newRow("1.8") << 8 << "DKRP" + QByteArray::fromHex("0061 0000 0003 001e 00000002") + "en"; +} + +void ClientProxyTests::keyRepeat() +{ + QFETCH(int, minor); + QFETCH(QByteArray, expected); + + ProxyUnderTest test(minor); + test.proxy->keyRepeat(kKey, kMask, kCount, kButton, kLang); + QCOMPARE(test.stream->take(), expected); +} + +void ClientProxyTests::keyUp_data() +{ + QTest::addColumn("minor"); + QTest::addColumn("expected"); + + QTest::newRow("1.0") << 0 << "DKUP" + QByteArray::fromHex("0061 0000"); + QTest::newRow("1.1") << 1 << "DKUP" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.6") << 6 << "DKUP" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.7") << 7 << "DKUP" + QByteArray::fromHex("0061 0000 001e"); + QTest::newRow("1.8") << 8 << "DKUP" + QByteArray::fromHex("0061 0000 001e"); +} + +void ClientProxyTests::keyUp() +{ + QFETCH(int, minor); + QFETCH(QByteArray, expected); + + ProxyUnderTest test(minor); + test.proxy->keyUp(kKey, kMask, kButton); + QCOMPARE(test.stream->take(), expected); +} + +QTEST_MAIN(ClientProxyTests) diff --git a/src/unittests/server/ClientProxyTests.h b/src/unittests/server/ClientProxyTests.h new file mode 100644 index 000000000..96a6b26fa --- /dev/null +++ b/src/unittests/server/ClientProxyTests.h @@ -0,0 +1,28 @@ +/* + * Deskflow -- mouse and keyboard sharing utility + * SPDX-FileCopyrightText: (C) 2026 Synergy App Ltd + * SPDX-License-Identifier: GPL-2.0-only WITH LicenseRef-OpenSSL-Exception + */ + +#pragma once + +#include "base/Log.h" + +#include + +class ClientProxyTests : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + void keyDown_data(); + void keyDown(); + void keyRepeat_data(); + void keyRepeat(); + void keyUp_data(); + void keyUp(); + +private: + Log m_log; +};